One more round of addressing AddNearbyContact review feedback

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<String[]> permissionLauncher =
-			registerForActivityResult(new RequestMultiplePermissions(), r ->
-					permissionManager.onRequestPermissionResult(r,
-							viewModel::showQrCodeFragmentIfAllowed));
 	private final ActivityResultLauncher<Integer> 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;

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;

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<String[]> 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));
 	}
 

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<String[]> requestPermissions;
 	private final boolean isBluetoothSupported;
 
-	AddNearbyContactPermissionManager(BaseActivity ctx,
+	AddNearbyContactPermissionManager(FragmentActivity ctx,
 			Consumer<String[]> 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();
 	}

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<Boolean> checkPermissions =
-			new MutableLiveEvent<>();
 	private final MutableLiveEvent<Boolean> requestBluetoothDiscoverable =
 			new MutableLiveEvent<>();
 	private final MutableLiveEvent<Boolean> showQrCodeFragment =
@@ -144,8 +143,9 @@ class AddNearbyContactViewModel extends AndroidViewModel
 	private final MutableLiveData<AddContactState> state =
 			new MutableLiveData<>();
 
-	final QrCodeDecoder qrCodeDecoder;
+	private final QrCodeDecoder qrCodeDecoder;
-	final BroadcastReceiver bluetoothReceiver = new BluetoothStateReceiver();
+	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<Boolean> checkPermissions) {
 		if (bluetoothDecision == REFUSED) {
 			bluetoothDecision = UNKNOWN; // Ask again
 		}
 		wasContinueClicked = true;
-		checkPermissions.setEvent(true);
+		if (checkPermissions.get()) showQrCodeFragmentIfAllowed();
-		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<Boolean> getCheckPermissions() {
+	QrCodeDecoder getQrCodeDecoder() {
-		return checkPermissions;
+		return qrCodeDecoder;
 	}
 
 	LiveEvent<Boolean> getRequestBluetoothDiscoverable() {
@@ -521,6 +522,9 @@ class AddNearbyContactViewModel extends AndroidViewModel
 		return showQrCodeFragment;
 	}
 
+	/**
+	 * This LiveData will be null initially.
+	 */
 	LiveData<AddContactState> getState() {
 		return state;
 	}