From 73b0c4ea8116c4b1088dcd84cb2f4d7181baf7ab Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 30 Apr 2018 05:00:51 +0200 Subject: TunnelEditorFragment: rewrite and simplify This should remove some null pointer dereferences and overall make the thing more robust. Signed-off-by: Jason A. Donenfeld --- .../android/fragment/TunnelEditorFragment.java | 186 +++++++++------------ 1 file changed, 78 insertions(+), 108 deletions(-) (limited to 'app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java') diff --git a/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java b/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java index 0b4d74d8..e4555512 100644 --- a/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java +++ b/app/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.java @@ -35,65 +35,50 @@ public class TunnelEditorFragment extends BaseFragment { private static final String TAG = "WireGuard/" + TunnelEditorFragment.class.getSimpleName(); private TunnelEditorFragmentBinding binding; - private boolean isViewStateRestored; - private Config localConfig = new Config(); - private Tunnel localTunnel; - private String originalName; - - private static T copyParcelable(final T original) { - if (original == null) - return null; - final Parcel parcel = Parcel.obtain(); - parcel.writeParcelable(original, 0); - parcel.setDataPosition(0); - final T copy = parcel.readParcelable(original.getClass().getClassLoader()); - parcel.recycle(); - return copy; + private Tunnel tunnel; + + private void onConfigLoaded(final String name, final Config config) { + binding.setConfig(new Config.Observable(config, name)); } - private void onConfigLoaded(final Config config) { - localConfig = copyParcelable(config); - if (binding != null && isViewStateRestored) - binding.setConfig(new Config.Observable(localConfig, originalName)); + @Override + public void onCreate(final Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + setHasOptionsMenu(true); } - private void onConfigSaved(@SuppressWarnings("unused") final Config config, - final Throwable throwable) { - final String message; - if (throwable == null) { - message = getString(R.string.config_save_success, localTunnel.getName()); - Log.d(TAG, message); - onFinished(); - } else { - final String error = ExceptionLoggers.unwrap(throwable).getMessage(); - message = getString(R.string.config_save_error, localTunnel.getName(), error); - Log.e(TAG, message, throwable); - if (binding != null) { - final CoordinatorLayout container = binding.mainContainer; - Snackbar.make(container, message, Snackbar.LENGTH_LONG).show(); - } - } + @Override + public void onSelectedTunnelChanged(final Tunnel oldTunnel, final Tunnel newTunnel) { + tunnel = newTunnel; + if (binding == null) + return; + binding.setConfig(new Config.Observable(null, null)); + if (tunnel != null) + tunnel.getConfigAsync().thenAccept(a -> onConfigLoaded(tunnel.getName(), a)); } @Override - public void onCreate(final Bundle savedInstanceState) { - super.onCreate(savedInstanceState); - if (savedInstanceState != null) { - localConfig = savedInstanceState.getParcelable(KEY_LOCAL_CONFIG); - originalName = savedInstanceState.getString(KEY_ORIGINAL_NAME); - } - // Erase the remains of creating or editing a different tunnel. - if (getSelectedTunnel() != null && !getSelectedTunnel().getName().equals(originalName)) { - // The config must be loaded asynchronously since it's not an observable property. - localConfig = null; - originalName = getSelectedTunnel().getName(); - getSelectedTunnel().getConfigAsync().thenAccept(this::onConfigLoaded); - } else if (getSelectedTunnel() == null && originalName != null) { - localConfig = new Config(); - originalName = null; + public void onSaveInstanceState(@NonNull final Bundle outState) { + outState.putParcelable(KEY_LOCAL_CONFIG, binding.getConfig()); + outState.putString(KEY_ORIGINAL_NAME, tunnel == null ? null : tunnel.getName()); + super.onSaveInstanceState(outState); + } + + @Override + public void onViewStateRestored(final Bundle savedInstanceState) { + if (savedInstanceState == null) { + onSelectedTunnelChanged(null, getSelectedTunnel()); + } else { + tunnel = getSelectedTunnel(); + Config.Observable config = savedInstanceState.getParcelable(KEY_LOCAL_CONFIG); + String originalName = savedInstanceState.getString(KEY_ORIGINAL_NAME); + if (tunnel != null && !tunnel.getName().equals(originalName)) + onSelectedTunnelChanged(null, tunnel); + else + binding.setConfig(config); } - localTunnel = getSelectedTunnel(); - setHasOptionsMenu(true); + + super.onViewStateRestored(savedInstanceState); } @Override @@ -131,7 +116,7 @@ public class TunnelEditorFragment extends BaseFragment { // Tell the activity to finish itself or go back to the detail view. getActivity().runOnUiThread(() -> { // The selected tunnel has to actually change, but we have to remember this one. - final Tunnel savedTunnel = localTunnel; + final Tunnel savedTunnel = tunnel; if (savedTunnel == getSelectedTunnel()) setSelectedTunnel(null); setSelectedTunnel(savedTunnel); @@ -142,32 +127,31 @@ public class TunnelEditorFragment extends BaseFragment { public boolean onOptionsItemSelected(final MenuItem item) { switch (item.getItemId()) { case R.id.menu_action_save: - final Tunnel selectedTunnel = getSelectedTunnel(); - if (localConfig != null) { - try { - binding.getConfig().commitData(localConfig); - } catch (Exception e) { - final String error = ExceptionLoggers.unwrap(e).getMessage(); - final String message = getString(R.string.config_save_error, localTunnel.getName(), error); - Log.e(TAG, message, e); - final CoordinatorLayout container = binding.mainContainer; - Snackbar.make(container, error, Snackbar.LENGTH_LONG).show(); - return false; - } + Config newConfig = new Config(); + try { + binding.getConfig().commitData(newConfig); + } catch (Exception e) { + final String error = ExceptionLoggers.unwrap(e).getMessage(); + final String tunnelName = tunnel == null ? binding.getConfig().getName() : tunnel.getName(); + final String message = getString(R.string.config_save_error, tunnelName, error); + Log.e(TAG, message, e); + final CoordinatorLayout container = binding.mainContainer; + Snackbar.make(container, error, Snackbar.LENGTH_LONG).show(); + return false; } - if (selectedTunnel == null) { + if (tunnel == null) { Log.d(TAG, "Attempting to create new tunnel " + binding.getConfig().getName()); final TunnelManager manager = Application.getComponent().getTunnelManager(); - manager.create(binding.getConfig().getName(), localConfig) + manager.create(binding.getConfig().getName(), newConfig) .whenComplete(this::onTunnelCreated); - } else if (!selectedTunnel.getName().equals(binding.getConfig().getName())) { + } else if (!tunnel.getName().equals(binding.getConfig().getName())) { Log.d(TAG, "Attempting to rename tunnel to " + binding.getConfig().getName()); - selectedTunnel.setName(binding.getConfig().getName()) - .whenComplete(this::onTunnelRenamed); - } else if (localConfig != null) { - Log.d(TAG, "Attempting to save config of " + selectedTunnel.getName()); - selectedTunnel.setConfig(localConfig) - .whenComplete(this::onConfigSaved); + tunnel.setName(binding.getConfig().getName()) + .whenComplete((a, b) -> onTunnelRenamed(tunnel, newConfig, a, b)); + } else { + Log.d(TAG, "Attempting to save config of " + tunnel.getName()); + tunnel.setConfig(newConfig) + .whenComplete((a, b) -> onConfigSaved(tunnel, a, b)); } return true; default: @@ -175,36 +159,30 @@ public class TunnelEditorFragment extends BaseFragment { } } - @Override - public void onSaveInstanceState(@NonNull final Bundle outState) { - outState.putParcelable(KEY_LOCAL_CONFIG, localConfig); - outState.putString(KEY_ORIGINAL_NAME, originalName); - super.onSaveInstanceState(outState); - } - - @Override - public void onSelectedTunnelChanged(final Tunnel oldTunnel, final Tunnel newTunnel) { - // Erase the remains of creating or editing a different tunnel. - if (newTunnel != null) { - // The config must be loaded asynchronously since it's not an observable property. - localConfig = null; - originalName = newTunnel.getName(); - newTunnel.getConfigAsync().thenAccept(this::onConfigLoaded); + private void onConfigSaved(final Tunnel savedTunnel, final Config config, + final Throwable throwable) { + final String message; + if (throwable == null) { + message = getString(R.string.config_save_success, savedTunnel.getName()); + Log.d(TAG, message); + onFinished(); } else { - localConfig = new Config(); - if (binding != null && isViewStateRestored) - binding.setConfig(new Config.Observable(localConfig, "")); - originalName = null; + final String error = ExceptionLoggers.unwrap(throwable).getMessage(); + message = getString(R.string.config_save_error, savedTunnel.getName(), error); + Log.e(TAG, message, throwable); + if (binding != null) { + final CoordinatorLayout container = binding.mainContainer; + Snackbar.make(container, message, Snackbar.LENGTH_LONG).show(); + } } - localTunnel = newTunnel; } - private void onTunnelCreated(final Tunnel tunnel, final Throwable throwable) { + private void onTunnelCreated(final Tunnel newTunnel, final Throwable throwable) { final String message; if (throwable == null) { + tunnel = newTunnel; message = getString(R.string.tunnel_create_success, tunnel.getName()); Log.d(TAG, message); - localTunnel = tunnel; onFinished(); } else { final String error = ExceptionLoggers.unwrap(throwable).getMessage(); @@ -217,14 +195,15 @@ public class TunnelEditorFragment extends BaseFragment { } } - private void onTunnelRenamed(final String name, final Throwable throwable) { + private void onTunnelRenamed(final Tunnel renamedTunnel, final Config newConfig, + final String name, final Throwable throwable) { final String message; if (throwable == null) { - message = getString(R.string.tunnel_rename_success, localTunnel.getName(), name); + message = getString(R.string.tunnel_rename_success, renamedTunnel.getName()); Log.d(TAG, message); // Now save the rest of configuration changes. - Log.d(TAG, "Attempting to save config of renamed tunnel " + localTunnel.getName()); - localTunnel.setConfig(localConfig).whenComplete(this::onConfigSaved); + Log.d(TAG, "Attempting to save config of renamed tunnel " + tunnel.getName()); + renamedTunnel.setConfig(newConfig).whenComplete((a, b) -> onConfigSaved(renamedTunnel, a, b)); } else { final String error = ExceptionLoggers.unwrap(throwable).getMessage(); message = getString(R.string.tunnel_rename_error, error); @@ -235,13 +214,4 @@ public class TunnelEditorFragment extends BaseFragment { } } } - - @Override - public void onViewStateRestored(final Bundle savedInstanceState) { - super.onViewStateRestored(savedInstanceState); - if (localConfig == null) - localConfig = new Config(); - binding.setConfig(new Config.Observable(localConfig, originalName)); - isViewStateRestored = true; - } } -- cgit v1.2.3-59-g8ed1b