Verified Commit 7bcfc113 authored by Torsten Grote's avatar Torsten Grote
Browse files

Address review feedback for feature branch

parent e9dbceef
Pipeline #7171 passed with stages
in 11 minutes and 54 seconds
......@@ -92,9 +92,9 @@
<li id="troubleshooting_1">If you can't download the app, try it with a different web
browser app.
</li>
<li id="troubleshooting_2">Ensure that your browser is allowed to download apps directly by
giving it the permission or enabling the installation of apps from "Unknown Sources" in
system settings.
<li id="troubleshooting_2">To install the downloaded app,
you might need to allow your browser to install unknown apps.
We recommend to undo that after successful installation.
</li>
</ol>
</div>
......
......@@ -61,15 +61,14 @@ class ConditionManager29 extends AbstractConditionManager {
}
private boolean areEssentialPermissionsGranted() {
boolean isWifiEnabled = wifiManager.isWifiEnabled();
if (LOG.isLoggable(INFO)) {
LOG.info(String.format("areEssentialPermissionsGranted(): " +
"locationPermission? %s, " +
"wifiManager.isWifiEnabled()? %b",
locationPermission,
wifiManager.isWifiEnabled()));
locationPermission, isWifiEnabled));
}
return locationPermission == Permission.GRANTED &&
wifiManager.isWifiEnabled();
return locationPermission == Permission.GRANTED && isWifiEnabled;
}
@Override
......
......@@ -15,6 +15,7 @@ import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
import org.briarproject.briar.R;
import org.briarproject.briar.android.fragment.BaseFragment;
import org.briarproject.briar.android.util.ActivityLaunchers.CreateDocumentAdvanced;
import java.util.List;
......@@ -22,7 +23,6 @@ import javax.inject.Inject;
import androidx.activity.result.ActivityResultLauncher;
import androidx.annotation.Nullable;
import androidx.fragment.app.Fragment;
import androidx.fragment.app.FragmentActivity;
import androidx.lifecycle.ViewModelProvider;
......@@ -33,7 +33,6 @@ import static android.content.pm.PackageManager.MATCH_DEFAULT_ONLY;
import static android.os.Build.VERSION.SDK_INT;
import static android.view.View.INVISIBLE;
import static android.view.View.VISIBLE;
import static androidx.activity.result.contract.ActivityResultContracts.CreateDocument;
import static androidx.transition.TransitionManager.beginDelayedTransition;
import static org.briarproject.briar.android.AppModule.getAndroidComponent;
import static org.briarproject.briar.android.hotspot.HotspotViewModel.getApkFileName;
......@@ -50,7 +49,7 @@ public class FallbackFragment extends BaseFragment {
private HotspotViewModel viewModel;
private final ActivityResultLauncher<String> launcher =
registerForActivityResult(new CreateDocument(),
registerForActivityResult(new CreateDocumentAdvanced(),
this::onDocumentCreated);
private Button fallbackButton;
private ProgressBar progressBar;
......@@ -92,8 +91,7 @@ public class FallbackFragment extends BaseFragment {
if (SDK_INT >= 19) launcher.launch(getApkFileName());
else viewModel.exportApk();
});
viewModel.getSavedApkToUri()
.observeEvent(this, uri -> shareUri(this, uri));
viewModel.getSavedApkToUri().observeEvent(this, this::shareUri);
}
private void onDocumentCreated(@Nullable Uri uri) {
......@@ -107,12 +105,12 @@ public class FallbackFragment extends BaseFragment {
progressBar.setVisibility(INVISIBLE);
}
static void shareUri(Fragment fragment, Uri uri) {
void shareUri(Uri uri) {
Intent i = new Intent(ACTION_SEND);
i.putExtra(EXTRA_STREAM, uri);
i.setType("*/*"); // gives us all sharing options
i.addFlags(FLAG_GRANT_READ_URI_PERMISSION);
Context ctx = fragment.requireContext();
Context ctx = requireContext();
if (SDK_INT <= 19) {
// Workaround for Android bug:
// ctx.grantUriPermission also needed for Android 4
......@@ -124,7 +122,7 @@ public class FallbackFragment extends BaseFragment {
FLAG_GRANT_READ_URI_PERMISSION);
}
}
fragment.startActivity(Intent.createChooser(i, null));
startActivity(Intent.createChooser(i, null));
}
}
......@@ -59,12 +59,12 @@ public class HotspotActivity extends BriarActivity
// check if fragment is already added
// to not lose state on configuration changes
if (fm.findFragmentByTag(tag) == null) {
if (!started.consume()) {
if (started.wasNotYetConsumed()) {
showFragment(fm, new HotspotFragment(), tag);
}
}
} else if (hotspotState instanceof HotspotError) {
HotspotError error = ((HotspotError) hotspotState);
HotspotError error = (HotspotError) hotspotState;
showErrorFragment(error.getError());
}
});
......
......@@ -2,13 +2,11 @@ package org.briarproject.briar.android.hotspot;
import android.os.Bundle;
import android.view.View;
import com.google.android.material.snackbar.Snackbar;
import android.widget.Toast;
import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
import org.briarproject.briar.R;
import org.briarproject.briar.android.util.BriarSnackbarBuilder;
import androidx.annotation.Nullable;
import androidx.fragment.app.Fragment;
......@@ -41,13 +39,8 @@ public class HotspotFragment extends AbstractTabsFragment {
private void onPeerConnected(boolean connected) {
if (!connected) return;
new BriarSnackbarBuilder()
.setAction(R.string.hotspot_peer_connected_action, v ->
showNextFragment())
.make(connectedButton, R.string.hotspot_peer_connected,
Snackbar.LENGTH_LONG)
.setAnchorView(connectedButton)
.show();
Toast.makeText(requireContext(), R.string.hotspot_peer_connected,
Toast.LENGTH_LONG).show();
}
private void showNextFragment() {
......
......@@ -48,7 +48,9 @@ import static android.net.wifi.p2p.WifiP2pManager.NO_SERVICE_REQUESTS;
import static android.net.wifi.p2p.WifiP2pManager.P2P_UNSUPPORTED;
import static android.os.Build.VERSION.SDK_INT;
import static android.os.PowerManager.FULL_WAKE_LOCK;
import static java.util.Objects.requireNonNull;
import static java.util.logging.Level.INFO;
import static java.util.logging.Level.WARNING;
import static java.util.logging.Logger.getLogger;
import static org.briarproject.briar.android.util.UiUtils.handleException;
......@@ -57,6 +59,7 @@ import static org.briarproject.briar.android.util.UiUtils.handleException;
class HotspotManager {
interface HotspotListener {
@UiThread
void onStartingHotspot();
@IoExecutor
......@@ -65,8 +68,7 @@ class HotspotManager {
@UiThread
void onDeviceConnected();
void onHotspotStopped();
@UiThread
void onHotspotError(String error);
}
......@@ -97,8 +99,9 @@ class HotspotManager {
private WifiManager.WifiLock wifiLock;
private PowerManager.WakeLock wakeLock;
private WifiP2pManager.Channel channel;
@Nullable
@RequiresApi(29)
private volatile NetworkConfig savedNetworkConfig;
private volatile NetworkConfig savedNetworkConfig = null;
@Inject
HotspotManager(Application ctx,
......@@ -157,9 +160,11 @@ class HotspotManager {
* We'll realize that the framework is busy when the ActionListener passed
* to {@link WifiP2pManager#createGroup} is called with onFailure(BUSY)
*/
@UiThread
private void startWifiP2pFramework(int attempt) {
if (LOG.isLoggable(INFO))
if (LOG.isLoggable(INFO)) {
LOG.info("startWifiP2pFramework attempt: " + attempt);
}
/*
* It is important that we call WifiP2pManager#initialize again
* for every attempt to starting the framework because otherwise,
......@@ -167,13 +172,12 @@ class HotspotManager {
*/
channel = wifiP2pManager.initialize(ctx, ctx.getMainLooper(), null);
if (channel == null) {
listener.onHotspotError(
releaseHotspotWithError(
ctx.getString(R.string.hotspot_error_no_wifi_direct));
return;
}
ActionListener listener = new ActionListener() {
@Override
// Callback for wifiP2pManager#createGroup() during startWifiP2pHotspot()
public void onSuccess() {
......@@ -183,7 +187,9 @@ class HotspotManager {
@Override
// Callback for wifiP2pManager#createGroup() during startWifiP2pHotspot()
public void onFailure(int reason) {
LOG.info("onFailure: " + reason);
if (LOG.isLoggable(INFO)) {
LOG.info("onFailure: " + reason);
}
if (reason == BUSY) {
// WifiP2p not ready yet or hotspot already running
restartWifiP2pFramework(attempt);
......@@ -210,21 +216,26 @@ class HotspotManager {
try {
if (SDK_INT >= 29) {
dbExecutor.execute(() -> {
Runnable createGroup = () -> {
NetworkConfig c = requireNonNull(savedNetworkConfig);
WifiP2pConfig config = new WifiP2pConfig.Builder()
.setGroupOperatingBand(GROUP_OWNER_BAND_2GHZ)
.setNetworkName(c.ssid)
.setPassphrase(c.password)
.build();
wifiP2pManager.createGroup(channel, config, listener);
};
if (savedNetworkConfig == null) {
// load savedNetworkConfig before starting hotspot
loadSavedNetworkConfig();
androidExecutor.runOnUiThread(() -> {
WifiP2pConfig config = new WifiP2pConfig.Builder()
.setGroupOperatingBand(GROUP_OWNER_BAND_2GHZ)
.setNetworkName(savedNetworkConfig.ssid)
.setPassphrase(savedNetworkConfig.password)
.build();
acquireLocks();
wifiP2pManager.createGroup(channel, config, listener);
dbExecutor.execute(() -> {
loadSavedNetworkConfig();
androidExecutor.runOnUiThread(createGroup);
});
});
} else {
// savedNetworkConfig was already loaded, create group now
createGroup.run();
}
} else {
acquireLocks();
wifiP2pManager.createGroup(channel, listener);
}
} catch (SecurityException e) {
......@@ -233,9 +244,11 @@ class HotspotManager {
}
}
@UiThread
private void restartWifiP2pFramework(int attempt) {
LOG.info("retrying to start WifiP2p framework");
if (attempt < MAX_FRAMEWORK_ATTEMPTS) {
if (SDK_INT >= 27) channel.close();
handler.postDelayed(() -> startWifiP2pFramework(attempt + 1),
RETRY_DELAY_MILLIS);
} else {
......@@ -250,19 +263,23 @@ class HotspotManager {
wifiP2pManager.removeGroup(channel, new ActionListener() {
@Override
public void onSuccess() {
releaseHotspot();
closeChannelAndReleaseLocks();
}
@Override
public void onFailure(int reason) {
// not propagating back error
releaseHotspot();
if (LOG.isLoggable(WARNING)) {
LOG.warning("Error removing Wifi P2P group: " + reason);
}
closeChannelAndReleaseLocks();
}
});
}
@SuppressLint("WakelockTimeout")
private void acquireLocks() {
// FLAG_KEEP_SCREEN_ON is not respected on some Huawei devices.
wakeLock = powerManager.newWakeLock(FULL_WAKE_LOCK, lockTag);
wakeLock.acquire();
// WIFI_MODE_FULL has no effect on API >= 29
......@@ -272,23 +289,21 @@ class HotspotManager {
wifiLock.acquire();
}
private void releaseHotspot() {
listener.onHotspotStopped();
closeChannelAndReleaseLocks();
}
@UiThread
private void releaseHotspotWithError(String error) {
listener.onHotspotError(error);
closeChannelAndReleaseLocks();
}
@UiThread
private void closeChannelAndReleaseLocks() {
if (SDK_INT >= 27) channel.close();
if (SDK_INT >= 27 && channel != null) channel.close();
channel = null;
wakeLock.release();
wifiLock.release();
if (wakeLock.isHeld()) wakeLock.release();
if (wifiLock.isHeld()) wifiLock.release();
}
@UiThread
private void requestGroupInfo(int attempt) {
if (LOG.isLoggable(INFO)) {
LOG.info("requestGroupInfo attempt: " + attempt);
......@@ -331,25 +346,20 @@ class HotspotManager {
LOG.info("group is null");
return false;
} else if (!group.getNetworkName().startsWith("DIRECT-")) {
if (LOG.isLoggable(INFO)) {
LOG.info("received networkName without prefix 'DIRECT-': " +
group.getNetworkName());
}
LOG.info("received networkName without prefix 'DIRECT-'");
return false;
} else if (SDK_INT >= 29) {
// if we get here, the savedNetworkConfig must have a value
String networkName = savedNetworkConfig.ssid;
String networkName = requireNonNull(savedNetworkConfig).ssid;
if (!networkName.equals(group.getNetworkName())) {
if (LOG.isLoggable(INFO)) {
LOG.info("expected networkName: " + networkName);
LOG.info("received networkName: " + group.getNetworkName());
}
LOG.info("expected networkName does not match received one");
return false;
}
}
return true;
}
@UiThread
private void retryRequestingGroupInfo(int attempt) {
LOG.info("retrying to request group info");
// On some devices we need to wait for the group info to become available
......@@ -364,17 +374,12 @@ class HotspotManager {
@UiThread
private void requestGroupInfoForConnection() {
if (LOG.isLoggable(INFO)) {
LOG.info("requestGroupInfo for connection");
}
LOG.info("requestGroupInfo for connection");
GroupInfoListener groupListener = group -> {
if (group == null || group.getClientList().isEmpty()) {
handler.postDelayed(this::requestGroupInfoForConnection,
RETRY_DELAY_MILLIS);
} else {
if (LOG.isLoggable(INFO)) {
LOG.info("client list " + group.getClientList());
}
listener.onDeviceConnected();
}
};
......@@ -433,26 +438,15 @@ class HotspotManager {
}
// exclude chars that are easy to confuse: 0 O, 5 S, 1 l I
private static final String digits = "2346789";
private static final String letters = "abcdefghijkmnopqrstuvwxyz";
private static final String LETTERS = "ABCDEFGHJKLMNPQRTUVWXYZ";
private static final String chars =
"2346789ABCDEFGHJKLMNPQRTUVWXYZabcdefghijkmnopqrstuvwxyz";
private String getRandomString(int length) {
char[] c = new char[length];
for (int i = 0; i < length; i++) {
if (random.nextBoolean()) {
c[i] = random(digits);
} else if (random.nextBoolean()) {
c[i] = random(letters);
} else {
c[i] = random(LETTERS);
}
c[i] = chars.charAt(random.nextInt(chars.length()));
}
return new String(c);
}
private char random(String universe) {
return universe.charAt(random.nextInt(universe.length()));
}
}
......@@ -63,10 +63,10 @@ abstract class HotspotState {
* to not repeat actions such as showing fragments on rotation changes.
*/
@UiThread
boolean consume() {
boolean wasNotYetConsumed() {
boolean old = consumed;
consumed = true;
return old;
return !old;
}
}
......
......@@ -141,12 +141,6 @@ class HotspotViewModel extends DbViewModel
peerConnected.setEvent(true);
}
@Override
public void onHotspotStopped() {
LOG.info("stopping webserver");
ioExecutor.execute(webServerManager::stopWebServer);
}
@Override
public void onHotspotError(String error) {
if (LOG.isLoggable(WARNING)) {
......@@ -170,7 +164,7 @@ class HotspotViewModel extends DbViewModel
public void onWebServerError() {
state.postValue(new HotspotError(getApplication()
.getString(R.string.hotspot_error_web_server_start)));
hotspotManager.stopWifiP2pHotspot();
stopHotspot();
}
void exportApk(Uri uri) {
......
package org.briarproject.briar.android.hotspot;
import android.content.Context;
import android.graphics.Bitmap;
import android.os.Bundle;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageView;
import android.widget.TextView;
import android.widget.Toast;
import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
......@@ -16,7 +18,6 @@ import org.briarproject.briar.android.hotspot.HotspotState.HotspotStarted;
import javax.inject.Inject;
import androidx.annotation.Nullable;
import androidx.core.util.Consumer;
import androidx.fragment.app.Fragment;
import androidx.lifecycle.ViewModelProvider;
......@@ -60,19 +61,23 @@ public class QrHotspotFragment extends Fragment {
TextView qrIntroView = v.findViewById(R.id.qrIntroView);
ImageView qrCodeView = v.findViewById(R.id.qrCodeView);
Consumer<HotspotStarted> consumer;
if (requireArguments().getBoolean(ARG_FOR_WIFI_CONNECT)) {
qrIntroView.setText(R.string.hotspot_qr_wifi);
consumer = state ->
qrCodeView.setImageBitmap(state.getNetworkConfig().qrCode);
} else {
qrIntroView.setText(R.string.hotspot_qr_site);
consumer = state ->
qrCodeView.setImageBitmap(state.getWebsiteConfig().qrCode);
}
boolean forWifi = requireArguments().getBoolean(ARG_FOR_WIFI_CONNECT);
qrIntroView.setText(forWifi ? R.string.hotspot_qr_wifi :
R.string.hotspot_qr_site);
viewModel.getState().observe(getViewLifecycleOwner(), state -> {
if (state instanceof HotspotStarted) {
consumer.accept((HotspotStarted) state);
HotspotStarted s = (HotspotStarted) state;
Bitmap qrCode = forWifi ? s.getNetworkConfig().qrCode :
s.getWebsiteConfig().qrCode;
if (qrCode == null) {
Toast.makeText(requireContext(), R.string.error,
Toast.LENGTH_SHORT).show();
qrCodeView.setImageResource(R.drawable.ic_image_broken);
} else {
qrCodeView.setImageBitmap(qrCode);
}
}
});
return v;
......
......@@ -718,7 +718,6 @@
<string name="hotspot_manual_wifi_ssid">Network name (SSID)</string>
<string name="hotspot_qr_wifi">Your phone is providing a Wi-Fi hotspot. People who want to download Briar can connect to the hotspot by scanning this QR code. When they have connected to the hotspot, press \'Next\'.</string>
<string name="hotspot_peer_connected">Successfully connected</string>
<string name="hotspot_peer_connected_action">Show download info</string>
<!-- Download link -->
<!-- The %s placeholder will be replaced with the translation of 'hotspot_scanning_a_qr_code' -->
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment