Screen Lock: More changes due to code review

briar-android/src/main/java/org/briarproject/briar/android/AppModule.java

@@ -204,7 +204,10 @@ public class AppModule {
 
 	@Provides
 	@Singleton
-	LockManager provideLockManager(LockManagerImpl lockManager) {
+	LockManager provideLockManager(LifecycleManager lifecycleManager,
+			EventBus eventBus, LockManagerImpl lockManager) {
+		lifecycleManager.registerService(lockManager);
+		eventBus.addListener(lockManager);
 		return lockManager;
 	}
 

briar-android/src/main/java/org/briarproject/briar/android/account/LockManagerImpl.java

@@ -8,10 +8,14 @@ import android.support.annotation.UiThread;
 
 import org.briarproject.bramble.api.db.DatabaseExecutor;
 import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.event.Event;
+import org.briarproject.bramble.api.event.EventListener;
+import org.briarproject.bramble.api.lifecycle.Service;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.bramble.api.settings.Settings;
 import org.briarproject.bramble.api.settings.SettingsManager;
+import org.briarproject.bramble.api.settings.event.SettingsUpdatedEvent;
 import org.briarproject.briar.api.android.AndroidNotificationManager;
 import org.briarproject.briar.api.android.LockManager;
 
@@ -30,7 +34,7 @@ import static org.briarproject.briar.android.util.UiUtils.hasScreenLock;
 @ThreadSafe
 @MethodsNotNullByDefault
 @ParametersNotNullByDefault
-public class LockManagerImpl implements LockManager {
+public class LockManagerImpl implements LockManager, Service, EventListener {
 
 	private static final Logger LOG =
 			Logger.getLogger(LockManagerImpl.class.getName());
@@ -42,6 +46,7 @@ public class LockManagerImpl implements LockManager {
 	private final Executor dbExecutor;
 
 	private volatile boolean locked = false;
+	private volatile boolean lockableSetting = false;
 	private final MutableLiveData<Boolean> lockable = new MutableLiveData<>();
 
 	@Inject
@@ -57,6 +62,16 @@ public class LockManagerImpl implements LockManager {
 		this.lockable.setValue(false);
 	}
 
+	@Override
+	public void startService() {
+		// only load the setting here, because database isn't open before
+		loadLockableSetting();
+	}
+
+	@Override
+	public void stopService() {
+	}
+
 	@Override
 	public LiveData<Boolean> isLockable() {
 		return lockable;
@@ -66,21 +81,10 @@ public class LockManagerImpl implements LockManager {
 	@Override
 	public void checkIfLockable() {
 		boolean oldValue = lockable.getValue();
-		dbExecutor.execute(() -> {
+		boolean newValue = hasScreenLock(appContext) && lockableSetting;
-			try {
+		if (oldValue != newValue) {
-				Settings settings =
+			this.lockable.setValue(newValue);
-						settingsManager.getSettings(SETTINGS_NAMESPACE);
+		}
-				boolean lockable =
-						settings.getBoolean(PREF_SCREEN_LOCK, false);
-				boolean newValue = hasScreenLock(appContext) && lockable;
-				if (oldValue != newValue) {
-					this.lockable.postValue(newValue);
-				}
-			} catch (DbException e) {
-				logException(LOG, WARNING, e);
-				this.lockable.postValue(false);
-			}
-		});
 	}
 
 	@Override
@@ -93,4 +97,32 @@ public class LockManagerImpl implements LockManager {
 		this.locked = locked;
 		notificationManager.updateForegroundNotification(locked);
 	}
+
+	@Override
+	public void eventOccurred(Event event) {
+		if (event instanceof SettingsUpdatedEvent) {
+			SettingsUpdatedEvent e = (SettingsUpdatedEvent) event;
+			String namespace = e.getNamespace();
+			if (namespace.equals(SETTINGS_NAMESPACE)) {
+				loadLockableSetting();
+			}
+		}
+	}
+
+	private void loadLockableSetting() {
+		dbExecutor.execute(() -> {
+			try {
+				Settings settings =
+						settingsManager.getSettings(SETTINGS_NAMESPACE);
+				lockableSetting = settings.getBoolean(PREF_SCREEN_LOCK, false);
+				boolean newValue = hasScreenLock(appContext) && lockableSetting;
+				lockable.postValue(newValue);
+			} catch (DbException e) {
+				logException(LOG, WARNING, e);
+				lockableSetting = false;
+				lockable.postValue(false);
+			}
+		});
+	}
+
 }

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

@@ -60,6 +60,7 @@ public abstract class BriarActivity extends BaseActivity {
 			if (result == RESULT_OK) briarController.startAndBindService();
 			else supportFinishAfterTransition();
 		} else if (request == REQUEST_UNLOCK) {
+			// if we don't finish here, we will enter onStart()
 			if (result != RESULT_OK) supportFinishAfterTransition();
 		}
 	}
@@ -73,7 +74,6 @@ public abstract class BriarActivity extends BaseActivity {
 		} else if (lockManager.isLocked()) {
 			Intent i = new Intent(this, UnlockActivity.class);
 			startActivityForResult(i, REQUEST_UNLOCK);
-			overridePendingTransition(0, 0);
 		} else if (SDK_INT >= 23) {
 			briarController.hasDozed(new UiResultHandler<Boolean>(this) {
 				@Override

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

@@ -13,5 +13,6 @@ public interface RequestCodes {
 	int REQUEST_DOZE_WHITELISTING = 9;
 	int REQUEST_ENABLE_BLUETOOTH = 10;
 	int REQUEST_UNLOCK = 11;
+	int REQUEST_KEYGUARD_UNLOCK = 12;
 
 }

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

@@ -1,15 +1,12 @@
 package org.briarproject.briar.android.login;
 
 import android.app.KeyguardManager;
-import android.content.Context;
 import android.content.Intent;
 import android.os.Bundle;
 import android.support.annotation.Nullable;
 import android.support.annotation.RequiresApi;
-import android.support.v4.app.ActivityCompat;
 import android.widget.Button;
 
-import org.briarproject.bramble.api.account.AccountManager;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.briar.R;
@@ -21,7 +18,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.activity.RequestCodes.REQUEST_KEYGUARD_UNLOCK;
 
 @RequiresApi(21)
 @MethodsNotNullByDefault
@@ -31,8 +28,6 @@ public class UnlockActivity extends BaseActivity {
 	private static final Logger LOG =
 			Logger.getLogger(UnlockActivity.class.getSimpleName());
 
-	@Inject
-	AccountManager accountManager;
 	@Inject
 	LockManager lockManager;
 
@@ -43,6 +38,7 @@ public class UnlockActivity extends BaseActivity {
 
 	public void onCreate(@Nullable Bundle state) {
 		super.onCreate(state);
+		overridePendingTransition(0, 0);
 		setContentView(R.layout.activity_unlock);
 
 		Button button = findViewById(R.id.unlock);
@@ -60,25 +56,24 @@ public class UnlockActivity extends BaseActivity {
 	protected void onActivityResult(int requestCode, int resultCode,
 			Intent data) {
 		super.onActivityResult(requestCode, resultCode, data);
-		if (requestCode == REQUEST_UNLOCK) {
+		if (requestCode == REQUEST_KEYGUARD_UNLOCK) {
 			if (resultCode == RESULT_OK) unlock();
 			else finish();
 		}
 	}
 
 	private void requestKeyguardUnlock() {
-		KeyguardManager keyguardManager = (KeyguardManager) getSystemService(
+		KeyguardManager keyguardManager =
-				Context.KEYGUARD_SERVICE);
+				(KeyguardManager) getSystemService(KEYGUARD_SERVICE);
-		assert keyguardManager != null;
+		if (keyguardManager == null) throw new AssertionError();
 		Intent intent = keyguardManager.createConfirmDeviceCredentialIntent(
-				getString(R.string.lock_unlock),
+				getString(R.string.lock_unlock), null);
-				getString(R.string.lock_unlock_description));
 		if (intent == null) {
 			// the user must have removed the screen lock since locked
 			LOG.warning("Unlocking without keyguard");
 			unlock();
 		} else {
-			startActivityForResult(intent, REQUEST_UNLOCK);
+			startActivityForResult(intent, REQUEST_KEYGUARD_UNLOCK);
 			overridePendingTransition(0, 0);
 		}
 	}
@@ -86,7 +81,8 @@ public class UnlockActivity extends BaseActivity {
 	private void unlock() {
 		lockManager.setLocked(false);
 		setResult(RESULT_OK);
-		ActivityCompat.finishAfterTransition(this);
+		finish();
+		overridePendingTransition(0, 0);
 	}
 
 }

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

@@ -418,11 +418,12 @@ public class SettingsFragment extends PreferenceFragmentCompat
 		// preferences not needed here, because handled by SharedPreferences:
 		// - pref_key_theme
 		// - pref_key_notify_sign_in
-		// preferences not needed here, because they have their own logic
+		// preferences partly needed here, because they have their own logic
 		// - pref_key_lock (screenLock -> displayScreenLockSetting())
 		enableBluetooth.setEnabled(enabled);
 		torNetwork.setEnabled(enabled);
 		torBlocked.setEnabled(enabled);
+		if (!enabled) screenLock.setEnabled(false);
 		notifyPrivateMessages.setEnabled(enabled);
 		notifyGroupMessages.setEnabled(enabled);
 		notifyForumPosts.setEnabled(enabled);

briar-android/src/main/res/layout/activity_unlock.xml

@@ -37,7 +37,7 @@
 
 	<Button
 		android:id="@+id/unlock"
-		style="@style/BriarButton.Default"
+		style="@style/BriarButton"
 		android:layout_width="match_parent"
 		android:layout_height="wrap_content"
 		android:layout_margin="@dimen/margin_large"

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

@@ -451,10 +451,7 @@
 
 	<!-- 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>
-	<!-- This is meant as an imperative, but should be shorter than: Lock the app! -->
-	<string name="lock_lock">Lock</string>
 
 </resources>