From 0ee4ade4047677f599411ae62709bf4ab6948e3b Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 7 Apr 2021 16:18:18 -0300 Subject: [PATCH] One more round of addressing AddNearbyContact review feedback --- .../add/nearby/AddNearbyContactActivity.java | 20 +---------- .../add/nearby/AddNearbyContactFragment.java | 4 +-- .../nearby/AddNearbyContactIntroFragment.java | 20 +++++++++-- .../AddNearbyContactPermissionManager.java | 18 +++++++--- .../add/nearby/AddNearbyContactViewModel.java | 34 +++++++++++-------- 5 files changed, 53 insertions(+), 43 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactActivity.java index 6b731812b..1cb3f8783 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactActivity.java @@ -25,7 +25,6 @@ import javax.annotation.Nullable; import javax.inject.Inject; import androidx.activity.result.ActivityResultLauncher; -import androidx.activity.result.contract.ActivityResultContracts.RequestMultiplePermissions; import androidx.appcompat.widget.Toolbar; import androidx.fragment.app.FragmentManager; import androidx.lifecycle.ViewModelProvider; @@ -50,12 +49,6 @@ public class AddNearbyContactActivity extends BriarActivity ViewModelProvider.Factory viewModelFactory; private AddNearbyContactViewModel viewModel; - private AddNearbyContactPermissionManager permissionManager; - - private final ActivityResultLauncher permissionLauncher = - registerForActivityResult(new RequestMultiplePermissions(), r -> - permissionManager.onRequestPermissionResult(r, - viewModel::showQrCodeFragmentIfAllowed)); private final ActivityResultLauncher bluetoothLauncher = registerForActivityResult(new RequestBluetoothDiscoverable(), this::onBluetoothDiscoverableResult); @@ -65,8 +58,6 @@ public class AddNearbyContactActivity extends BriarActivity component.inject(this); viewModel = new ViewModelProvider(this, viewModelFactory) .get(AddNearbyContactViewModel.class); - permissionManager = new AddNearbyContactPermissionManager(this, - permissionLauncher::launch, viewModel.isBluetoothSupported()); } @Override @@ -79,8 +70,6 @@ public class AddNearbyContactActivity extends BriarActivity if (state == null) { showInitialFragment(AddNearbyContactIntroFragment.newInstance()); } - viewModel.getCheckPermissions().observeEvent(this, check -> - permissionManager.checkPermissions()); viewModel.getRequestBluetoothDiscoverable().observeEvent(this, r -> requestBluetoothDiscoverable()); // never false viewModel.getShowQrCodeFragment().observeEvent(this, show -> { @@ -92,13 +81,6 @@ public class AddNearbyContactActivity extends BriarActivity .observe(this, this::onAddContactStateChanged); } - @Override - public void onStart() { - super.onStart(); - // Permissions may have been granted manually while we were stopped - permissionManager.resetPermissions(); - } - @Override protected void onPostResume() { super.onPostResume(); @@ -165,7 +147,7 @@ public class AddNearbyContactActivity extends BriarActivity } } - private void onAddContactStateChanged(AddContactState state) { + private void onAddContactStateChanged(@Nullable AddContactState state) { if (state instanceof ContactExchangeFinished) { ContactExchangeResult result = ((ContactExchangeFinished) state).result; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactFragment.java index 6f0a841cb..cd1a23f40 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactFragment.java @@ -100,7 +100,7 @@ public class AddNearbyContactFragment extends BaseFragment public void onActivityCreated(@Nullable Bundle savedInstanceState) { super.onActivityCreated(savedInstanceState); requireActivity().setRequestedOrientation(SCREEN_ORIENTATION_NOSENSOR); - cameraView.setPreviewConsumer(viewModel.qrCodeDecoder); + cameraView.setPreviewConsumer(viewModel.getQrCodeDecoder()); } @Override @@ -153,7 +153,7 @@ public class AddNearbyContactFragment extends BaseFragment } @UiThread - private void onAddContactStateChanged(AddContactState state) { + private void onAddContactStateChanged(@Nullable AddContactState state) { if (state instanceof AddContactState.KeyAgreementListening) { Bitmap qrCode = ((AddContactState.KeyAgreementListening) state).qrCode; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactIntroFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactIntroFragment.java index 7bc666550..5a9aa3864 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactIntroFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactIntroFragment.java @@ -15,6 +15,8 @@ import org.briarproject.briar.android.fragment.BaseFragment; import javax.annotation.Nullable; import javax.inject.Inject; +import androidx.activity.result.ActivityResultLauncher; +import androidx.activity.result.contract.ActivityResultContracts.RequestMultiplePermissions; import androidx.lifecycle.ViewModelProvider; import static android.view.View.FOCUS_DOWN; @@ -23,15 +25,22 @@ import static android.view.View.FOCUS_DOWN; @ParametersNotNullByDefault public class AddNearbyContactIntroFragment extends BaseFragment { - public static final String TAG = AddNearbyContactIntroFragment.class.getName(); + public static final String TAG = + AddNearbyContactIntroFragment.class.getName(); @Inject ViewModelProvider.Factory viewModelFactory; private AddNearbyContactViewModel viewModel; + private AddNearbyContactPermissionManager permissionManager; private ScrollView scrollView; + private final ActivityResultLauncher permissionLauncher = + registerForActivityResult(new RequestMultiplePermissions(), r -> + permissionManager.onRequestPermissionResult(r, + viewModel::showQrCodeFragmentIfAllowed)); + public static AddNearbyContactIntroFragment newInstance() { Bundle args = new Bundle(); AddNearbyContactIntroFragment @@ -45,6 +54,9 @@ public class AddNearbyContactIntroFragment extends BaseFragment { component.inject(this); viewModel = new ViewModelProvider(requireActivity(), viewModelFactory) .get(AddNearbyContactViewModel.class); + permissionManager = new AddNearbyContactPermissionManager( + requireActivity(), permissionLauncher::launch, + viewModel.isBluetoothSupported()); } @Nullable @@ -57,13 +69,17 @@ public class AddNearbyContactIntroFragment extends BaseFragment { false); scrollView = v.findViewById(R.id.scrollView); View button = v.findViewById(R.id.continueButton); - button.setOnClickListener(view -> viewModel.onContinueClicked()); + button.setOnClickListener(view -> viewModel.onContinueClicked(() -> + permissionManager.checkPermissions() + )); return v; } @Override public void onStart() { super.onStart(); + // Permissions may have been granted manually while we were stopped + permissionManager.resetPermissions(); scrollView.post(() -> scrollView.fullScroll(FOCUS_DOWN)); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactPermissionManager.java b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactPermissionManager.java index 168087611..d2d387b5f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactPermissionManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactPermissionManager.java @@ -1,23 +1,26 @@ package org.briarproject.briar.android.contact.add.nearby; +import android.content.ActivityNotFoundException; import android.content.Context; import android.content.Intent; import android.location.LocationManager; +import android.widget.Toast; import org.briarproject.briar.R; -import org.briarproject.briar.android.activity.BaseActivity; import java.util.Map; 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.Manifest.permission.CAMERA; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.os.Build.VERSION.SDK_INT; import static android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS; +import static android.widget.Toast.LENGTH_LONG; import static androidx.core.app.ActivityCompat.shouldShowRequestPermissionRationale; import static androidx.core.content.ContextCompat.checkSelfPermission; import static org.briarproject.briar.android.util.UiUtils.getGoToSettingsListener; @@ -31,11 +34,11 @@ class AddNearbyContactPermissionManager { private Permission cameraPermission = Permission.UNKNOWN; private Permission locationPermission = Permission.UNKNOWN; - private final BaseActivity ctx; + private final FragmentActivity ctx; private final Consumer requestPermissions; private final boolean isBluetoothSupported; - AddNearbyContactPermissionManager(BaseActivity ctx, + AddNearbyContactPermissionManager(FragmentActivity ctx, Consumer requestPermissions, boolean isBluetoothSupported) { this.ctx = ctx; @@ -70,7 +73,7 @@ class AddNearbyContactPermissionManager { !isBluetoothSupported); } - boolean areEssentialPermissionsGranted() { + private boolean areEssentialPermissionsGranted() { return cameraPermission == Permission.GRANTED && (SDK_INT < 23 || locationPermission == Permission.GRANTED || !isBluetoothSupported); @@ -141,7 +144,12 @@ class AddNearbyContactPermissionManager { builder.setPositiveButton(R.string.permission_location_setting_button, (dialog, which) -> { Intent i = new Intent(ACTION_LOCATION_SOURCE_SETTINGS); - ctx.startActivity(i); + try { + ctx.startActivity(i); + } catch (ActivityNotFoundException e) { + Toast.makeText(ctx, R.string.error_start_activity, + LENGTH_LONG).show(); + } }); builder.show(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactViewModel.java index 5dc2da575..eca13d40d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/nearby/AddNearbyContactViewModel.java @@ -65,6 +65,7 @@ import javax.inject.Provider; import androidx.annotation.Nullable; import androidx.annotation.UiThread; +import androidx.core.util.Supplier; import androidx.lifecycle.AndroidViewModel; import androidx.lifecycle.LiveData; import androidx.lifecycle.MutableLiveData; @@ -135,8 +136,6 @@ class AddNearbyContactViewModel extends AndroidViewModel private final ContactExchangeManager contactExchangeManager; private final ConnectionManager connectionManager; - private final MutableLiveEvent checkPermissions = - new MutableLiveEvent<>(); private final MutableLiveEvent requestBluetoothDiscoverable = new MutableLiveEvent<>(); private final MutableLiveEvent showQrCodeFragment = @@ -144,8 +143,9 @@ class AddNearbyContactViewModel extends AndroidViewModel private final MutableLiveData state = new MutableLiveData<>(); - final QrCodeDecoder qrCodeDecoder; - final BroadcastReceiver bluetoothReceiver = new BluetoothStateReceiver(); + private final QrCodeDecoder qrCodeDecoder; + private final BroadcastReceiver bluetoothReceiver = + new BluetoothStateReceiver(); @Nullable private final BluetoothAdapter bt; @@ -171,7 +171,7 @@ class AddNearbyContactViewModel extends AndroidViewModel private boolean hasEnabledBluetooth = false; @Nullable - private KeyAgreementTask task; + private volatile KeyAgreementTask task; private volatile boolean gotLocalPayload = false, gotRemotePayload = false; @Inject @@ -213,13 +213,12 @@ class AddNearbyContactViewModel extends AndroidViewModel } @UiThread - void onContinueClicked() { + void onContinueClicked(Supplier checkPermissions) { if (bluetoothDecision == REFUSED) { bluetoothDecision = UNKNOWN; // Ask again } wasContinueClicked = true; - checkPermissions.setEvent(true); - showQrCodeFragmentIfAllowed(); + if (checkPermissions.get()) showQrCodeFragmentIfAllowed(); } @UiThread @@ -228,7 +227,7 @@ class AddNearbyContactViewModel extends AndroidViewModel } @UiThread - boolean isWifiReady() { + private boolean isWifiReady() { if (wifiPlugin == null) return true; // Continue without wifi State state = wifiPlugin.getState(); // Wait for plugin to become enabled @@ -236,7 +235,7 @@ class AddNearbyContactViewModel extends AndroidViewModel } @UiThread - boolean isBluetoothReady() { + private boolean isBluetoothReady() { if (bt == null || bluetoothPlugin == null) { // Continue without Bluetooth return true; @@ -256,7 +255,7 @@ class AddNearbyContactViewModel extends AndroidViewModel } @UiThread - void enableWifiIfWeShould() { + private void enableWifiIfWeShould() { if (hasEnabledWifi) return; if (wifiPlugin == null) return; State state = wifiPlugin.getState(); @@ -268,7 +267,7 @@ class AddNearbyContactViewModel extends AndroidViewModel } @UiThread - void enableBluetoothIfWeShould() { + private void enableBluetoothIfWeShould() { if (bluetoothDecision != BluetoothDecision.ACCEPTED) return; if (hasEnabledBluetooth) return; if (bluetoothPlugin == null || !isBluetoothSupported()) return; @@ -281,7 +280,7 @@ class AddNearbyContactViewModel extends AndroidViewModel } @UiThread - void startAddingContact() { + private void startAddingContact() { // If we return to the intro fragment, the continue button needs to be // clicked again before showing the QR code fragment wasContinueClicked = false; @@ -292,6 +291,8 @@ class AddNearbyContactViewModel extends AndroidViewModel // Bluetooth again hasEnabledWifi = false; hasEnabledBluetooth = false; + // reset state, so we don't show an old QR code again + state.setValue(null); // start to listen with a KeyAgreementTask startListening(); showQrCodeFragment.setEvent(true); @@ -509,8 +510,8 @@ class AddNearbyContactViewModel extends AndroidViewModel showQrCodeFragmentIfAllowed(); } - LiveEvent getCheckPermissions() { - return checkPermissions; + QrCodeDecoder getQrCodeDecoder() { + return qrCodeDecoder; } LiveEvent getRequestBluetoothDiscoverable() { @@ -521,6 +522,9 @@ class AddNearbyContactViewModel extends AndroidViewModel return showQrCodeFragment; } + /** + * This LiveData will be null initially. + */ LiveData getState() { return state; }