Address review comments

This commit is contained in:
Torsten Grote
2017-11-21 15:54:48 -02:00
parent ec2f372933
commit 4ca86ee4eb
7 changed files with 50 additions and 79 deletions

View File

@@ -54,7 +54,6 @@ public class BriarService extends Service {
private final Binder binder = new BriarBinder(); private final Binder binder = new BriarBinder();
@Nullable @Nullable
private BriarBroadcastReceiver receiver = null; private BriarBroadcastReceiver receiver = null;
private boolean hasDozed = false;
@Inject @Inject
protected DatabaseConfig databaseConfig; protected DatabaseConfig databaseConfig;
@@ -63,7 +62,7 @@ public class BriarService extends Service {
protected volatile LifecycleManager lifecycleManager; protected volatile LifecycleManager lifecycleManager;
@Inject @Inject
protected volatile AndroidExecutor androidExecutor; protected volatile AndroidExecutor androidExecutor;
private volatile boolean started = false; private volatile boolean started = false, hasDozed = false;
@Override @Override
public void onCreate() { public void onCreate() {
@@ -185,7 +184,7 @@ public class BriarService extends Service {
private void registerBroadcastReceiver() { private void registerBroadcastReceiver() {
if (SDK_INT < 23) return; if (SDK_INT < 23) return;
IntentFilter filter = new IntentFilter(ACTION_DEVICE_IDLE_MODE_CHANGED); IntentFilter filter = new IntentFilter(ACTION_DEVICE_IDLE_MODE_CHANGED);
if (receiver == null) receiver = new BriarBroadcastReceiver(); receiver = new BriarBroadcastReceiver();
registerReceiver(receiver, filter); registerReceiver(receiver, filter);
} }

View File

@@ -68,7 +68,10 @@ public abstract class BriarActivity extends BaseActivity {
briarController.hasDozed(new UiResultHandler<Boolean>(this) { briarController.hasDozed(new UiResultHandler<Boolean>(this) {
@Override @Override
public void onResultUi(Boolean result) { public void onResultUi(Boolean result) {
if (result) showDozeDialog(); if (result) {
showDozeDialog(getString(R.string.warning_dozed,
getString(R.string.app_name)));
}
} }
}); });
} }
@@ -109,11 +112,10 @@ public abstract class BriarActivity extends BaseActivity {
return toolbar; return toolbar;
} }
private void showDozeDialog() { protected void showDozeDialog(String message) {
AlertDialog.Builder b = AlertDialog.Builder b =
new AlertDialog.Builder(this, R.style.BriarDialogTheme); new AlertDialog.Builder(this, R.style.BriarDialogTheme);
b.setMessage(getString(R.string.warning_dozed, b.setMessage(message);
getString(R.string.app_name)));
b.setView(R.layout.checkbox); b.setView(R.layout.checkbox);
b.setPositiveButton(R.string.fix, b.setPositiveButton(R.string.fix,
(dialog, which) -> { (dialog, which) -> {
@@ -127,7 +129,7 @@ public abstract class BriarActivity extends BaseActivity {
CheckBox checkBox = (CheckBox) ((AlertDialog) dialog) CheckBox checkBox = (CheckBox) ((AlertDialog) dialog)
.findViewById(R.id.checkbox); .findViewById(R.id.checkbox);
if (checkBox.isChecked()) if (checkBox.isChecked())
briarController.doNotNotifyWhenDozed(); briarController.doNotAskAgainForDozeWhiteListing();
}); });
b.show(); b.show();
} }

View File

@@ -14,7 +14,7 @@ public interface BriarController extends ActivityLifecycleController {
*/ */
void hasDozed(ResultHandler<Boolean> handler); void hasDozed(ResultHandler<Boolean> handler);
void doNotNotifyWhenDozed(); void doNotAskAgainForDozeWhiteListing();
void signOut(ResultHandler<Void> eventHandler); void signOut(ResultHandler<Void> eventHandler);
} }

View File

@@ -4,6 +4,8 @@ import android.app.Activity;
import android.content.Intent; import android.content.Intent;
import android.os.IBinder; import android.os.IBinder;
import android.support.annotation.CallSuper; import android.support.annotation.CallSuper;
import android.support.annotation.Nullable;
import android.support.annotation.WorkerThread;
import org.briarproject.bramble.api.db.DatabaseConfig; import org.briarproject.bramble.api.db.DatabaseConfig;
import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DatabaseExecutor;
@@ -28,7 +30,7 @@ public class BriarControllerImpl implements BriarController {
private static final Logger LOG = private static final Logger LOG =
Logger.getLogger(BriarControllerImpl.class.getName()); Logger.getLogger(BriarControllerImpl.class.getName());
private static final String HAS_DOZED_ASK_AGAIN = "hasDozedAskAgain"; public static final String DOZE_ASK_AGAIN = "dozeAskAgain";
private final BriarServiceConnection serviceConnection; private final BriarServiceConnection serviceConnection;
private final DatabaseConfig databaseConfig; private final DatabaseConfig databaseConfig;
@@ -39,6 +41,9 @@ public class BriarControllerImpl implements BriarController {
private boolean bound = false; private boolean bound = false;
@Nullable
private volatile BriarService service;
@Inject @Inject
BriarControllerImpl(BriarServiceConnection serviceConnection, BriarControllerImpl(BriarServiceConnection serviceConnection,
DatabaseConfig databaseConfig, DatabaseConfig databaseConfig,
@@ -76,6 +81,15 @@ public class BriarControllerImpl implements BriarController {
activity.startService(new Intent(activity, BriarService.class)); activity.startService(new Intent(activity, BriarService.class));
bound = activity.bindService(new Intent(activity, BriarService.class), bound = activity.bindService(new Intent(activity, BriarService.class),
serviceConnection, 0); serviceConnection, 0);
if (!bound) throw new IllegalStateException();
new Thread(() -> {
try {
service = getBriarService();
} catch (InterruptedException e) {
LOG.warning("Interrupted while waiting for service");
}
}).start();
} }
@Override @Override
@@ -85,27 +99,19 @@ public class BriarControllerImpl implements BriarController {
@Override @Override
public void hasDozed(ResultHandler<Boolean> handler) { public void hasDozed(ResultHandler<Boolean> handler) {
// check this first, to hit the DbThread only when really necessary BriarService briarService = service;
if (!needsDozeWhitelisting(activity)) { if (briarService == null || !briarService.hasDozed() ||
!needsDozeWhitelisting(activity)) {
handler.onResult(false); handler.onResult(false);
return; return;
} }
if (briarService.hasDozed()) briarService.resetDozeFlag();
databaseExecutor.execute(() -> { databaseExecutor.execute(() -> {
try { try {
Settings settings = Settings settings =
settingsManager.getSettings(SETTINGS_NAMESPACE); settingsManager.getSettings(SETTINGS_NAMESPACE);
boolean ask = settings.getBoolean(HAS_DOZED_ASK_AGAIN, true); boolean ask = settings.getBoolean(DOZE_ASK_AGAIN, true);
if (!ask) { handler.onResult(ask);
handler.onResult(false);
return;
}
IBinder binder = serviceConnection.waitForBinder();
BriarService service =
((BriarService.BriarBinder) binder).getService();
handler.onResult(service.hasDozed());
service.resetDozeFlag();
} catch (InterruptedException e) {
LOG.warning("Interrupted while waiting for service");
} catch (DbException e) { } catch (DbException e) {
if (LOG.isLoggable(WARNING)) if (LOG.isLoggable(WARNING))
LOG.log(WARNING, e.toString(), e); LOG.log(WARNING, e.toString(), e);
@@ -114,11 +120,11 @@ public class BriarControllerImpl implements BriarController {
} }
@Override @Override
public void doNotNotifyWhenDozed() { public void doNotAskAgainForDozeWhiteListing() {
databaseExecutor.execute(() -> { databaseExecutor.execute(() -> {
try { try {
Settings settings = new Settings(); Settings settings = new Settings();
settings.putBoolean(HAS_DOZED_ASK_AGAIN, false); settings.putBoolean(DOZE_ASK_AGAIN, false);
settingsManager.mergeSettings(settings, SETTINGS_NAMESPACE); settingsManager.mergeSettings(settings, SETTINGS_NAMESPACE);
} catch (DbException e) { } catch (DbException e) {
if (LOG.isLoggable(WARNING)) if (LOG.isLoggable(WARNING))
@@ -131,12 +137,11 @@ public class BriarControllerImpl implements BriarController {
public void signOut(ResultHandler<Void> eventHandler) { public void signOut(ResultHandler<Void> eventHandler) {
new Thread() { new Thread() {
@Override @Override
@SuppressWarnings("ConstantConditions")
public void run() { public void run() {
try { try {
if (service == null) service = getBriarService();
// Wait for the service to finish starting up // Wait for the service to finish starting up
IBinder binder = serviceConnection.waitForBinder();
BriarService service =
((BriarService.BriarBinder) binder).getService();
service.waitForStartup(); service.waitForStartup();
// Shut down the service and wait for it to shut down // Shut down the service and wait for it to shut down
LOG.info("Shutting down service"); LOG.info("Shutting down service");
@@ -150,6 +155,12 @@ public class BriarControllerImpl implements BriarController {
}.start(); }.start();
} }
@WorkerThread
private BriarService getBriarService() throws InterruptedException {
IBinder binder = serviceConnection.waitForBinder();
return ((BriarService.BriarBinder) binder).getService();
}
private void unbindService() { private void unbindService() {
if (bound) activity.unbindService(serviceConnection); if (bound) activity.unbindService(serviceConnection);
} }

View File

@@ -1,7 +1,6 @@
package org.briarproject.briar.android.navdrawer; package org.briarproject.briar.android.navdrawer;
import android.annotation.SuppressLint; import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.content.Intent; import android.content.Intent;
import android.content.res.Configuration; import android.content.res.Configuration;
import android.os.Bundle; import android.os.Bundle;
@@ -12,14 +11,12 @@ import android.support.v4.app.FragmentTransaction;
import android.support.v4.content.ContextCompat; import android.support.v4.content.ContextCompat;
import android.support.v4.widget.DrawerLayout; import android.support.v4.widget.DrawerLayout;
import android.support.v7.app.ActionBarDrawerToggle; import android.support.v7.app.ActionBarDrawerToggle;
import android.support.v7.app.AlertDialog;
import android.support.v7.widget.Toolbar; import android.support.v7.widget.Toolbar;
import android.view.LayoutInflater; import android.view.LayoutInflater;
import android.view.MenuItem; import android.view.MenuItem;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import android.widget.BaseAdapter; import android.widget.BaseAdapter;
import android.widget.CheckBox;
import android.widget.GridView; import android.widget.GridView;
import android.widget.ImageView; import android.widget.ImageView;
import android.widget.TextView; import android.widget.TextView;
@@ -54,12 +51,10 @@ import static android.support.v4.view.GravityCompat.START;
import static android.support.v4.widget.DrawerLayout.LOCK_MODE_LOCKED_CLOSED; import static android.support.v4.widget.DrawerLayout.LOCK_MODE_LOCKED_CLOSED;
import static android.view.View.GONE; import static android.view.View.GONE;
import static android.view.View.VISIBLE; import static android.view.View.VISIBLE;
import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_DOZE_WHITELISTING;
import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_PASSWORD; import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_PASSWORD;
import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.NO; import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.NO;
import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.UPDATE; import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.UPDATE;
import static org.briarproject.briar.android.util.UiUtils.getDaysUntilExpiry; import static org.briarproject.briar.android.util.UiUtils.getDaysUntilExpiry;
import static org.briarproject.briar.android.util.UiUtils.getDozeWhitelistingIntent;
public class NavDrawerActivity extends BriarActivity implements public class NavDrawerActivity extends BriarActivity implements
BaseFragmentListener, TransportStateListener, BaseFragmentListener, TransportStateListener,
@@ -158,12 +153,14 @@ public class NavDrawerActivity extends BriarActivity implements
protected void onActivityResult(int request, int result, Intent data) { protected void onActivityResult(int request, int result, Intent data) {
super.onActivityResult(request, result, data); super.onActivityResult(request, result, data);
if (request == REQUEST_PASSWORD && result == RESULT_OK) { if (request == REQUEST_PASSWORD && result == RESULT_OK) {
controller.askDozeWhitelisting(this, controller.shouldAskForDozeWhitelisting(this,
new UiResultHandler<Boolean>(this) { new UiResultHandler<Boolean>(this) {
@Override @Override
public void onResultUi(Boolean ask) { public void onResultUi(Boolean ask) {
if (!ask) return; if (ask) {
requestDozeWhitelisting(); showDozeDialog(
getString(R.string.setup_doze_intro));
}
} }
}); });
} }
@@ -328,28 +325,6 @@ public class NavDrawerActivity extends BriarActivity implements
expiryWarning.setVisibility(VISIBLE); expiryWarning.setVisibility(VISIBLE);
} }
@TargetApi(23)
private void requestDozeWhitelisting() {
AlertDialog.Builder b =
new AlertDialog.Builder(this, R.style.BriarDialogTheme);
b.setMessage(R.string.setup_doze_intro);
b.setView(R.layout.checkbox);
b.setPositiveButton(R.string.ok,
(dialog, which) -> {
Intent i = getDozeWhitelistingIntent(
NavDrawerActivity.this);
startActivityForResult(i, REQUEST_DOZE_WHITELISTING);
});
b.setNegativeButton(R.string.cancel,
(dialog, which) -> {
CheckBox checkBox = (CheckBox) ((AlertDialog) dialog)
.findViewById(R.id.checkbox);
if (checkBox.isChecked())
controller.doNotAskAgainForDozeWhiteListing();
});
b.show();
}
private void initializeTransports(LayoutInflater inflater) { private void initializeTransports(LayoutInflater inflater) {
transports = new ArrayList<>(3); transports = new ArrayList<>(3);

View File

@@ -18,9 +18,7 @@ public interface NavDrawerController extends ActivityLifecycleController {
void expiryWarningDismissed(); void expiryWarningDismissed();
void askDozeWhitelisting(final Context ctx, void shouldAskForDozeWhitelisting(Context ctx,
final ResultHandler<Boolean> handler); ResultHandler<Boolean> handler);
void doNotAskAgainForDozeWhiteListing();
} }

View File

@@ -29,6 +29,7 @@ import javax.inject.Inject;
import static java.util.logging.Level.INFO; import static java.util.logging.Level.INFO;
import static java.util.logging.Level.WARNING; import static java.util.logging.Level.WARNING;
import static org.briarproject.briar.android.BriarApplication.EXPIRY_DATE; import static org.briarproject.briar.android.BriarApplication.EXPIRY_DATE;
import static org.briarproject.briar.android.controller.BriarControllerImpl.DOZE_ASK_AGAIN;
import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.NO; import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.NO;
import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.SHOW; import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.SHOW;
import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.UPDATE; import static org.briarproject.briar.android.navdrawer.NavDrawerController.ExpiryWarning.UPDATE;
@@ -44,7 +45,6 @@ public class NavDrawerControllerImpl extends DbControllerImpl
Logger.getLogger(NavDrawerControllerImpl.class.getName()); Logger.getLogger(NavDrawerControllerImpl.class.getName());
private static final String EXPIRY_DATE_WARNING = "expiryDateWarning"; private static final String EXPIRY_DATE_WARNING = "expiryDateWarning";
private static final String EXPIRY_SHOW_UPDATE = "expiryShowUpdate"; private static final String EXPIRY_SHOW_UPDATE = "expiryShowUpdate";
private static final String DOZE_ASK_AGAIN = "dozeAskAgain";
private final PluginManager pluginManager; private final PluginManager pluginManager;
private final SettingsManager settingsManager; private final SettingsManager settingsManager;
@@ -158,7 +158,7 @@ public class NavDrawerControllerImpl extends DbControllerImpl
} }
@Override @Override
public void askDozeWhitelisting(Context ctx, public void shouldAskForDozeWhitelisting(Context ctx,
ResultHandler<Boolean> handler) { ResultHandler<Boolean> handler) {
// check this first, to hit the DbThread only when really necessary // check this first, to hit the DbThread only when really necessary
if (!needsDozeWhitelisting(ctx)) { if (!needsDozeWhitelisting(ctx)) {
@@ -179,20 +179,6 @@ public class NavDrawerControllerImpl extends DbControllerImpl
}); });
} }
@Override
public void doNotAskAgainForDozeWhiteListing() {
runOnDbThread(() -> {
try {
Settings settings = new Settings();
settings.putBoolean(DOZE_ASK_AGAIN, false);
settingsManager.mergeSettings(settings, SETTINGS_NAMESPACE);
} catch (DbException e) {
if (LOG.isLoggable(WARNING))
LOG.log(WARNING, e.toString(), e);
}
});
}
@Override @Override
public boolean isTransportRunning(TransportId transportId) { public boolean isTransportRunning(TransportId transportId) {
Plugin plugin = pluginManager.getPlugin(transportId); Plugin plugin = pluginManager.getPlugin(transportId);