Commit f7d18c0f authored by ortuno's avatar ortuno Committed by Commit bot

bluetooth: Fix crash when accepting location permission

http://crrev.com/2369673003 introduced code to close the chooser when the
window looses focus. This works great for when the user switches apps but
it sadly causes the chooser to get closed when the permission prompt opens
up. Then when the user accepts the permission request chrome crashes
because the chooser was closed.

Checks if the chooser is still opened before restarting the scan.

Changes the testing framework to simulate a focus change when requesting
permission and adds asserts to make sure native functions are not being
called after the native object has been destroyed.

BUG=661770

Review-Url: https://codereview.chromium.org/2476563002
Cr-Commit-Position: refs/heads/master@{#430197}
parent 549e6180
......@@ -185,6 +185,11 @@ public class BluetoothChooserDialog
@Override
public void onRequestPermissionsResult(String[] permissions, int[] grantResults) {
// The chooser might have been closed during the request.
if (mNativeBluetoothChooserDialogPtr == 0) {
return;
}
for (int i = 0; i < permissions.length; i++) {
if (permissions[i].equals(Manifest.permission.ACCESS_COARSE_LOCATION)) {
if (checkLocationServicesAndPermission()) {
......
......@@ -64,17 +64,27 @@ public class BluetoothChooserDialogTest extends ChromeActivityTestCaseBase<Chrom
@Override
void nativeRestartSearch(long nativeBluetoothChooserAndroid) {
assertTrue(mNativeBluetoothChooserDialogPtr != 0);
mRestartSearchCount++;
}
@Override
void nativeShowBluetoothOverviewLink(long nativeBluetoothChooserAndroid) {}
void nativeShowBluetoothOverviewLink(long nativeBluetoothChooserAndroid) {
// We shouldn't be running native functions if the native class has been destroyed.
assertTrue(mNativeBluetoothChooserDialogPtr != 0);
}
@Override
void nativeShowBluetoothAdapterOffLink(long nativeBluetoothChooserAndroid) {}
void nativeShowBluetoothAdapterOffLink(long nativeBluetoothChooserAndroid) {
// We shouldn't be running native functions if the native class has been destroyed.
assertTrue(mNativeBluetoothChooserDialogPtr != 0);
}
@Override
void nativeShowNeedLocationPermissionLink(long nativeBluetoothChooserAndroid) {}
void nativeShowNeedLocationPermissionLink(long nativeBluetoothChooserAndroid) {
// We shouldn't be running native functions if the native class has been destroyed.
assertTrue(mNativeBluetoothChooserDialogPtr != 0);
}
}
private ActivityWindowAndroid mWindowAndroid;
......@@ -253,7 +263,7 @@ public class BluetoothChooserDialogTest extends ChromeActivityTestCaseBase<Chrom
final View progress = dialog.findViewById(R.id.progress);
final TestAndroidPermissionDelegate permissionDelegate =
new TestAndroidPermissionDelegate();
new TestAndroidPermissionDelegate(dialog);
mWindowAndroid.setAndroidPermissionDelegate(permissionDelegate);
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
......@@ -296,11 +306,16 @@ public class BluetoothChooserDialogTest extends ChromeActivityTestCaseBase<Chrom
}
});
assertEquals(1, mChooserDialog.mRestartSearchCount);
assertEquals(removeLinkTags(getActivity().getString(R.string.bluetooth_searching)),
statusView.getText().toString());
// TODO(661862): Remove once the dialog no longer closes when the window loses
// focus.
assertTrue(mChooserDialog.mFinishedEventType != -1);
mChooserDialog.closeDialog();
// TODO(661862): Uncomment once the dialog no longer closes when the window
// loses focus.
// assertEquals(1, mChooserDialog.mRestartSearchCount);
// assertEquals(removeLinkTags(getActivity().getString(R.string.bluetooth_searching)),
// statusView.getText().toString());
// mChooserDialog.closeDialog();
}
@LargeTest
......@@ -318,7 +333,7 @@ public class BluetoothChooserDialogTest extends ChromeActivityTestCaseBase<Chrom
final View progress = dialog.findViewById(R.id.progress);
final TestAndroidPermissionDelegate permissionDelegate =
new TestAndroidPermissionDelegate();
new TestAndroidPermissionDelegate(dialog);
mWindowAndroid.setAndroidPermissionDelegate(permissionDelegate);
// Grant permissions, and turn off location services.
......@@ -364,9 +379,14 @@ public class BluetoothChooserDialogTest extends ChromeActivityTestCaseBase<Chrom
// TODO(jyasskin): Test when the user denies Chrome the ability to ask for permission.
private static class TestAndroidPermissionDelegate implements AndroidPermissionDelegate {
Dialog mDialog = null;
PermissionCallback mCallback = null;
String[] mPermissionsRequested = null;
public TestAndroidPermissionDelegate(Dialog dialog) {
mDialog = dialog;
}
@Override
public boolean hasPermission(String permission) {
return false;
......@@ -376,12 +396,16 @@ public class BluetoothChooserDialogTest extends ChromeActivityTestCaseBase<Chrom
public boolean canRequestPermission(String permission) {
return true;
}
@Override
public boolean isPermissionRevokedByPolicy(String permission) {
return false;
}
@Override
public void requestPermissions(String[] permissions, PermissionCallback callback) {
// Requesting for permission takes away focus from the window.
mDialog.onWindowFocusChanged(false /* hasFocus */);
mPermissionsRequested = permissions;
if (permissions.length == 1
&& permissions[0].equals(Manifest.permission.ACCESS_COARSE_LOCATION)) {
......
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