Screen Lock: Second round of addressing review comments

This commit is contained in:
Torsten Grote
2018-08-07 15:08:38 -03:00
parent 6863727646
commit 02ff37b187
39 changed files with 125 additions and 124 deletions

View File

@@ -321,11 +321,8 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
@Override
public void updateForegroundNotification(boolean lockable,
boolean locked) {
NotificationManager nm = (NotificationManager) appContext
.getSystemService(NOTIFICATION_SERVICE);
assert nm != null;
Notification n = getForegroundNotification(lockable, locked);
nm.notify(ONGOING_NOTIFICATION_ID, n);
notificationManager.notify(ONGOING_NOTIFICATION_ID, n);
}
private void showContactNotification(ContactId c) {

View File

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

View File

@@ -4,14 +4,18 @@ import android.app.Application;
import android.arch.lifecycle.LiveData;
import android.arch.lifecycle.MutableLiveData;
import android.content.Context;
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,10 +34,10 @@ import static org.briarproject.briar.android.util.UiUtils.hasScreenLock;
@ThreadSafe
@MethodsNotNullByDefault
@ParametersNotNullByDefault
public class LockManagerImpl implements LockManager, Service {
public class LockManagerImpl implements LockManager, Service, EventListener {
private static final Logger LOG =
Logger.getLogger(LockManagerImpl.class.getSimpleName());
Logger.getLogger(LockManagerImpl.class.getName());
private final Context appContext;
private final SettingsManager settingsManager;
@@ -53,6 +57,7 @@ public class LockManagerImpl implements LockManager, Service {
this.notificationManager = notificationManager;
this.dbExecutor = dbExecutor;
// setting these in the constructor makes #getValue() @NonNull
this.locked.setValue(false);
this.lockable.setValue(false);
}
@@ -61,17 +66,7 @@ public class LockManagerImpl implements LockManager, Service {
public void startService() {
lockable.observeForever(this::onLockableChanged);
if (hasScreenLock(appContext)) {
dbExecutor.execute(() -> {
try {
Settings settings =
settingsManager.getSettings(SETTINGS_NAMESPACE);
boolean lockable =
settings.getBoolean(PREF_SCREEN_LOCK, false);
this.lockable.postValue(lockable);
} catch (DbException e) {
logException(LOG, WARNING, e);
}
});
loadLockableSetting();
} else {
lockable.postValue(false);
}
@@ -82,6 +77,33 @@ public class LockManagerImpl implements LockManager, Service {
lockable.removeObserver(this::onLockableChanged);
}
@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);
boolean lockable =
settings.getBoolean(PREF_SCREEN_LOCK, false);
boolean newValue = hasScreenLock(appContext) && lockable;
this.lockable.postValue(newValue);
} catch (DbException e) {
logException(LOG, WARNING, e);
this.lockable.postValue(false);
}
});
}
private void onLockableChanged(boolean lockable) {
notificationManager
.updateForegroundNotification(lockable, locked.getValue());
@@ -92,6 +114,7 @@ public class LockManagerImpl implements LockManager, Service {
return lockable;
}
@UiThread
@Override
public void recheckLockable() {
boolean oldValue = this.lockable.getValue();
@@ -101,15 +124,6 @@ public class LockManagerImpl implements LockManager, Service {
}
}
@Override
public void updateLockableSetting(boolean lockable) {
boolean oldValue = this.lockable.getValue();
boolean newValue = hasScreenLock(appContext) && lockable;
if (oldValue != newValue) {
this.lockable.setValue(newValue);
}
}
@Override
public LiveData<Boolean> isLocked() {
return locked;

View File

@@ -35,7 +35,6 @@ 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 {
@@ -71,12 +70,12 @@ public abstract class BriarActivity extends BaseActivity {
if (!briarController.accountSignedIn() && !isFinishing()) {
Intent i = new Intent(this, PasswordActivity.class);
startActivityForResult(i, REQUEST_PASSWORD);
} else if(lockManager.isLocked().getValue()) {
} else if (lockManager.isLocked().getValue()) {
Intent i = new Intent(this, UnlockActivity.class);
startActivityForResult(i, REQUEST_UNLOCK);
} else {
lockManager.isLocked().observe(this, locked -> {
if (locked != null && locked) showAndroidHomeScreen(this);
if (locked != null && locked) moveTaskToBack(true);
});
lockManager.recheckLockable();
if (SDK_INT >= 23) {

View File

@@ -22,7 +22,6 @@ 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)
@@ -61,7 +60,7 @@ public class UnlockActivity extends BaseActivity {
@Override
public void onBackPressed() {
showAndroidHomeScreen(this);
moveTaskToBack(true);
}
@Override

View File

@@ -41,7 +41,6 @@ import org.briarproject.briar.android.Localizer;
import org.briarproject.briar.android.navdrawer.NavDrawerActivity;
import org.briarproject.briar.android.util.UiUtils;
import org.briarproject.briar.android.util.UserFeedback;
import org.briarproject.briar.api.android.LockManager;
import java.util.ArrayList;
import java.util.Collections;
@@ -141,8 +140,6 @@ public class SettingsFragment extends PreferenceFragmentCompat
@Inject
volatile EventBus eventBus;
@Inject
LockManager lockManager;
@Inject
AndroidExecutor androidExecutor;
@@ -184,8 +181,6 @@ public class SettingsFragment extends PreferenceFragmentCompat
"pref_key_notify_lock_screen");
notifySound = findPreference("pref_key_notify_sound");
setSettingsEnabled(false);
language.setOnPreferenceChangeListener(this);
theme.setOnPreferenceChangeListener((preference, newValue) -> {
if (getActivity() != null) {
@@ -239,7 +234,6 @@ public class SettingsFragment extends PreferenceFragmentCompat
testing.setVisible(false);
}
loadSettings();
}
@Override
@@ -256,7 +250,8 @@ public class SettingsFragment extends PreferenceFragmentCompat
public void onStart() {
super.onStart();
eventBus.addListener(this);
updateScreenLockSetting(settings != null);
setSettingsEnabled(false);
loadSettings();
}
@Override
@@ -365,13 +360,8 @@ public class SettingsFragment extends PreferenceFragmentCompat
enableBluetooth.setValue(Boolean.toString(btSetting));
torNetwork.setValue(Integer.toString(torNetworkSetting));
torBlocked.setChecked(torBlockedSetting);
displayScreenLockSetting();
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));
@@ -428,10 +418,11 @@ 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
// - pref_key_lock (screenLock -> displayScreenLockSetting())
enableBluetooth.setEnabled(enabled);
torNetwork.setEnabled(enabled);
torBlocked.setEnabled(enabled);
updateScreenLockSetting(enabled);
notifyPrivateMessages.setEnabled(enabled);
notifyGroupMessages.setEnabled(enabled);
notifyForumPosts.setEnabled(enabled);
@@ -441,15 +432,20 @@ public class SettingsFragment extends PreferenceFragmentCompat
notifySound.setEnabled(enabled);
}
private void updateScreenLockSetting(boolean enabled) {
private void displayScreenLockSetting() {
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));
if (getActivity() != null && hasScreenLock(getActivity())) {
screenLock.setEnabled(true);
screenLock.setChecked(
settings.getBoolean(PREF_SCREEN_LOCK, false));
screenLock.setSummary(R.string.pref_lock_summary);
} else {
screenLock.setEnabled(false);
screenLock.setChecked(false);
screenLock.setSummary(R.string.pref_lock_disabled_summary);
}
}
}
@@ -509,7 +505,6 @@ public class SettingsFragment extends PreferenceFragmentCompat
boolean torBlockedSetting = (Boolean) newValue;
storeTorBlockedSetting(torBlockedSetting);
} else if (preference == screenLock) {
lockManager.updateLockableSetting((Boolean) newValue);
Settings s = new Settings();
s.putBoolean(PREF_SCREEN_LOCK, (Boolean) newValue);
storeSettings(s);

View File

@@ -50,8 +50,9 @@ public class SplashScreenActivity extends BaseActivity {
if (accountManager.hasDatabaseKey()) {
Intent i;
if (lockManager.isLocked().getValue()) {
// The database is already open, so start main activity which
// will open the activity to unlock, then brings main to front.
// The database needs to be opened for the app to be locked.
// Start main activity right away. It will open UnlockActivity.
// Otherwise, we would end up with two screen unlock inputs.
i = new Intent(this, NavDrawerActivity.class);
} else {
i = new Intent(this, OpenDatabaseActivity.class);

View File

@@ -37,6 +37,7 @@ import org.briarproject.briar.android.widget.LinkDialogFragment;
import javax.annotation.Nullable;
import static android.content.Context.KEYGUARD_SERVICE;
import static android.content.Context.POWER_SERVICE;
import static android.content.Intent.CATEGORY_DEFAULT;
import static android.content.Intent.FLAG_ACTIVITY_NEW_TASK;
@@ -232,19 +233,13 @@ public class UiUtils {
public static boolean hasScreenLock(Context ctx) {
if (SDK_INT < 21) return false;
KeyguardManager keyguardManager = (KeyguardManager) ctx
.getSystemService(Context.KEYGUARD_SERVICE);
KeyguardManager keyguardManager =
(KeyguardManager) ctx.getSystemService(KEYGUARD_SERVICE);
if (keyguardManager == null) return false;
// check if there's a lock mechanism we can use, try to ignore SIM
// check if there's a lock mechanism we can use
// first one is true if SIM card is locked, so use second if available
return (SDK_INT < 23 && keyguardManager.isKeyguardSecure()) ||
(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);
}
}

View File

@@ -1,15 +1,15 @@
package org.briarproject.briar.api.android;
import android.arch.lifecycle.LiveData;
import android.support.annotation.UiThread;
public interface LockManager {
LiveData<Boolean> isLockable();
@UiThread
void recheckLockable();
void updateLockableSetting(boolean lockable);
LiveData<Boolean> isLocked();
void setLocked(boolean locked);