From 355c487ec9f791ace6420490eddea7390d5d98fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= Date: Wed, 21 Jul 2021 11:34:31 +0200 Subject: [PATCH 1/4] Split ConditionManager into API-specific versions * On API 29+ we need the location permission to start the hotspot, while on lower API levels, we don't. In order to handle permissions and other conditions in a clear manner depending the API level of the device the app is running on, have separate extensions of the base ConditionManager class. * Take special care to handle situations gracefully where the Wifi is disabled and the user tries to start the hotspot. We cannot simply rely on Wifi being enabled as a sufficient condition that allows us to start the hotspot. We need to wait for WifiP2p to be available. While it is tricky to obtain that state (it involves registering a broadcast receiver for the WIFI_P2P_STATE_CHANGED_ACTION broadcast, keeping track of changes there and even then things are still ugly. It can happen that WifiP2p is available *before* Wifi is. Also it can happen that WifiP2p never becomes available because some other application has already opened a hotspot. Instead of checking that state, we now just try (and retry repeatedly after a delay) to start the hotspot (and the WifiP2p framework) hoping that is becomes availabe within a reasonable amount of time after Wifi has been detected to be on. Currently we try 5 times with a delay of 1 second. * Improve the behavior of disabling and re-enabling the 'start hotspot' button, so that it becomes impossible to double-tap it, but still making sure that the button get re-enabled as soon as the UI is back in a state where the user should be able to tap the button again. --- .../android/hotspot/ConditionManager.java | 166 +++--------------- .../hotspot/ConditionManager29Impl.java | 136 ++++++++++++++ .../android/hotspot/ConditionManagerImpl.java | 83 +++++++++ .../android/hotspot/HotspotIntroFragment.java | 43 +++-- .../briar/android/hotspot/HotspotManager.java | 112 ++++++++---- .../android/hotspot/HotspotViewModel.java | 2 +- .../briar/android/util/UiUtils.java | 29 +++ briar-android/src/main/res/values/strings.xml | 1 + 8 files changed, 375 insertions(+), 197 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java index c0fc3b3f0..ba9119535 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java @@ -1,171 +1,53 @@ package org.briarproject.briar.android.hotspot; -import android.content.DialogInterface.OnClickListener; -import android.content.Intent; import android.net.wifi.WifiManager; -import android.provider.Settings; -import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.briar.R; - -import androidx.activity.result.ActivityResult; -import androidx.activity.result.ActivityResultLauncher; -import androidx.annotation.Nullable; -import androidx.annotation.StringRes; -import androidx.appcompat.app.AlertDialog; +import androidx.core.util.Consumer; import androidx.fragment.app.FragmentActivity; -import static android.Manifest.permission.ACCESS_FINE_LOCATION; import static android.content.Context.WIFI_SERVICE; -import static android.os.Build.VERSION.SDK_INT; -import static androidx.core.app.ActivityCompat.shouldShowRequestPermissionRationale; -import static org.briarproject.briar.android.util.UiUtils.getGoToSettingsListener; /** - * This class ensures that the conditions to open a hotspot are fulfilled. - *

