From f9faf01dc0f847e51faafbe9b4b5082719cbf3cb Mon Sep 17 00:00:00 2001 From: cyBerta Date: Mon, 7 Aug 2023 01:25:39 +0200 Subject: Fixes various bugs related to the re-creation of the SetupActivity and the loaded Fragments after an layout orientation change. * fixes the initialization of fragments via a Bundle * saves state before the Activity/Fragments get destroyed and loads the old state on re-creation * fixes onFragmentSelected() callback handling --- .../providersetup/activities/SetupActivity.java | 24 +++++++++- .../providersetup/fragments/BaseSetupFragment.java | 54 ++++++++++++++++----- .../fragments/CircumventionSetupFragment.java | 8 ++-- .../fragments/ConfigureProviderFragment.java | 42 +++++++--------- .../fragments/EmptyPermissionSetupFragment.java | 48 ++++++++++++++----- .../fragments/NotificationSetupFragment.java | 8 ++-- .../fragments/ProviderSelectionFragment.java | 56 ++++++++++------------ .../fragments/SetupSuccessFragment.java | 9 ++-- .../fragments/VpnPermissionSetupFragment.java | 14 ++---- .../viewmodel/ProviderSelectionViewModel.java | 4 ++ 10 files changed, 159 insertions(+), 108 deletions(-) (limited to 'app') diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/SetupActivity.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/SetupActivity.java index 439a0f6a..27ca6658 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/SetupActivity.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/SetupActivity.java @@ -54,9 +54,13 @@ import se.leap.bitmaskclient.tor.TorStatusObservable; public class SetupActivity extends AppCompatActivity implements SetupActivityCallback, ProviderSetupFailedDialog.DownloadFailedDialogInterface { + public static final String EXTRA_PROVIDER = "EXTRA_PROVIDER"; + public static final String EXTRA_CURRENT_POSITION = "EXTRA_CURRENT_POSITION"; private static final String TAG = SetupActivity.class.getSimpleName(); ActivitySetupBinding binding; Provider provider; + private int currentPosition = 0; + private final HashSet cancelCallbacks = new HashSet<>(); private FragmentManagerEnhanced fragmentManager; SetupViewPagerAdapter adapter; @@ -65,6 +69,11 @@ public class SetupActivity extends AppCompatActivity implements SetupActivityCal @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); + if (savedInstanceState != null) { + provider = savedInstanceState.getParcelable(EXTRA_PROVIDER); + currentPosition = savedInstanceState.getInt(EXTRA_CURRENT_POSITION); + } + binding = ActivitySetupBinding.inflate(getLayoutInflater()); setContentView(binding.getRoot()); fragmentManager = new FragmentManagerEnhanced(getSupportFragmentManager()); @@ -97,6 +106,7 @@ public class SetupActivity extends AppCompatActivity implements SetupActivityCal @Override public void onPageSelected(int position) { super.onPageSelected(position); + currentPosition = position; for (int i = 0; i < indicatorViews.size(); i++) { ((ViewGroup) indicatorViews.get(i)). getChildAt(0). @@ -110,6 +120,8 @@ public class SetupActivity extends AppCompatActivity implements SetupActivityCal }); binding.viewPager.setAdapter(adapter); binding.viewPager.setUserInputEnabled(false); + binding.viewPager.setCurrentItem(currentPosition, false); + binding.setupNextButton.setOnClickListener(v -> { int currentPos = binding.viewPager.getCurrentItem(); int newPos = currentPos + 1; @@ -123,7 +135,15 @@ public class SetupActivity extends AppCompatActivity implements SetupActivityCal cancel(); }); setupActionBar(); + } + @Override + protected void onSaveInstanceState(@NonNull Bundle outState) { + super.onSaveInstanceState(outState); + if (provider != null) { + outState.putParcelable(EXTRA_PROVIDER, provider); + outState.putInt(EXTRA_CURRENT_POSITION, currentPosition); + } } private void cancel() { @@ -164,7 +184,7 @@ public class SetupActivity extends AppCompatActivity implements SetupActivityCal final Drawable upArrow = ResourcesCompat.getDrawable(getResources(), R.drawable.ic_back, getTheme()); actionBar.setHomeAsUpIndicator(upArrow); - actionBar.setDisplayHomeAsUpEnabled(ProviderObservable.getInstance().getCurrentProvider().isConfigured()); + actionBar.setDisplayHomeAsUpEnabled(currentPosition == 0 && ProviderObservable.getInstance().getCurrentProvider().isConfigured()); ViewHelper.setActivityBarColor(this, R.color.bg_setup_status_bar, R.color.bg_setup_action_bar, R.color.colorActionBarTitleFont); @ColorInt int titleColor = ContextCompat.getColor(context, R.color.colorActionBarTitleFont); actionBarTitle.setTitleTextColor(titleColor); @@ -244,7 +264,7 @@ public class SetupActivity extends AppCompatActivity implements SetupActivityCal @Override public int getCurrentPosition() { - return binding.viewPager.getCurrentItem(); + return currentPosition; } @Override diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/BaseSetupFragment.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/BaseSetupFragment.java index 0ddf8ffc..151361ba 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/BaseSetupFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/BaseSetupFragment.java @@ -1,8 +1,11 @@ package se.leap.bitmaskclient.providersetup.fragments; import android.content.Context; +import android.os.Bundle; +import android.view.View; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.fragment.app.Fragment; import androidx.viewpager2.widget.ViewPager2; @@ -10,8 +13,10 @@ import se.leap.bitmaskclient.providersetup.activities.SetupActivityCallback; public class BaseSetupFragment extends Fragment { - SetupActivityCallback setupActivityCallback; + public static String EXTRA_POSITION = "EXTRA_POSITION"; private boolean callFragmentSelected = false; + SetupActivityCallback setupActivityCallback; + private final ViewPager2.OnPageChangeCallback viewPagerCallback = new ViewPager2.OnPageChangeCallback() { @Override public void onPageSelected(int position) { @@ -23,10 +28,21 @@ public class BaseSetupFragment extends Fragment { } } }; - private final int position; + private int position; - public BaseSetupFragment(int position) { - this.position = position; + public static Bundle initBundle(int pos) { + Bundle bundle = new Bundle(); + bundle.putInt(EXTRA_POSITION, pos); + return bundle; + } + + public BaseSetupFragment() { + } + + @Override + public void onCreate(@Nullable Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + this.position = getArguments().getInt(EXTRA_POSITION); } @Override @@ -34,15 +50,26 @@ public class BaseSetupFragment extends Fragment { super.onAttach(context); if (getActivity() instanceof SetupActivityCallback) { setupActivityCallback = (SetupActivityCallback) getActivity(); - setupActivityCallback.registerOnPageChangeCallback(viewPagerCallback); - if (setupActivityCallback.getCurrentPosition() == position) { - handleCallFragmentSelected(); - } } else { throw new IllegalStateException("These setup fragments are closely coupled to SetupActivityCallback interface. Activities instantiating them are required to implement the interface"); } } + @Override + public void onSaveInstanceState(@NonNull Bundle outState) { + outState.putInt(EXTRA_POSITION, position); + super.onSaveInstanceState(outState); + } + + @Override + public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) { + super.onViewCreated(view, savedInstanceState); + setupActivityCallback.registerOnPageChangeCallback(viewPagerCallback); + if (setupActivityCallback.getCurrentPosition() == position) { + handleCallFragmentSelected(); + } + } + private void handleCallFragmentSelected() { if (!callFragmentSelected) { callFragmentSelected = true; @@ -51,11 +78,16 @@ public class BaseSetupFragment extends Fragment { } @Override - public void onDetach() { - super.onDetach(); + public void onDestroyView() { setupActivityCallback.removeOnPageChangeCallback(viewPagerCallback); - setupActivityCallback = null; callFragmentSelected = false; + super.onDestroyView(); + } + + @Override + public void onDetach() { + setupActivityCallback = null; + super.onDetach(); } public void onFragmentSelected() { diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/CircumventionSetupFragment.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/CircumventionSetupFragment.java index 4a08bc4e..11fa582b 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/CircumventionSetupFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/CircumventionSetupFragment.java @@ -14,12 +14,10 @@ import se.leap.bitmaskclient.databinding.FCircumventionSetupBinding; public class CircumventionSetupFragment extends BaseSetupFragment { - private CircumventionSetupFragment(int position) { - super(position); - } - public static CircumventionSetupFragment newInstance(int position) { - return new CircumventionSetupFragment(position); + CircumventionSetupFragment fragment = new CircumventionSetupFragment(); + fragment.setArguments(initBundle(position)); + return fragment; } @Override diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/ConfigureProviderFragment.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/ConfigureProviderFragment.java index c8e19055..01bedfa0 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/ConfigureProviderFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/ConfigureProviderFragment.java @@ -27,7 +27,6 @@ import static se.leap.bitmaskclient.tor.TorStatusObservable.getLastSnowflakeLog; import static se.leap.bitmaskclient.tor.TorStatusObservable.getLastTorLog; import android.app.Activity; -import android.content.Context; import android.content.Intent; import android.os.Bundle; import android.view.LayoutInflater; @@ -43,9 +42,7 @@ import java.util.List; import java.util.Observable; import java.util.Observer; -import se.leap.bitmaskclient.R; import se.leap.bitmaskclient.base.models.Provider; -import se.leap.bitmaskclient.base.utils.PreferenceHelper; import se.leap.bitmaskclient.databinding.FConfigureProviderBinding; import se.leap.bitmaskclient.eip.EipSetupListener; import se.leap.bitmaskclient.eip.EipSetupObserver; @@ -58,18 +55,16 @@ public class ConfigureProviderFragment extends BaseSetupFragment implements Obse private static final String TAG = ConfigureProviderFragment.class.getSimpleName(); - public static ConfigureProviderFragment newInstance(int position) { - return new ConfigureProviderFragment(position); - } - FConfigureProviderBinding binding; private boolean isExpanded = false; private boolean ignoreProviderAPIUpdates = false; private TorLogAdapter torLogAdapter; - private ConfigureProviderFragment(int position) { - super(position); + public static ConfigureProviderFragment newInstance(int position) { + ConfigureProviderFragment fragment = new ConfigureProviderFragment(); + fragment.setArguments(initBundle(position)); + return fragment; } @Override @@ -98,10 +93,21 @@ public class ConfigureProviderFragment extends BaseSetupFragment implements Obse return binding.getRoot(); } + @Override + public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) { + super.onViewCreated(view, savedInstanceState); + setupActivityCallback.registerCancelCallback(this); + TorStatusObservable.getInstance().addObserver(this); + EipSetupObserver.addListener(this); + } + @Override public void onDestroyView() { - super.onDestroyView(); + setupActivityCallback.removeCancelCallback(this); + TorStatusObservable.getInstance().deleteObserver(this); + EipSetupObserver.removeListener(this); binding = null; + super.onDestroyView(); } @Override @@ -115,22 +121,6 @@ public class ConfigureProviderFragment extends BaseSetupFragment implements Obse ProviderAPICommand.execute(getContext(), SET_UP_PROVIDER, setupActivityCallback.getSelectedProvider()); } - @Override - public void onAttach(@NonNull Context context) { - super.onAttach(context); - setupActivityCallback.registerCancelCallback(this); - TorStatusObservable.getInstance().addObserver(this); - EipSetupObserver.addListener(this); - } - - @Override - public void onDetach() { - setupActivityCallback.removeCancelCallback(this); - TorStatusObservable.getInstance().deleteObserver(this); - EipSetupObserver.removeListener(this); - super.onDetach(); - } - protected void showConnectionDetails() { binding.connectionDetailLogs.addOnScrollListener(new RecyclerView.OnScrollListener() { diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/EmptyPermissionSetupFragment.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/EmptyPermissionSetupFragment.java index 1574edff..4226d804 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/EmptyPermissionSetupFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/EmptyPermissionSetupFragment.java @@ -15,10 +15,12 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import se.leap.bitmaskclient.databinding.FEmptyPermissionSetupBinding; -import se.leap.bitmaskclient.databinding.FVpnPermissionSetupBinding; public class EmptyPermissionSetupFragment extends BaseSetupFragment { + public static String EXTRA_VPN_INTENT = "EXTRA_VPN_INTENT"; + public static String EXTRA_NOTIFICATION_PERMISSON_ACTION = "EXTRA_NOTIFICATION_PERMISSON_ACTION"; + private String notificationPermissionAction = null; private Intent vpnPermissionIntent = null; @@ -51,22 +53,28 @@ public class EmptyPermissionSetupFragment extends BaseSetupFragment { } ); - private EmptyPermissionSetupFragment(int position, String permissionAction) { - super(position); - this.notificationPermissionAction = permissionAction; - } - - private EmptyPermissionSetupFragment(int position, Intent vpnPermissionIntent) { - super(position); - this.vpnPermissionIntent = vpnPermissionIntent; - } public static EmptyPermissionSetupFragment newInstance(int position, Intent vpnPermissionIntent) { - return new EmptyPermissionSetupFragment(position, vpnPermissionIntent); + Bundle bundle = initBundle(position); + bundle.putParcelable(EXTRA_VPN_INTENT, vpnPermissionIntent); + EmptyPermissionSetupFragment fragment = new EmptyPermissionSetupFragment(); + fragment.setArguments(bundle); + return fragment; } public static EmptyPermissionSetupFragment newInstance(int position, String notificationPermissionAction) { - return new EmptyPermissionSetupFragment(position, notificationPermissionAction); + Bundle bundle = initBundle(position); + bundle.putString(EXTRA_NOTIFICATION_PERMISSON_ACTION, notificationPermissionAction); + EmptyPermissionSetupFragment fragment = new EmptyPermissionSetupFragment(); + fragment.setArguments(bundle); + return fragment; + } + + @Override + public void onCreate(@Nullable Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + this.vpnPermissionIntent = getArguments().getParcelable(EXTRA_VPN_INTENT); + this.notificationPermissionAction = getArguments().getString(EXTRA_NOTIFICATION_PERMISSON_ACTION); } @Override @@ -76,6 +84,22 @@ public class EmptyPermissionSetupFragment extends BaseSetupFragment { return binding.getRoot(); } + @Override + public void onSaveInstanceState(@NonNull Bundle outState) { + if (vpnPermissionIntent != null) { + outState.putParcelable(EXTRA_VPN_INTENT, vpnPermissionIntent); + } + if (notificationPermissionAction != null) { + outState.putString(EXTRA_NOTIFICATION_PERMISSON_ACTION, notificationPermissionAction); + } + super.onSaveInstanceState(outState); + } + + @Override + public void onViewStateRestored(@Nullable Bundle savedInstanceState) { + super.onViewStateRestored(savedInstanceState); + } + @Override public void onFragmentSelected() { super.onFragmentSelected(); diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/NotificationSetupFragment.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/NotificationSetupFragment.java index 72374bb9..a9589336 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/NotificationSetupFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/NotificationSetupFragment.java @@ -12,12 +12,10 @@ import se.leap.bitmaskclient.databinding.FNotificationSetupBinding; public class NotificationSetupFragment extends BaseSetupFragment { - private NotificationSetupFragment(int position) { - super(position); - } - public static NotificationSetupFragment newInstance(int position) { - return new NotificationSetupFragment(position); + NotificationSetupFragment fragment = new NotificationSetupFragment(); + fragment.setArguments(initBundle(position)); + return fragment; } @Override diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/ProviderSelectionFragment.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/ProviderSelectionFragment.java index ba3ff4aa..e8f37e43 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/ProviderSelectionFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/ProviderSelectionFragment.java @@ -34,14 +34,13 @@ public class ProviderSelectionFragment extends BaseSetupFragment implements Canc private FProviderSelectionBinding binding; - private ProviderSelectionFragment(int position) { - super(position); - } - public static ProviderSelectionFragment newInstance(int position) { - return new ProviderSelectionFragment(position); + ProviderSelectionFragment fragment = new ProviderSelectionFragment(); + fragment.setArguments(initBundle(position)); + return fragment; } + @Override public void onCreate(@Nullable Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -72,6 +71,20 @@ public class ProviderSelectionFragment extends BaseSetupFragment implements Canc radioButtons.add(radioButton); binding.editCustomProvider.setVisibility(viewModel.getEditProviderVisibility()); + return binding.getRoot(); + } + + @Override + public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) { + super.onViewCreated(view, savedInstanceState); + setupActivityCallback.registerCancelCallback(this); + } + + @Override + public void onFragmentSelected() { + super.onFragmentSelected(); + setupActivityCallback.setCancelButtonHidden(true); + setupActivityCallback.setNavigationButtonHidden(false); binding.providerRadioGroup.setOnCheckedChangeListener((group, checkedId) -> { viewModel.setSelected(checkedId); for (RadioButton rb : radioButtons) { @@ -86,7 +99,6 @@ public class ProviderSelectionFragment extends BaseSetupFragment implements Canc setupActivityCallback.onProviderSelected(new Provider(binding.editCustomProvider.getText().toString())); } }); - binding.providerRadioGroup.check(viewModel.getSelected()); binding.editCustomProvider.addTextChangedListener(new TextWatcher() { @Override @@ -95,9 +107,11 @@ public class ProviderSelectionFragment extends BaseSetupFragment implements Canc @Override public void onTextChanged(CharSequence s, int start, int before, int count) { viewModel.setCustomUrl(s.toString()); - setupActivityCallback.onSetupStepValidationChanged(viewModel.isValidConfig()); - if (viewModel.isValidConfig()) { - setupActivityCallback.onProviderSelected(new Provider(s.toString())); + if (viewModel.isCustomProviderSelected()) { + setupActivityCallback.onSetupStepValidationChanged(viewModel.isValidConfig()); + if (viewModel.isValidConfig()) { + setupActivityCallback.onProviderSelected(new Provider(s.toString())); + } } } @@ -110,33 +124,15 @@ public class ProviderSelectionFragment extends BaseSetupFragment implements Canc ViewHelper.hideKeyboardFrom(getContext(), v); } }); - return binding.getRoot(); - } - - @Override - public void onFragmentSelected() { - super.onFragmentSelected(); - setupActivityCallback.setCancelButtonHidden(true); - setupActivityCallback.setNavigationButtonHidden(false); - } - - @Override - public void onAttach(@NonNull Context context) { - super.onAttach(context); - setupActivityCallback.registerCancelCallback(this); - } - - @Override - public void onDetach() { - setupActivityCallback.removeCancelCallback(this); - super.onDetach(); + binding.providerRadioGroup.check(viewModel.getSelected()); } @Override public void onDestroyView() { - super.onDestroyView(); + setupActivityCallback.removeCancelCallback(this); binding = null; radioButtons = null; + super.onDestroyView(); } @Override diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/SetupSuccessFragment.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/SetupSuccessFragment.java index 448e357a..daf4ed6c 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/SetupSuccessFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/SetupSuccessFragment.java @@ -13,13 +13,10 @@ import se.leap.bitmaskclient.databinding.FSetupSuccessBinding; public class SetupSuccessFragment extends BaseSetupFragment { - - private SetupSuccessFragment(int position) { - super(position); - } - public static SetupSuccessFragment newInstance(int position) { - return new SetupSuccessFragment(position); + SetupSuccessFragment fragment = new SetupSuccessFragment(); + fragment.setArguments(initBundle(position)); + return fragment; } @Override diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/VpnPermissionSetupFragment.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/VpnPermissionSetupFragment.java index a6af0b36..188ba9ac 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/VpnPermissionSetupFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/VpnPermissionSetupFragment.java @@ -13,13 +13,10 @@ import se.leap.bitmaskclient.databinding.FVpnPermissionSetupBinding; public class VpnPermissionSetupFragment extends BaseSetupFragment { - - private VpnPermissionSetupFragment(int position) { - super(position); - } - public static VpnPermissionSetupFragment newInstance(int position) { - return new VpnPermissionSetupFragment(position); + VpnPermissionSetupFragment fragment = new VpnPermissionSetupFragment(); + fragment.setArguments(initBundle(position)); + return fragment; } @Override @@ -29,11 +26,6 @@ public class VpnPermissionSetupFragment extends BaseSetupFragment { return binding.getRoot(); } - @Override - public void onAttach(@NonNull Context context) { - super.onAttach(context); - } - @Override public void onFragmentSelected() { super.onFragmentSelected(); diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/viewmodel/ProviderSelectionViewModel.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/viewmodel/ProviderSelectionViewModel.java index d1954ffb..aa2fe7cb 100644 --- a/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/viewmodel/ProviderSelectionViewModel.java +++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/fragments/viewmodel/ProviderSelectionViewModel.java @@ -54,6 +54,10 @@ public class ProviderSelectionViewModel extends ViewModel { return true; } + public boolean isCustomProviderSelected() { + return selected == ADD_PROVIDER; + } + public CharSequence getProviderDescription(Context context) { if (selected == ADD_PROVIDER) { return context.getText(R.string.add_provider_description); -- cgit v1.2.3