Screen Lock: Address first round of review comments

briar-android/src/main/java/org/briarproject/briar/android/activity/BriarActivity.java

@@ -35,6 +35,7 @@ import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_PASSW
 import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_UNLOCK;
 import static org.briarproject.briar.android.util.UiUtils.getDozeWhitelistingIntent;
 import static org.briarproject.briar.android.util.UiUtils.isSamsung7;
+import static org.briarproject.briar.android.util.UiUtils.showAndroidHomeScreen;
 
 @SuppressLint("Registered")
 public abstract class BriarActivity extends BaseActivity {
@@ -73,18 +74,30 @@ public abstract class BriarActivity extends BaseActivity {
 		} else if(lockManager.isLocked().getValue()) {
 			Intent i = new Intent(this, UnlockActivity.class);
 			startActivityForResult(i, REQUEST_UNLOCK);
-		} else if (SDK_INT >= 23) {
-			briarController.hasDozed(new UiResultHandler<Boolean>(this) {
-				@Override
-				public void onResultUi(Boolean result) {
-					if (result) {
-						showDozeDialog(getString(R.string.warning_dozed,
-								getString(R.string.app_name)));
-					}
-				}
+		} else {
+			lockManager.isLocked().observe(this, locked -> {
+				if (locked != null && locked) showAndroidHomeScreen(this);
 			});
+			lockManager.recheckLockable();
+			if (SDK_INT >= 23) {
+				briarController.hasDozed(new UiResultHandler<Boolean>(this) {
+					@Override
+					public void onResultUi(Boolean result) {
+						if (result) {
+							showDozeDialog(getString(R.string.warning_dozed,
+									getString(R.string.app_name)));
+						}
+					}
+				});
+			}
 		}
-		lockManager.recheckLockable();
+	}
+
+	@Override
+	protected void onStop() {
+		super.onStop();
+		// only react to lock changes while in foreground
+		lockManager.isLocked().removeObservers(this);
 	}
 
 	public void setSceneTransitionAnimation() {
@@ -125,10 +138,6 @@ public abstract class BriarActivity extends BaseActivity {
 		return toolbar;
 	}
 
-	protected void onLockableChanged(boolean lockable) {
-
-	}
-
 	protected void showDozeDialog(String message) {
 		AlertDialog.Builder b =
 				new AlertDialog.Builder(this, R.style.BriarDialogTheme);

briar-android/src/main/java/org/briarproject/briar/android/login/UnlockActivity.java

@@ -22,6 +22,7 @@ import java.util.logging.Logger;
 import javax.inject.Inject;
 
 import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_UNLOCK;
+import static org.briarproject.briar.android.util.UiUtils.showAndroidHomeScreen;
 import static org.briarproject.briar.api.android.AndroidNotificationManager.ACTION_LOCK;
 
 @RequiresApi(21)
@@ -52,11 +53,17 @@ public class UnlockActivity extends BaseActivity {
 		Intent intent = getIntent();
 		if (intent != null && ACTION_LOCK.equals(intent.getAction())) {
 			lockManager.setLocked(true);
+			finish();
 		} else {
 			requestKeyguardUnlock();
 		}
 	}
 
+	@Override
+	public void onBackPressed() {
+		showAndroidHomeScreen(this);
+	}
+
 	@Override
 	protected void onActivityResult(int requestCode, int resultCode,
 			Intent data) {
@@ -70,9 +77,9 @@ public class UnlockActivity extends BaseActivity {
 		KeyguardManager keyguardManager = (KeyguardManager) getSystemService(
 				Context.KEYGUARD_SERVICE);
 		assert keyguardManager != null;
-		Intent intent = keyguardManager
-				.createConfirmDeviceCredentialIntent(getString(R.string.lock_unlock),
-						null);
+		Intent intent = keyguardManager.createConfirmDeviceCredentialIntent(
+				getString(R.string.lock_unlock),
+				getString(R.string.lock_unlock_description));
 		if (intent == null) {
 			// the user must have removed the screen lock since locked
 			LOG.warning("Unlocking without keyguard");

briar-android/src/main/java/org/briarproject/briar/android/settings/SettingsFragment.java

@@ -256,6 +256,7 @@ public class SettingsFragment extends PreferenceFragmentCompat
 	public void onStart() {
 		super.onStart();
 		eventBus.addListener(this);
+		updateScreenLockSetting(settings != null);
 	}
 
 	@Override
@@ -350,10 +351,8 @@ public class SettingsFragment extends PreferenceFragmentCompat
 						PREF_TOR_NETWORK_ALWAYS);
 				boolean torBlockedSetting =
 						torSettings.getBoolean(PREF_TOR_DISABLE_BLOCKED, true);
-				boolean screenLockSetting =
-						settings.getBoolean(PREF_SCREEN_LOCK, false);
-				displaySettings(btSetting, torNetworkSetting, torBlockedSetting,
-						screenLockSetting);
+				displaySettings(btSetting, torNetworkSetting,
+						torBlockedSetting);
 			} catch (DbException e) {
 				logException(LOG, WARNING, e);
 			}
@@ -361,13 +360,18 @@ public class SettingsFragment extends PreferenceFragmentCompat
 	}
 
 	private void displaySettings(boolean btSetting, int torNetworkSetting,
-			boolean torBlockedSetting, boolean screenLockSetting) {
+			boolean torBlockedSetting) {
 		listener.runOnUiThreadUnlessDestroyed(() -> {
 			enableBluetooth.setValue(Boolean.toString(btSetting));
 			torNetwork.setValue(Integer.toString(torNetworkSetting));
 			torBlocked.setChecked(torBlockedSetting);
-			screenLock.setChecked(screenLockSetting);
 
+			if (SDK_INT >= 21) {
+				boolean screenLockable =
+						settings.getBoolean(PREF_SCREEN_LOCK, false);
+				screenLock.setChecked(
+						screenLockable && hasScreenLock(getActivity()));
+			}
 			if (SDK_INT < 26) {
 				notifyPrivateMessages.setChecked(settings.getBoolean(
 						PREF_NOTIFY_PRIVATE, true));
@@ -427,12 +431,7 @@ public class SettingsFragment extends PreferenceFragmentCompat
 		enableBluetooth.setEnabled(enabled);
 		torNetwork.setEnabled(enabled);
 		torBlocked.setEnabled(enabled);
-		if (enabled && getActivity() != null && hasScreenLock(getActivity())) {
-			screenLock.setEnabled(true);
-		} else {
-			screenLock.setEnabled(false);
-			screenLock.setSummary(getString(R.string.lock_disabled));
-		}
+		updateScreenLockSetting(enabled);
 		notifyPrivateMessages.setEnabled(enabled);
 		notifyGroupMessages.setEnabled(enabled);
 		notifyForumPosts.setEnabled(enabled);
@@ -442,6 +441,18 @@ public class SettingsFragment extends PreferenceFragmentCompat
 		notifySound.setEnabled(enabled);
 	}
 
+	private void updateScreenLockSetting(boolean enabled) {
+		if (SDK_INT < 21) {
+			screenLock.setVisible(false);
+		} else if (enabled && hasScreenLock(getActivity())) {
+			screenLock.setEnabled(true);
+		} else {
+			screenLock.setEnabled(false);
+			screenLock.setChecked(false);
+			screenLock.setSummary(getString(R.string.lock_disabled));
+		}
+	}
+
 	@TargetApi(26)
 	private void setupNotificationPreference(CheckBoxPreference pref,
 			String channelId, @StringRes int summary) {

briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java

@@ -240,4 +240,11 @@ public class UiUtils {
 				(SDK_INT >= 23 && keyguardManager.isDeviceSecure());
 	}
 
+	public static void showAndroidHomeScreen(Context ctx) {
+		Intent i = new Intent(Intent.ACTION_MAIN);
+		i.addCategory(Intent.CATEGORY_HOME);
+		i.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+		ctx.startActivity(i);
+	}
+
 }

briar-android/src/main/res/values/strings.xml

@@ -358,7 +358,7 @@
 	<!-- Settings Security and Panic -->
 	<string name="security_settings_title">Security</string>
 	<string name="pref_lock_title">Screen Lock</string>
-	<string name="pref_lock_summary">Lock app access with Android screen lock or fingerprint</string>
+	<string name="pref_lock_summary">Use the device\'s screen lock to protect Briar while signed in</string>
 	<string name="change_password">Change password</string>
 	<string name="current_password">Current password</string>
 	<string name="choose_new_password">New password</string>
@@ -450,9 +450,10 @@
 
 	<!-- App Locking -->
 	<string name="lock_unlock">Unlock Briar</string>
+	<string name="lock_unlock_description">Enter your device PIN, pattern or password to continue</string>
 	<string name="lock_is_locked">Briar is locked</string>
 	<string name="lock_tap_to_unlock">Tap to unlock</string>
-	<string name="lock_disabled">Only available on Android 5 or newer and when a system screen lock is set</string>
+	<string name="lock_disabled">Set up a screen lock for your device to protect Briar while signed in</string>
 	<!-- This is meant as an imperative, but should be shorter than: Lock the app! -->
 	<string name="lock_lock">Lock</string>