- * Be sure to call {@link #onRequestPermissionResult(Boolean)} and - * {@link #onRequestWifiEnabledResult()} when you get the - * {@link ActivityResult}. - *

- * As soon as {@link #checkAndRequestConditions()} returns true, - * all conditions are fulfilled. + * Abstract base class for the ConditionManagers that ensure that the conditions + * to open a hotspot are fulfilled. There are different extensions of this for + * API levels lower than 29 and 29+. */ -@NotNullByDefault -class ConditionManager { +abstract class ConditionManager { - private enum Permission { + enum Permission { UNKNOWN, GRANTED, SHOW_RATIONALE, PERMANENTLY_DENIED } - private Permission locationPermission = Permission.UNKNOWN; - private Permission wifiSetting = Permission.SHOW_RATIONALE; + protected final Consumer permissionUpdateCallback; + protected FragmentActivity ctx; + protected WifiManager wifiManager; - private final FragmentActivity ctx; - private final WifiManager wifiManager; - private final ActivityResultLauncher locationRequest; - private final ActivityResultLauncher wifiRequest; + ConditionManager(Consumer permissionUpdateCallback) { + this.permissionUpdateCallback = permissionUpdateCallback; + } - ConditionManager(FragmentActivity ctx, - ActivityResultLauncher locationRequest, - ActivityResultLauncher wifiRequest) { + /** + * Pass a FragmentActivity context here during `onCreateView()`. + */ + void init(FragmentActivity ctx) { this.ctx = ctx; this.wifiManager = (WifiManager) ctx.getApplicationContext() .getSystemService(WIFI_SERVICE); - this.locationRequest = locationRequest; - this.wifiRequest = wifiRequest; } /** - * Call this to reset state when UI starts, - * because state might have changed. + * Call this during onStart() in the fragment where the ConditionManager + * is used. */ - void resetPermissions() { - locationPermission = Permission.UNKNOWN; - wifiSetting = Permission.SHOW_RATIONALE; - } - - /** - * This makes a request for location permission. - * If {@link #checkAndRequestConditions()} returns true, you can continue. - */ - void startConditionChecks() { - locationRequest.launch(ACCESS_FINE_LOCATION); - } + abstract void onStart(); /** + * Check if all required conditions are met such that the hotspot can be + * started. If any precondition is not met yet, bring up relevant dialogs + * asking the user to grant relevant permissions or take relevant actions. + * * @return true if conditions are fulfilled and flow can continue. */ - boolean checkAndRequestConditions() { - if (areEssentialPermissionsGranted()) return true; - - // If an essential permission has been permanently denied, ask the - // user to change the setting - if (locationPermission == Permission.PERMANENTLY_DENIED) { - showDenialDialog(R.string.permission_location_title, - R.string.permission_hotspot_location_denied_body, - getGoToSettingsListener(ctx)); - return false; - } - if (wifiSetting == Permission.PERMANENTLY_DENIED) { - showDenialDialog(R.string.wifi_settings_title, - R.string.wifi_settings_request_denied_body, - (d, w) -> requestEnableWiFi()); - return false; - } - - // Should we show the rationale for location permission or Wi-Fi? - if (locationPermission == Permission.SHOW_RATIONALE) { - showRationale(R.string.permission_location_title, - R.string.permission_hotspot_location_request_body, - this::requestPermissions); - } else if (wifiSetting == Permission.SHOW_RATIONALE) { - showRationale(R.string.wifi_settings_title, - R.string.wifi_settings_request_enable_body, - this::requestEnableWiFi); - } - return false; - } - - void onRequestPermissionResult(@Nullable Boolean granted) { - if (granted != null && granted) { - locationPermission = Permission.GRANTED; - } else if (shouldShowRequestPermissionRationale(ctx, - ACCESS_FINE_LOCATION)) { - locationPermission = Permission.SHOW_RATIONALE; - } else { - locationPermission = Permission.PERMANENTLY_DENIED; - } - } - - void onRequestWifiEnabledResult() { - wifiSetting = wifiManager.isWifiEnabled() ? Permission.GRANTED : - Permission.PERMANENTLY_DENIED; - } - - private boolean areEssentialPermissionsGranted() { - if (SDK_INT < 29) { - if (!wifiManager.isWifiEnabled()) { - //noinspection deprecation - return wifiManager.setWifiEnabled(true); - } - return true; - } else { - return locationPermission == Permission.GRANTED - && wifiManager.isWifiEnabled(); - } - } - - private void showDenialDialog(@StringRes int title, @StringRes int body, - OnClickListener onOkClicked) { - AlertDialog.Builder builder = new AlertDialog.Builder(ctx); - builder.setTitle(title); - builder.setMessage(body); - builder.setPositiveButton(R.string.ok, onOkClicked); - builder.setNegativeButton(R.string.cancel, - (dialog, which) -> ctx.supportFinishAfterTransition()); - builder.show(); - } - - private void showRationale(@StringRes int title, @StringRes int body, - Runnable onContinueClicked) { - AlertDialog.Builder builder = new AlertDialog.Builder(ctx); - builder.setTitle(title); - builder.setMessage(body); - builder.setNeutralButton(R.string.continue_button, - (dialog, which) -> onContinueClicked.run()); - builder.show(); - } - - private void requestPermissions() { - locationRequest.launch(ACCESS_FINE_LOCATION); - } - - private void requestEnableWiFi() { - Intent i = SDK_INT < 29 ? - new Intent(Settings.ACTION_WIFI_SETTINGS) : - new Intent(Settings.Panel.ACTION_WIFI); - wifiRequest.launch(i); - } + abstract boolean checkAndRequestConditions(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java new file mode 100644 index 000000000..94d79f382 --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java @@ -0,0 +1,136 @@ +package org.briarproject.briar.android.hotspot; + +import android.content.Intent; +import android.provider.Settings; + +import org.briarproject.briar.R; + +import java.util.logging.Logger; + +import androidx.activity.result.ActivityResultCaller; +import androidx.activity.result.ActivityResultLauncher; +import androidx.activity.result.contract.ActivityResultContracts.RequestPermission; +import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult; +import androidx.annotation.Nullable; +import androidx.annotation.RequiresApi; +import androidx.core.util.Consumer; + +import static android.Manifest.permission.ACCESS_FINE_LOCATION; +import static androidx.core.app.ActivityCompat.shouldShowRequestPermissionRationale; +import static java.util.logging.Level.INFO; +import static java.util.logging.Logger.getLogger; +import static org.briarproject.briar.android.util.UiUtils.getGoToSettingsListener; +import static org.briarproject.briar.android.util.UiUtils.showDenialDialog; +import static org.briarproject.briar.android.util.UiUtils.showRationale; + +/** + * This class ensures that the conditions to open a hotspot are fulfilled on + * API levels >= 29. + *

+ * As soon as {@link #checkAndRequestConditions()} returns true, + * all conditions are fulfilled. + */ +@RequiresApi(29) +class ConditionManager29Impl extends ConditionManager { + + private static final Logger LOG = + getLogger(ConditionManager29Impl.class.getName()); + + private Permission locationPermission = Permission.UNKNOWN; + + private final ActivityResultLauncher locationRequest; + private final ActivityResultLauncher wifiRequest; + + ConditionManager29Impl(ActivityResultCaller arc, + Consumer permissionUpdateCallback) { + super(permissionUpdateCallback); + locationRequest = arc.registerForActivityResult( + new RequestPermission(), granted -> { + onRequestPermissionResult(granted); + permissionUpdateCallback.accept(true); + }); + wifiRequest = arc.registerForActivityResult( + new StartActivityForResult(), + result -> permissionUpdateCallback.accept(true)); + } + + @Override + void onStart() { + locationPermission = Permission.UNKNOWN; + } + + private boolean areEssentialPermissionsGranted() { + if (LOG.isLoggable(INFO)) { + LOG.info(String.format("areEssentialPermissionsGranted(): " + + "locationPermission? %s, " + + "wifiManager.isWifiEnabled()? %b", + locationPermission, + wifiManager.isWifiEnabled())); + } + return locationPermission == Permission.GRANTED && + wifiManager.isWifiEnabled(); + } + + @Override + boolean checkAndRequestConditions() { + if (areEssentialPermissionsGranted()) return true; + + if (locationPermission == Permission.UNKNOWN) { + locationRequest.launch(ACCESS_FINE_LOCATION); + return false; + } + + // If the location permission has been permanently denied, ask the + // user to change the setting + if (locationPermission == Permission.PERMANENTLY_DENIED) { + showDenialDialog(ctx, R.string.permission_location_title, + R.string.permission_hotspot_location_denied_body, + getGoToSettingsListener(ctx), + () -> permissionUpdateCallback.accept(false)); + return false; + } + + // Should we show the rationale for location permission? + if (locationPermission == Permission.SHOW_RATIONALE) { + showRationale(ctx, R.string.permission_location_title, + R.string.permission_hotspot_location_request_body, + this::requestPermissions, + () -> permissionUpdateCallback.accept(false)); + return false; + } + + // If Wifi is not enabled, we show the rationale for enabling Wifi? + if (!wifiManager.isWifiEnabled()) { + showRationale(ctx, R.string.wifi_settings_title, + R.string.wifi_settings_request_enable_body, + this::requestEnableWiFi, + () -> permissionUpdateCallback.accept(false)); + return false; + } + + // we shouldn't usually reach this point, but if we do, return false + // anyway to force a recheck. Maybe some condition changed in the + // meantime. + return false; + } + + private void onRequestPermissionResult(@Nullable Boolean granted) { + if (granted != null && granted) { + locationPermission = Permission.GRANTED; + } else if (shouldShowRequestPermissionRationale(ctx, + ACCESS_FINE_LOCATION)) { + locationPermission = Permission.SHOW_RATIONALE; + } else { + locationPermission = Permission.PERMANENTLY_DENIED; + } + } + + private void requestPermissions() { + locationRequest.launch(ACCESS_FINE_LOCATION); + } + + private void requestEnableWiFi() { + wifiRequest.launch(new Intent(Settings.Panel.ACTION_WIFI)); + } + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java new file mode 100644 index 000000000..456f89ae3 --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java @@ -0,0 +1,83 @@ +package org.briarproject.briar.android.hotspot; + +import android.content.Intent; +import android.provider.Settings; + +import org.briarproject.briar.R; + +import java.util.logging.Logger; + +import androidx.activity.result.ActivityResultCaller; +import androidx.activity.result.ActivityResultLauncher; +import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult; +import androidx.core.util.Consumer; + +import static java.util.logging.Level.INFO; +import static java.util.logging.Logger.getLogger; +import static org.briarproject.briar.android.util.UiUtils.showRationale; + +/** + * This class ensures that the conditions to open a hotspot are fulfilled on + * API levels < 29. + *

+ * As soon as {@link #checkAndRequestConditions()} returns true, + * all conditions are fulfilled. + */ +class ConditionManagerImpl extends ConditionManager { + + private static final Logger LOG = + getLogger(ConditionManagerImpl.class.getName()); + + private final ActivityResultLauncher wifiRequest; + + ConditionManagerImpl(ActivityResultCaller arc, + Consumer permissionUpdateCallback) { + super(permissionUpdateCallback); + wifiRequest = arc.registerForActivityResult( + new StartActivityForResult(), + result -> permissionUpdateCallback.accept(true)); + } + + @Override + void onStart() { + // nothing to do here + } + + private boolean areEssentialPermissionsGranted() { + if (LOG.isLoggable(INFO)) { + LOG.info(String.format("areEssentialPermissionsGranted(): " + + "wifiManager.isWifiEnabled()? %b", + wifiManager.isWifiEnabled())); + } + return wifiManager.isWifiEnabled(); + } + + @Override + boolean checkAndRequestConditions() { + if (areEssentialPermissionsGranted()) return true; + + if (!wifiManager.isWifiEnabled()) { + // Try enabling the Wifi and return true if that seems to have been + // successful, i.e. "Wifi is either already in the requested state, or + // in progress toward the requested state". + if (wifiManager.setWifiEnabled(true)) { + LOG.info("Enabled wifi"); + return true; + } + + // Wifi is not enabled and we can't seem to enable it, so ask the user + // to enable it for us. + showRationale(ctx, R.string.wifi_settings_title, + R.string.wifi_settings_request_enable_body, + this::requestEnableWiFi, + () -> permissionUpdateCallback.accept(false)); + } + + return false; + } + + private void requestEnableWiFi() { + wifiRequest.launch(new Intent(Settings.ACTION_WIFI_SETTINGS)); + } + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java index 7979cb956..e5f9e6b50 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.hotspot; import android.content.Context; -import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.os.Bundle; @@ -20,15 +19,13 @@ import org.briarproject.briar.R; import javax.inject.Inject; -import androidx.activity.result.ActivityResultLauncher; -import androidx.activity.result.contract.ActivityResultContracts.RequestPermission; -import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult; import androidx.annotation.Nullable; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentActivity; import androidx.lifecycle.ViewModelProvider; import static android.content.pm.ApplicationInfo.FLAG_TEST_ONLY; +import static android.os.Build.VERSION.SDK_INT; import static android.view.View.INVISIBLE; import static android.view.View.VISIBLE; import static androidx.transition.TransitionManager.beginDelayedTransition; @@ -45,22 +42,14 @@ public class HotspotIntroFragment extends Fragment { ViewModelProvider.Factory viewModelFactory; private HotspotViewModel viewModel; - private ConditionManager conditionManager; private Button startButton; private ProgressBar progressBar; private TextView progressTextView; - private final ActivityResultLauncher locationRequest = - registerForActivityResult(new RequestPermission(), granted -> { - conditionManager.onRequestPermissionResult(granted); - startHotspot(); - }); - private final ActivityResultLauncher wifiRequest = - registerForActivityResult(new StartActivityForResult(), result -> { - conditionManager.onRequestWifiEnabledResult(); - startHotspot(); - }); + private final ConditionManager conditionManager = SDK_INT < 29 ? + new ConditionManagerImpl(this, this::onPermissionUpdate) : + new ConditionManager29Impl(this, this::onPermissionUpdate); @Override public void onAttach(Context context) { @@ -69,8 +58,6 @@ public class HotspotIntroFragment extends Fragment { getAndroidComponent(activity).inject(this); viewModel = new ViewModelProvider(activity, viewModelFactory) .get(HotspotViewModel.class); - conditionManager = - new ConditionManager(activity, locationRequest, wifiRequest); } @Override @@ -84,10 +71,9 @@ public class HotspotIntroFragment extends Fragment { progressBar = v.findViewById(R.id.progressBar); progressTextView = v.findViewById(R.id.progressTextView); - startButton.setOnClickListener(button -> { - startButton.setEnabled(false); - conditionManager.startConditionChecks(); - }); + startButton.setOnClickListener(this::onButtonClick); + + conditionManager.init(requireActivity()); return v; } @@ -95,11 +81,15 @@ public class HotspotIntroFragment extends Fragment { @Override public void onStart() { super.onStart(); - conditionManager.resetPermissions(); + conditionManager.onStart(); + } + + private void onButtonClick(View view) { + startButton.setEnabled(false); + startHotspot(); } private void startHotspot() { - startButton.setEnabled(true); if (conditionManager.checkAndRequestConditions()) { showInstallWarningIfNeeded(); beginDelayedTransition((ViewGroup) requireView()); @@ -110,6 +100,13 @@ public class HotspotIntroFragment extends Fragment { } } + private void onPermissionUpdate(boolean recheckPermissions) { + startButton.setEnabled(true); + if (recheckPermissions) { + startHotspot(); + } + } + private void showInstallWarningIfNeeded() { Context ctx = requireContext(); ApplicationInfo applicationInfo; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java index a27cd0851..4ecf5d911 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java @@ -54,7 +54,7 @@ import static org.briarproject.briar.android.util.UiUtils.handleException; @MethodsNotNullByDefault @ParametersNotNullByDefault -class HotspotManager implements ActionListener { +class HotspotManager { interface HotspotListener { void onStartingHotspot(); @@ -72,6 +72,7 @@ class HotspotManager implements ActionListener { private static final Logger LOG = getLogger(HotspotManager.class.getName()); + private static final int MAX_FRAMEWORK_ATTEMPTS = 5; private static final int MAX_GROUP_INFO_ATTEMPTS = 5; private static final int RETRY_DELAY_MILLIS = 1000; private static final String HOTSPOT_NAMESPACE = "hotspot"; @@ -133,12 +134,80 @@ class HotspotManager implements ActionListener { return; } listener.onStartingHotspot(); + acquireLocks(); + startWifiP2pFramework(1); + } + + /** + * As soon as Wifi is enabled, we try starting the WifiP2p framework. + * If Wifi has just been enabled, it is possible that will fail. If that + * happens we try again for MAX_FRAMEWORK_ATTEMPTS times after a delay of + * RETRY_DELAY_MILLIS after each attempt. + *

+ * Rationale: it can take a few milliseconds for WifiP2p to become available + * after enabling Wifi. Depending on the API level it is possible to check this + * using {@link WifiP2pManager#requestP2pState} or register a BroadcastReceiver + * on the WIFI_P2P_STATE_CHANGED_ACTION to get notified when WifiP2p is really + * available. Trying to implement a solution that works reliably using these + * checks turned out to be a long rabbit-hole with lots of corner cases and + * workarounds for specific situations. + * Instead we now rely on this trial-and-error approach of just starting + * the framework and retrying if it fails. + *

+ * We'll realize that the framework is busy when the ActionListener passed + * to {@link WifiP2pManager#createGroup} is called with onFailure(BUSY) + */ + void startWifiP2pFramework(int attempt) { + if (LOG.isLoggable(INFO)) + LOG.info("startWifiP2pFramework attempt: " + attempt); + /* + * It is important that we call WifiP2pManager#initialize again + * for every attempt to starting the framework because otherwise, + * createGroup() will continue to fail with a BUSY state. + */ channel = wifiP2pManager.initialize(ctx, ctx.getMainLooper(), null); if (channel == null) { listener.onHotspotError( ctx.getString(R.string.hotspot_error_no_wifi_direct)); return; } + + ActionListener listener = new ActionListener() { + + @Override + // Callback for wifiP2pManager#createGroup() during startWifiP2pHotspot() + public void onSuccess() { + requestGroupInfo(1); + } + + @Override + // Callback for wifiP2pManager#createGroup() during startWifiP2pHotspot() + public void onFailure(int reason) { + LOG.info("onFailure: " + reason); + if (reason == BUSY) { + // WifiP2p not ready yet or hotspot already running + restartWifiP2pFramework(attempt); + } else if (reason == P2P_UNSUPPORTED) { + releaseHotspotWithError(ctx.getString( + R.string.hotspot_error_start_callback_failed, + "p2p unsupported")); + } else if (reason == ERROR) { + releaseHotspotWithError(ctx.getString( + R.string.hotspot_error_start_callback_failed, + "p2p error")); + } else if (reason == NO_SERVICE_REQUESTS) { + releaseHotspotWithError(ctx.getString( + R.string.hotspot_error_start_callback_failed, + "no service requests")); + } else { + // all cases covered, in doubt set to error + releaseHotspotWithError(ctx.getString( + R.string.hotspot_error_start_callback_failed_unknown, + reason)); + } + } + }; + try { if (SDK_INT >= 29) { dbExecutor.execute(() -> { @@ -151,12 +220,12 @@ class HotspotManager implements ActionListener { .setPassphrase(savedNetworkConfig.password) .build(); acquireLocks(); - wifiP2pManager.createGroup(channel, config, this); + wifiP2pManager.createGroup(channel, config, listener); }); }); } else { acquireLocks(); - wifiP2pManager.createGroup(channel, this); + wifiP2pManager.createGroup(channel, listener); } } catch (SecurityException e) { // this should never happen, because we request permissions before @@ -164,37 +233,18 @@ class HotspotManager implements ActionListener { } } - @Override - // Callback for wifiP2pManager#createGroup() during startWifiP2pHotspot() - public void onSuccess() { - requestGroupInfo(1); - } - - @Override - // Callback for wifiP2pManager#createGroup() during startWifiP2pHotspot() - public void onFailure(int reason) { - if (reason == BUSY) { - // Hotspot already running - requestGroupInfo(1); - } else if (reason == P2P_UNSUPPORTED) { - releaseHotspotWithError(ctx.getString( - R.string.hotspot_error_start_callback_failed, - "p2p unsupported")); - } else if (reason == ERROR) { - releaseHotspotWithError(ctx.getString( - R.string.hotspot_error_start_callback_failed, "p2p error")); - } else if (reason == NO_SERVICE_REQUESTS) { - releaseHotspotWithError(ctx.getString( - R.string.hotspot_error_start_callback_failed, - "no service requests")); + private void restartWifiP2pFramework(int attempt) { + LOG.info("retrying to start WifiP2p framework"); + if (attempt < MAX_FRAMEWORK_ATTEMPTS) { + handler.postDelayed(() -> startWifiP2pFramework(attempt + 1), + RETRY_DELAY_MILLIS); } else { - // all cases covered, in doubt set to error - releaseHotspotWithError(ctx.getString( - R.string.hotspot_error_start_callback_failed_unknown, - reason)); + releaseHotspotWithError( + ctx.getString(R.string.hotspot_error_framework_busy)); } } + @UiThread void stopWifiP2pHotspot() { if (channel == null) return; wifiP2pManager.removeGroup(channel, new ActionListener() { @@ -301,7 +351,7 @@ class HotspotManager implements ActionListener { } private void retryRequestingGroupInfo(int attempt) { - LOG.info("retrying"); + LOG.info("retrying to request group info"); // On some devices we need to wait for the group info to become available if (attempt < MAX_GROUP_INFO_ATTEMPTS) { handler.postDelayed(() -> requestGroupInfo(attempt + 1), diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java index 7b923e1e0..5bf44eaa7 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java @@ -110,7 +110,7 @@ class HotspotViewModel extends DbViewModel } @UiThread - private void stopHotspot() { + void stopHotspot() { ioExecutor.execute(webServerManager::stopWebServer); hotspotManager.stopWifiP2pHotspot(); notificationManager.clearHotspotNotification(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java index 2146bffd1..22486c4fe 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java @@ -6,6 +6,7 @@ import android.app.Activity; import android.app.KeyguardManager; import android.content.ActivityNotFoundException; import android.content.Context; +import android.content.DialogInterface; import android.content.DialogInterface.OnClickListener; import android.content.Intent; import android.content.res.Resources; @@ -52,6 +53,7 @@ import androidx.annotation.ColorRes; import androidx.annotation.DrawableRes; import androidx.annotation.Nullable; import androidx.annotation.RequiresApi; +import androidx.annotation.StringRes; import androidx.annotation.UiThread; import androidx.appcompat.app.AlertDialog; import androidx.core.content.ContextCompat; @@ -561,4 +563,31 @@ public class UiUtils { activity.getWindow().setSoftInputMode(SOFT_INPUT_ADJUST_RESIZE | SOFT_INPUT_STATE_HIDDEN); } + + public static void showDenialDialog(FragmentActivity ctx, + @StringRes int title, + @StringRes int body, DialogInterface.OnClickListener onOkClicked, + Runnable onDismiss) { + AlertDialog.Builder builder = new AlertDialog.Builder(ctx); + builder.setTitle(title); + builder.setMessage(body); + builder.setPositiveButton(R.string.ok, onOkClicked); + builder.setNegativeButton(R.string.cancel, + (dialog, which) -> ctx.supportFinishAfterTransition()); + builder.setOnDismissListener(dialog -> onDismiss.run()); + builder.show(); + } + + public static void showRationale(Context ctx, @StringRes int title, + @StringRes int body, + Runnable onContinueClicked, Runnable onDismiss) { + AlertDialog.Builder builder = new AlertDialog.Builder(ctx); + builder.setTitle(title); + builder.setMessage(body); + builder.setNeutralButton(R.string.continue_button, + (dialog, which) -> onContinueClicked.run()); + builder.setOnDismissListener(dialog -> onDismiss.run()); + builder.show(); + } + } diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index c4198032d..4fbebca60 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -757,6 +757,7 @@ Error starting web server! Error presenting website.\n\nPlease send feedback (with anonymous data) via the Briar app if the issue persists. Warning: This app was installed with Android Studio and can NOT be installed on another device. + Unable to start the hotspot. If you have another hotspot running or are sharing your internet connection via Wifi, try stopping that and try again afterwards. From 46e391645ce404e9319cb797169fc3d0352f8c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= Date: Wed, 21 Jul 2021 13:19:26 +0200 Subject: [PATCH 2/4] Reduce visibility of a field and two methods --- .../briarproject/briar/android/hotspot/ConditionManager.java | 2 +- .../org/briarproject/briar/android/hotspot/HotspotManager.java | 2 +- .../briarproject/briar/android/hotspot/HotspotViewModel.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java index ba9119535..746bc8a21 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java @@ -20,7 +20,7 @@ abstract class ConditionManager { protected final Consumer permissionUpdateCallback; protected FragmentActivity ctx; - protected WifiManager wifiManager; + WifiManager wifiManager; ConditionManager(Consumer permissionUpdateCallback) { this.permissionUpdateCallback = permissionUpdateCallback; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java index 4ecf5d911..377a91fe3 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotManager.java @@ -157,7 +157,7 @@ class HotspotManager { * We'll realize that the framework is busy when the ActionListener passed * to {@link WifiP2pManager#createGroup} is called with onFailure(BUSY) */ - void startWifiP2pFramework(int attempt) { + private void startWifiP2pFramework(int attempt) { if (LOG.isLoggable(INFO)) LOG.info("startWifiP2pFramework attempt: " + attempt); /* diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java index 5bf44eaa7..7b923e1e0 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotViewModel.java @@ -110,7 +110,7 @@ class HotspotViewModel extends DbViewModel } @UiThread - void stopHotspot() { + private void stopHotspot() { ioExecutor.execute(webServerManager::stopWebServer); hotspotManager.stopWifiP2pHotspot(); notificationManager.clearHotspotNotification(); From 93eadb88f365454f901451240b2b124dc50f6f05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= Date: Wed, 21 Jul 2021 16:02:49 +0200 Subject: [PATCH 3/4] Apply review feedback --- .../android/hotspot/ConditionManager.java | 31 +++++++++++++++++++ .../hotspot/ConditionManager29Impl.java | 9 +++--- .../android/hotspot/ConditionManagerImpl.java | 4 +-- .../android/hotspot/HotspotIntroFragment.java | 6 ++-- .../briar/android/util/UiUtils.java | 28 ----------------- briar-android/src/main/res/values/strings.xml | 2 +- 6 files changed, 42 insertions(+), 38 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java index 746bc8a21..050bb8a01 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java @@ -1,7 +1,13 @@ package org.briarproject.briar.android.hotspot; +import android.content.Context; +import android.content.DialogInterface; import android.net.wifi.WifiManager; +import org.briarproject.briar.R; + +import androidx.annotation.StringRes; +import androidx.appcompat.app.AlertDialog; import androidx.core.util.Consumer; import androidx.fragment.app.FragmentActivity; @@ -50,4 +56,29 @@ abstract class ConditionManager { */ abstract boolean checkAndRequestConditions(); + void showDenialDialog(FragmentActivity ctx, + @StringRes int title, @StringRes int body, + DialogInterface.OnClickListener onOkClicked, Runnable onDismiss) { + AlertDialog.Builder builder = new AlertDialog.Builder(ctx); + builder.setTitle(title); + builder.setMessage(body); + builder.setPositiveButton(R.string.ok, onOkClicked); + builder.setNegativeButton(R.string.cancel, + (dialog, which) -> ctx.supportFinishAfterTransition()); + builder.setOnDismissListener(dialog -> onDismiss.run()); + builder.show(); + } + + void showRationale(Context ctx, @StringRes int title, + @StringRes int body, Runnable onContinueClicked, + Runnable onDismiss) { + AlertDialog.Builder builder = new AlertDialog.Builder(ctx); + builder.setTitle(title); + builder.setMessage(body); + builder.setNeutralButton(R.string.continue_button, + (dialog, which) -> onContinueClicked.run()); + builder.setOnDismissListener(dialog -> onDismiss.run()); + builder.show(); + } + } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java index 94d79f382..464fa4c77 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java @@ -17,11 +17,10 @@ import androidx.core.util.Consumer; import static android.Manifest.permission.ACCESS_FINE_LOCATION; import static androidx.core.app.ActivityCompat.shouldShowRequestPermissionRationale; +import static java.lang.Boolean.TRUE; import static java.util.logging.Level.INFO; import static java.util.logging.Logger.getLogger; import static org.briarproject.briar.android.util.UiUtils.getGoToSettingsListener; -import static org.briarproject.briar.android.util.UiUtils.showDenialDialog; -import static org.briarproject.briar.android.util.UiUtils.showRationale; /** * This class ensures that the conditions to open a hotspot are fulfilled on @@ -47,11 +46,13 @@ class ConditionManager29Impl extends ConditionManager { locationRequest = arc.registerForActivityResult( new RequestPermission(), granted -> { onRequestPermissionResult(granted); - permissionUpdateCallback.accept(true); + permissionUpdateCallback.accept(TRUE.equals(granted)); }); wifiRequest = arc.registerForActivityResult( new StartActivityForResult(), - result -> permissionUpdateCallback.accept(true)); + result -> permissionUpdateCallback + .accept(wifiManager.isWifiEnabled()) + ); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java index 456f89ae3..5d7ff1079 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java @@ -14,7 +14,6 @@ import androidx.core.util.Consumer; import static java.util.logging.Level.INFO; import static java.util.logging.Logger.getLogger; -import static org.briarproject.briar.android.util.UiUtils.showRationale; /** * This class ensures that the conditions to open a hotspot are fulfilled on @@ -35,7 +34,8 @@ class ConditionManagerImpl extends ConditionManager { super(permissionUpdateCallback); wifiRequest = arc.registerForActivityResult( new StartActivityForResult(), - result -> permissionUpdateCallback.accept(true)); + result -> permissionUpdateCallback + .accept(wifiManager.isWifiEnabled())); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java index e5f9e6b50..716ea4f97 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java @@ -86,10 +86,10 @@ public class HotspotIntroFragment extends Fragment { private void onButtonClick(View view) { startButton.setEnabled(false); - startHotspot(); + startHotspotIfConditionsFulfilled(); } - private void startHotspot() { + private void startHotspotIfConditionsFulfilled() { if (conditionManager.checkAndRequestConditions()) { showInstallWarningIfNeeded(); beginDelayedTransition((ViewGroup) requireView()); @@ -103,7 +103,7 @@ public class HotspotIntroFragment extends Fragment { private void onPermissionUpdate(boolean recheckPermissions) { startButton.setEnabled(true); if (recheckPermissions) { - startHotspot(); + startHotspotIfConditionsFulfilled(); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java index 22486c4fe..ae1e43131 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java @@ -6,7 +6,6 @@ import android.app.Activity; import android.app.KeyguardManager; import android.content.ActivityNotFoundException; import android.content.Context; -import android.content.DialogInterface; import android.content.DialogInterface.OnClickListener; import android.content.Intent; import android.content.res.Resources; @@ -53,7 +52,6 @@ import androidx.annotation.ColorRes; import androidx.annotation.DrawableRes; import androidx.annotation.Nullable; import androidx.annotation.RequiresApi; -import androidx.annotation.StringRes; import androidx.annotation.UiThread; import androidx.appcompat.app.AlertDialog; import androidx.core.content.ContextCompat; @@ -564,30 +562,4 @@ public class UiUtils { SOFT_INPUT_STATE_HIDDEN); } - public static void showDenialDialog(FragmentActivity ctx, - @StringRes int title, - @StringRes int body, DialogInterface.OnClickListener onOkClicked, - Runnable onDismiss) { - AlertDialog.Builder builder = new AlertDialog.Builder(ctx); - builder.setTitle(title); - builder.setMessage(body); - builder.setPositiveButton(R.string.ok, onOkClicked); - builder.setNegativeButton(R.string.cancel, - (dialog, which) -> ctx.supportFinishAfterTransition()); - builder.setOnDismissListener(dialog -> onDismiss.run()); - builder.show(); - } - - public static void showRationale(Context ctx, @StringRes int title, - @StringRes int body, - Runnable onContinueClicked, Runnable onDismiss) { - AlertDialog.Builder builder = new AlertDialog.Builder(ctx); - builder.setTitle(title); - builder.setMessage(body); - builder.setNeutralButton(R.string.continue_button, - (dialog, which) -> onContinueClicked.run()); - builder.setOnDismissListener(dialog -> onDismiss.run()); - builder.show(); - } - } diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 4fbebca60..421780a9e 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -757,7 +757,7 @@ Error starting web server! Error presenting website.\n\nPlease send feedback (with anonymous data) via the Briar app if the issue persists. Warning: This app was installed with Android Studio and can NOT be installed on another device. - Unable to start the hotspot. If you have another hotspot running or are sharing your internet connection via Wifi, try stopping that and try again afterwards. + Unable to start the hotspot.\n\nIf you have another hotspot running or are sharing your internet connection via Wi-Fi, try stopping that and try again afterwards. From 6337b8626663bd3980be097b5cf38f69bcf92ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= Date: Wed, 21 Jul 2021 16:29:23 +0200 Subject: [PATCH 4/4] Rename ConditionManager classes --- .../hotspot/AbstractConditionManager.java | 84 ++++++++++++ .../android/hotspot/ConditionManager.java | 121 +++++++++--------- ...ger29Impl.java => ConditionManager29.java} | 6 +- .../android/hotspot/ConditionManagerImpl.java | 83 ------------ .../android/hotspot/HotspotIntroFragment.java | 6 +- 5 files changed, 150 insertions(+), 150 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/hotspot/AbstractConditionManager.java rename briar-android/src/main/java/org/briarproject/briar/android/hotspot/{ConditionManager29Impl.java => ConditionManager29.java} (96%) delete mode 100644 briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/AbstractConditionManager.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/AbstractConditionManager.java new file mode 100644 index 000000000..855fd845d --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/AbstractConditionManager.java @@ -0,0 +1,84 @@ +package org.briarproject.briar.android.hotspot; + +import android.content.Context; +import android.content.DialogInterface; +import android.net.wifi.WifiManager; + +import org.briarproject.briar.R; + +import androidx.annotation.StringRes; +import androidx.appcompat.app.AlertDialog; +import androidx.core.util.Consumer; +import androidx.fragment.app.FragmentActivity; + +import static android.content.Context.WIFI_SERVICE; + +/** + * Abstract base class for the ConditionManagers that ensure that the conditions + * to open a hotspot are fulfilled. There are different extensions of this for + * API levels lower than 29 and 29+. + */ +abstract class AbstractConditionManager { + + enum Permission { + UNKNOWN, GRANTED, SHOW_RATIONALE, PERMANENTLY_DENIED + } + + protected final Consumer permissionUpdateCallback; + protected FragmentActivity ctx; + WifiManager wifiManager; + + AbstractConditionManager(Consumer permissionUpdateCallback) { + this.permissionUpdateCallback = permissionUpdateCallback; + } + + /** + * Pass a FragmentActivity context here during `onCreateView()`. + */ + void init(FragmentActivity ctx) { + this.ctx = ctx; + this.wifiManager = (WifiManager) ctx.getApplicationContext() + .getSystemService(WIFI_SERVICE); + } + + /** + * Call this during onStart() in the fragment where the ConditionManager + * is used. + */ + abstract void onStart(); + + /** + * Check if all required conditions are met such that the hotspot can be + * started. If any precondition is not met yet, bring up relevant dialogs + * asking the user to grant relevant permissions or take relevant actions. + * + * @return true if conditions are fulfilled and flow can continue. + */ + abstract boolean checkAndRequestConditions(); + + void showDenialDialog(FragmentActivity ctx, + @StringRes int title, @StringRes int body, + DialogInterface.OnClickListener onOkClicked, Runnable onDismiss) { + AlertDialog.Builder builder = new AlertDialog.Builder(ctx); + builder.setTitle(title); + builder.setMessage(body); + builder.setPositiveButton(R.string.ok, onOkClicked); + builder.setNegativeButton(R.string.cancel, + (dialog, which) -> ctx.supportFinishAfterTransition()); + builder.setOnDismissListener(dialog -> onDismiss.run()); + builder.show(); + } + + void showRationale(Context ctx, @StringRes int title, + @StringRes int body, Runnable onContinueClicked, + Runnable onDismiss) { + AlertDialog.Builder builder = new AlertDialog.Builder(ctx); + builder.setTitle(title); + builder.setMessage(body); + builder.setNeutralButton(R.string.continue_button, + (dialog, which) -> onContinueClicked.run()); + builder.setOnDismissListener(dialog -> onDismiss.run()); + builder.show(); + } + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java index 050bb8a01..95c0d57fe 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager.java @@ -1,84 +1,83 @@ package org.briarproject.briar.android.hotspot; -import android.content.Context; -import android.content.DialogInterface; -import android.net.wifi.WifiManager; +import android.content.Intent; +import android.provider.Settings; import org.briarproject.briar.R; -import androidx.annotation.StringRes; -import androidx.appcompat.app.AlertDialog; -import androidx.core.util.Consumer; -import androidx.fragment.app.FragmentActivity; +import java.util.logging.Logger; -import static android.content.Context.WIFI_SERVICE; +import androidx.activity.result.ActivityResultCaller; +import androidx.activity.result.ActivityResultLauncher; +import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult; +import androidx.core.util.Consumer; + +import static java.util.logging.Level.INFO; +import static java.util.logging.Logger.getLogger; /** - * Abstract base class for the ConditionManagers that ensure that the conditions - * to open a hotspot are fulfilled. There are different extensions of this for - * API levels lower than 29 and 29+. + * This class ensures that the conditions to open a hotspot are fulfilled on + * API levels < 29. + *

+ * As soon as {@link #checkAndRequestConditions()} returns true, + * all conditions are fulfilled. */ -abstract class ConditionManager { +class ConditionManager extends AbstractConditionManager { - enum Permission { - UNKNOWN, GRANTED, SHOW_RATIONALE, PERMANENTLY_DENIED + private static final Logger LOG = + getLogger(ConditionManager.class.getName()); + + private final ActivityResultLauncher wifiRequest; + + ConditionManager(ActivityResultCaller arc, + Consumer permissionUpdateCallback) { + super(permissionUpdateCallback); + wifiRequest = arc.registerForActivityResult( + new StartActivityForResult(), + result -> permissionUpdateCallback + .accept(wifiManager.isWifiEnabled())); } - protected final Consumer permissionUpdateCallback; - protected FragmentActivity ctx; - WifiManager wifiManager; - - ConditionManager(Consumer permissionUpdateCallback) { - this.permissionUpdateCallback = permissionUpdateCallback; + @Override + void onStart() { + // nothing to do here } - /** - * Pass a FragmentActivity context here during `onCreateView()`. - */ - void init(FragmentActivity ctx) { - this.ctx = ctx; - this.wifiManager = (WifiManager) ctx.getApplicationContext() - .getSystemService(WIFI_SERVICE); + private boolean areEssentialPermissionsGranted() { + if (LOG.isLoggable(INFO)) { + LOG.info(String.format("areEssentialPermissionsGranted(): " + + "wifiManager.isWifiEnabled()? %b", + wifiManager.isWifiEnabled())); + } + return wifiManager.isWifiEnabled(); } - /** - * Call this during onStart() in the fragment where the ConditionManager - * is used. - */ - abstract void onStart(); + @Override + boolean checkAndRequestConditions() { + if (areEssentialPermissionsGranted()) return true; - /** - * Check if all required conditions are met such that the hotspot can be - * started. If any precondition is not met yet, bring up relevant dialogs - * asking the user to grant relevant permissions or take relevant actions. - * - * @return true if conditions are fulfilled and flow can continue. - */ - abstract boolean checkAndRequestConditions(); + if (!wifiManager.isWifiEnabled()) { + // Try enabling the Wifi and return true if that seems to have been + // successful, i.e. "Wifi is either already in the requested state, or + // in progress toward the requested state". + if (wifiManager.setWifiEnabled(true)) { + LOG.info("Enabled wifi"); + return true; + } - void showDenialDialog(FragmentActivity ctx, - @StringRes int title, @StringRes int body, - DialogInterface.OnClickListener onOkClicked, Runnable onDismiss) { - AlertDialog.Builder builder = new AlertDialog.Builder(ctx); - builder.setTitle(title); - builder.setMessage(body); - builder.setPositiveButton(R.string.ok, onOkClicked); - builder.setNegativeButton(R.string.cancel, - (dialog, which) -> ctx.supportFinishAfterTransition()); - builder.setOnDismissListener(dialog -> onDismiss.run()); - builder.show(); + // Wifi is not enabled and we can't seem to enable it, so ask the user + // to enable it for us. + showRationale(ctx, R.string.wifi_settings_title, + R.string.wifi_settings_request_enable_body, + this::requestEnableWiFi, + () -> permissionUpdateCallback.accept(false)); + } + + return false; } - void showRationale(Context ctx, @StringRes int title, - @StringRes int body, Runnable onContinueClicked, - Runnable onDismiss) { - AlertDialog.Builder builder = new AlertDialog.Builder(ctx); - builder.setTitle(title); - builder.setMessage(body); - builder.setNeutralButton(R.string.continue_button, - (dialog, which) -> onContinueClicked.run()); - builder.setOnDismissListener(dialog -> onDismiss.run()); - builder.show(); + private void requestEnableWiFi() { + wifiRequest.launch(new Intent(Settings.ACTION_WIFI_SETTINGS)); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29.java similarity index 96% rename from briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java rename to briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29.java index 464fa4c77..a895e6188 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29Impl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManager29.java @@ -30,17 +30,17 @@ import static org.briarproject.briar.android.util.UiUtils.getGoToSettingsListene * all conditions are fulfilled. */ @RequiresApi(29) -class ConditionManager29Impl extends ConditionManager { +class ConditionManager29 extends AbstractConditionManager { private static final Logger LOG = - getLogger(ConditionManager29Impl.class.getName()); + getLogger(ConditionManager29.class.getName()); private Permission locationPermission = Permission.UNKNOWN; private final ActivityResultLauncher locationRequest; private final ActivityResultLauncher wifiRequest; - ConditionManager29Impl(ActivityResultCaller arc, + ConditionManager29(ActivityResultCaller arc, Consumer permissionUpdateCallback) { super(permissionUpdateCallback); locationRequest = arc.registerForActivityResult( diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java deleted file mode 100644 index 5d7ff1079..000000000 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/ConditionManagerImpl.java +++ /dev/null @@ -1,83 +0,0 @@ -package org.briarproject.briar.android.hotspot; - -import android.content.Intent; -import android.provider.Settings; - -import org.briarproject.briar.R; - -import java.util.logging.Logger; - -import androidx.activity.result.ActivityResultCaller; -import androidx.activity.result.ActivityResultLauncher; -import androidx.activity.result.contract.ActivityResultContracts.StartActivityForResult; -import androidx.core.util.Consumer; - -import static java.util.logging.Level.INFO; -import static java.util.logging.Logger.getLogger; - -/** - * This class ensures that the conditions to open a hotspot are fulfilled on - * API levels < 29. - *

- * As soon as {@link #checkAndRequestConditions()} returns true, - * all conditions are fulfilled. - */ -class ConditionManagerImpl extends ConditionManager { - - private static final Logger LOG = - getLogger(ConditionManagerImpl.class.getName()); - - private final ActivityResultLauncher wifiRequest; - - ConditionManagerImpl(ActivityResultCaller arc, - Consumer permissionUpdateCallback) { - super(permissionUpdateCallback); - wifiRequest = arc.registerForActivityResult( - new StartActivityForResult(), - result -> permissionUpdateCallback - .accept(wifiManager.isWifiEnabled())); - } - - @Override - void onStart() { - // nothing to do here - } - - private boolean areEssentialPermissionsGranted() { - if (LOG.isLoggable(INFO)) { - LOG.info(String.format("areEssentialPermissionsGranted(): " + - "wifiManager.isWifiEnabled()? %b", - wifiManager.isWifiEnabled())); - } - return wifiManager.isWifiEnabled(); - } - - @Override - boolean checkAndRequestConditions() { - if (areEssentialPermissionsGranted()) return true; - - if (!wifiManager.isWifiEnabled()) { - // Try enabling the Wifi and return true if that seems to have been - // successful, i.e. "Wifi is either already in the requested state, or - // in progress toward the requested state". - if (wifiManager.setWifiEnabled(true)) { - LOG.info("Enabled wifi"); - return true; - } - - // Wifi is not enabled and we can't seem to enable it, so ask the user - // to enable it for us. - showRationale(ctx, R.string.wifi_settings_title, - R.string.wifi_settings_request_enable_body, - this::requestEnableWiFi, - () -> permissionUpdateCallback.accept(false)); - } - - return false; - } - - private void requestEnableWiFi() { - wifiRequest.launch(new Intent(Settings.ACTION_WIFI_SETTINGS)); - } - -} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java index 716ea4f97..fd6112bd0 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/hotspot/HotspotIntroFragment.java @@ -47,9 +47,9 @@ public class HotspotIntroFragment extends Fragment { private ProgressBar progressBar; private TextView progressTextView; - private final ConditionManager conditionManager = SDK_INT < 29 ? - new ConditionManagerImpl(this, this::onPermissionUpdate) : - new ConditionManager29Impl(this, this::onPermissionUpdate); + private final AbstractConditionManager conditionManager = SDK_INT < 29 ? + new ConditionManager(this, this::onPermissionUpdate) : + new ConditionManager29(this, this::onPermissionUpdate); @Override public void onAttach(Context context) {