From 28e8a498f4348d26ddcc07a341591b0183c4bdd6 Mon Sep 17 00:00:00 2001 From: perja Date: Mon, 8 Feb 2016 10:39:12 -0800 Subject: bluetooth: android: Fix a couple of crashes when adapter is turned on/off. These changes fixes the crashes found when toggling the adapter on/off when device chooser dialog is used in web-bluetooth. BUG=570610 Review URL: https://codereview.chromium.org/1610053005 Cr-Commit-Position: refs/heads/master@{#374145} --- .../device/bluetooth/ChromeBluetoothAdapter.java | 29 +++++++- .../org/chromium/device/bluetooth/Wrappers.java | 49 +++++++++---- device/bluetooth/bluetooth_adapter_android.cc | 7 +- device/bluetooth/bluetooth_adapter_unittest.cc | 85 ++++++++++++++++++++++ .../src/org/chromium/device/bluetooth/Fakes.java | 51 ++++++++++++- device/bluetooth/test/bluetooth_test_android.cc | 5 ++ device/bluetooth/test/bluetooth_test_android.h | 4 + 7 files changed, 207 insertions(+), 23 deletions(-) diff --git a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java index 0774ea2..0a54a0b 100644 --- a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java +++ b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java @@ -111,6 +111,16 @@ final class ChromeBluetoothAdapter { return isPresent() && mAdapter.isEnabled(); } + // Implements BluetoothAdapterAndroid::SetPowered. + @CalledByNative + private boolean setPowered(boolean powered) { + if (powered) { + return isPresent() && mAdapter.enable(); + } else { + return isPresent() && mAdapter.disable(); + } + } + // Implements BluetoothAdapterAndroid::IsDiscoverable. @CalledByNative private boolean isDiscoverable() { @@ -184,6 +194,10 @@ final class ChromeBluetoothAdapter { private boolean startScan() { Wrappers.BluetoothLeScannerWrapper scanner = mAdapter.getBluetoothLeScanner(); + if (scanner == null) { + return false; + } + if (!canScan()) { return false; } @@ -199,6 +213,11 @@ final class ChromeBluetoothAdapter { scanner.startScan(null /* filters */, scanMode, mScanCallback); } catch (IllegalArgumentException e) { Log.e(TAG, "Cannot start scan: " + e); + mScanCallback = null; + return false; + } catch (IllegalStateException e) { + Log.e(TAG, "Adapter is off. Cannot start scan: " + e); + mScanCallback = null; return false; } return true; @@ -212,12 +231,16 @@ final class ChromeBluetoothAdapter { if (mScanCallback == null) { return false; } + try { - mAdapter.getBluetoothLeScanner().stopScan(mScanCallback); + Wrappers.BluetoothLeScannerWrapper scanner = mAdapter.getBluetoothLeScanner(); + if (scanner != null) { + scanner.stopScan(mScanCallback); + } } catch (IllegalArgumentException e) { Log.e(TAG, "Cannot stop scan: " + e); - mScanCallback = null; - return false; + } catch (IllegalStateException e) { + Log.e(TAG, "Adapter is off. Cannot stop scan: " + e); } mScanCallback = null; return true; diff --git a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/Wrappers.java b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/Wrappers.java index 2ccdaae..9287f85 100644 --- a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/Wrappers.java +++ b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/Wrappers.java @@ -46,13 +46,15 @@ import java.util.UUID; class Wrappers { private static final String TAG = "Bluetooth"; + public static final int DEVICE_CLASS_UNSPECIFIED = 0x1F00; + /** * Wraps android.bluetooth.BluetoothAdapter. */ static class BluetoothAdapterWrapper { private final BluetoothAdapter mAdapter; protected final ContextWrapper mContext; - protected final BluetoothLeScannerWrapper mScanner; + protected BluetoothLeScannerWrapper mScannerWrapper; /** * Creates a BluetoothAdapterWrapper using the default @@ -94,34 +96,42 @@ class Wrappers { Log.i(TAG, "BluetoothAdapterWrapper.create failed: Default adapter not found."); return null; } else { - return new BluetoothAdapterWrapper(adapter, new ContextWrapper(context), - new BluetoothLeScannerWrapper(adapter.getBluetoothLeScanner())); + return new BluetoothAdapterWrapper(adapter, new ContextWrapper(context)); } } - public BluetoothAdapterWrapper(BluetoothAdapter adapter, ContextWrapper context, - BluetoothLeScannerWrapper scanner) { + public BluetoothAdapterWrapper(BluetoothAdapter adapter, ContextWrapper context) { mAdapter = adapter; mContext = context; - mScanner = scanner; - } - - public ContextWrapper getContext() { - return mContext; } - public BluetoothLeScannerWrapper getBluetoothLeScanner() { - return mScanner; + public boolean disable() { + return mAdapter.disable(); } - public boolean isEnabled() { - return mAdapter.isEnabled(); + public boolean enable() { + return mAdapter.enable(); } public String getAddress() { return mAdapter.getAddress(); } + public BluetoothLeScannerWrapper getBluetoothLeScanner() { + BluetoothLeScanner scanner = mAdapter.getBluetoothLeScanner(); + if (scanner == null) { + return null; + } + if (mScannerWrapper == null) { + mScannerWrapper = new BluetoothLeScannerWrapper(scanner); + } + return mScannerWrapper; + } + + public ContextWrapper getContext() { + return mContext; + } + public String getName() { return mAdapter.getName(); } @@ -133,6 +143,10 @@ class Wrappers { public boolean isDiscovering() { return mAdapter.isDiscovering(); } + + public boolean isEnabled() { + return mAdapter.isEnabled(); + } } /** @@ -155,7 +169,7 @@ class Wrappers { * Wraps android.bluetooth.BluetoothLeScanner. */ static class BluetoothLeScannerWrapper { - private final BluetoothLeScanner mScanner; + protected final BluetoothLeScanner mScanner; private final HashMap mCallbacks; public BluetoothLeScannerWrapper(BluetoothLeScanner scanner) { @@ -276,6 +290,11 @@ class Wrappers { } public int getBluetoothClass_getDeviceClass() { + if (mDevice == null || mDevice.getBluetoothClass() == null) { + // BluetoothDevice.getBluetoothClass() returns null if adapter has been powered off. + // Return DEVICE_CLASS_UNSPECIFIED in these cases. + return DEVICE_CLASS_UNSPECIFIED; + } return mDevice.getBluetoothClass().getDeviceClass(); } diff --git a/device/bluetooth/bluetooth_adapter_android.cc b/device/bluetooth/bluetooth_adapter_android.cc index 144ad52..00cead7 100644 --- a/device/bluetooth/bluetooth_adapter_android.cc +++ b/device/bluetooth/bluetooth_adapter_android.cc @@ -77,7 +77,12 @@ bool BluetoothAdapterAndroid::IsPowered() const { void BluetoothAdapterAndroid::SetPowered(bool powered, const base::Closure& callback, const ErrorCallback& error_callback) { - NOTIMPLEMENTED(); + if (Java_ChromeBluetoothAdapter_setPowered(AttachCurrentThread(), + j_adapter_.obj(), powered)) { + callback.Run(); + } else { + error_callback.Run(); + } } bool BluetoothAdapterAndroid::IsDiscoverable() const { diff --git a/device/bluetooth/bluetooth_adapter_unittest.cc b/device/bluetooth/bluetooth_adapter_unittest.cc index 12351c9..45a5271 100644 --- a/device/bluetooth/bluetooth_adapter_unittest.cc +++ b/device/bluetooth/bluetooth_adapter_unittest.cc @@ -479,6 +479,43 @@ TEST_F(BluetoothTest, DiscoverySession) { } #endif // defined(OS_ANDROID) +// Android only: this test is specific for Android and should not be +// enabled for other platforms. +#if defined(OS_ANDROID) +TEST_F(BluetoothTest, AdapterIllegalStateBeforeStartScan) { + if (!PlatformSupportsLowEnergy()) { + LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test."; + return; + } + InitWithFakeAdapter(); + ForceIllegalStateException(); + StartLowEnergyDiscoverySessionExpectedToFail(); + EXPECT_EQ(0, callback_count_); + EXPECT_EQ(1, error_callback_count_); + EXPECT_FALSE(adapter_->IsDiscovering()); +} +#endif // defined(OS_ANDROID) + +// Android only: this test is specific for Android and should not be +// enabled for other platforms. +#if defined(OS_ANDROID) +TEST_F(BluetoothTest, AdapterIllegalStateBeforeStopScan) { + if (!PlatformSupportsLowEnergy()) { + LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test."; + return; + } + InitWithFakeAdapter(); + StartLowEnergyDiscoverySession(); + EXPECT_EQ(1, callback_count_); + EXPECT_EQ(0, error_callback_count_); + EXPECT_TRUE(adapter_->IsDiscovering()); + ForceIllegalStateException(); + discovery_sessions_[0]->Stop(GetCallback(Call::EXPECTED), + GetErrorCallback(Call::NOT_EXPECTED)); + EXPECT_FALSE(adapter_->IsDiscovering()); +} +#endif // defined(OS_ANDROID) + #if defined(OS_ANDROID) || defined(OS_MACOSX) // Checks that discovery fails (instead of hanging) when permissions are denied. TEST_F(BluetoothTest, NoPermissions) { @@ -616,4 +653,52 @@ TEST_F(BluetoothTest, DiscoverMultipleLowEnergyDevices) { } #endif // defined(OS_ANDROID) || defined(OS_MACOSX) +#if defined(OS_ANDROID) +TEST_F(BluetoothTest, TogglePowerFakeAdapter) { + InitWithFakeAdapter(); + ASSERT_TRUE(adapter_->IsPresent()); + ASSERT_TRUE(adapter_->IsPowered()); + + // Check if power can be turned off. + adapter_->SetPowered(false, GetCallback(Call::EXPECTED), + GetErrorCallback(Call::NOT_EXPECTED)); + EXPECT_FALSE(adapter_->IsPowered()); + + // Check if power can be turned on again. + adapter_->SetPowered(true, GetCallback(Call::EXPECTED), + GetErrorCallback(Call::NOT_EXPECTED)); + EXPECT_TRUE(adapter_->IsPowered()); +} +#endif // defined(OS_ANDROID) + +#if defined(OS_ANDROID) +TEST_F(BluetoothTest, TogglePowerBeforeScan) { + InitWithFakeAdapter(); + ASSERT_TRUE(adapter_->IsPresent()); + ASSERT_TRUE(adapter_->IsPowered()); + + // Turn off adapter. + adapter_->SetPowered(false, GetCallback(Call::EXPECTED), + GetErrorCallback(Call::NOT_EXPECTED)); + ASSERT_FALSE(adapter_->IsPowered()); + + // Try to perform a scan. + StartLowEnergyDiscoverySessionExpectedToFail(); + + // Turn on adapter. + adapter_->SetPowered(true, GetCallback(Call::EXPECTED), + GetErrorCallback(Call::NOT_EXPECTED)); + ASSERT_TRUE(adapter_->IsPowered()); + + // Try to perform a scan again. + ResetEventCounts(); + StartLowEnergyDiscoverySession(); + EXPECT_EQ(1, callback_count_); + EXPECT_EQ(0, error_callback_count_); + EXPECT_TRUE(adapter_->IsDiscovering()); + ASSERT_EQ((size_t)1, discovery_sessions_.size()); + EXPECT_TRUE(discovery_sessions_[0]->IsActive()); +} +#endif // defined(OS_ANDROID) + } // namespace device diff --git a/device/bluetooth/test/android/java/src/org/chromium/device/bluetooth/Fakes.java b/device/bluetooth/test/android/java/src/org/chromium/device/bluetooth/Fakes.java index 65705fa..538f618 100644 --- a/device/bluetooth/test/android/java/src/org/chromium/device/bluetooth/Fakes.java +++ b/device/bluetooth/test/android/java/src/org/chromium/device/bluetooth/Fakes.java @@ -43,6 +43,7 @@ class Fakes { static class FakeBluetoothAdapter extends Wrappers.BluetoothAdapterWrapper { private final FakeContext mFakeContext; private final FakeBluetoothLeScanner mFakeScanner; + private boolean mPowered = true; final long mNativeBluetoothTestAndroid; /** @@ -55,10 +56,10 @@ class Fakes { } private FakeBluetoothAdapter(long nativeBluetoothTestAndroid) { - super(null, new FakeContext(), new FakeBluetoothLeScanner()); + super(null, new FakeContext()); mNativeBluetoothTestAndroid = nativeBluetoothTestAndroid; mFakeContext = (FakeContext) mContext; - mFakeScanner = (FakeBluetoothLeScanner) mScanner; + mFakeScanner = new FakeBluetoothLeScanner(); } @CalledByNative("FakeBluetoothAdapter") @@ -71,6 +72,10 @@ class Fakes { */ @CalledByNative("FakeBluetoothAdapter") public void discoverLowEnergyDevice(int deviceOrdinal) { + if (mFakeScanner == null) { + return; + } + switch (deviceOrdinal) { case 1: { ArrayList uuids = new ArrayList(2); @@ -115,11 +120,25 @@ class Fakes { } } + @CalledByNative("FakeBluetoothAdapter") + public void forceIllegalStateException() { + if (mFakeScanner != null) { + mFakeScanner.forceIllegalStateException(); + } + } + // ----------------------------------------------------------------------------------------- // BluetoothAdapterWrapper overrides: @Override - public boolean isEnabled() { + public boolean disable() { + mPowered = false; + return true; + } + + @Override + public boolean enable() { + mPowered = true; return true; } @@ -129,6 +148,14 @@ class Fakes { } @Override + public Wrappers.BluetoothLeScannerWrapper getBluetoothLeScanner() { + if (isEnabled()) { + return mFakeScanner; + } + return null; + } + + @Override public String getName() { return "FakeBluetoothAdapter"; } @@ -139,6 +166,11 @@ class Fakes { } @Override + public boolean isEnabled() { + return mPowered; + } + + @Override public boolean isDiscovering() { return false; } @@ -166,6 +198,7 @@ class Fakes { */ static class FakeBluetoothLeScanner extends Wrappers.BluetoothLeScannerWrapper { public Wrappers.ScanCallbackWrapper mScanCallback; + private boolean mThrowException; private FakeBluetoothLeScanner() { super(null); @@ -178,6 +211,9 @@ class Fakes { throw new IllegalArgumentException( "FakeBluetoothLeScanner does not support multiple scans."); } + if (mThrowException) { + throw new IllegalStateException("Adapter is off."); + } mScanCallback = callback; } @@ -186,8 +222,15 @@ class Fakes { if (mScanCallback != callback) { throw new IllegalArgumentException("No scan in progress."); } + if (mThrowException) { + throw new IllegalStateException("Adapter is off."); + } mScanCallback = null; } + + void forceIllegalStateException() { + mThrowException = true; + } } /** @@ -288,7 +331,7 @@ class Fakes { @Override public int getBluetoothClass_getDeviceClass() { - return 0x1F00; // Unspecified Device Class + return Wrappers.DEVICE_CLASS_UNSPECIFIED; } @Override diff --git a/device/bluetooth/test/bluetooth_test_android.cc b/device/bluetooth/test/bluetooth_test_android.cc index b31f5df..43299bc 100644 --- a/device/bluetooth/test/bluetooth_test_android.cc +++ b/device/bluetooth/test/bluetooth_test_android.cc @@ -81,6 +81,11 @@ BluetoothDevice* BluetoothTestAndroid::DiscoverLowEnergyDevice( return observer.last_device(); } +void BluetoothTestAndroid::ForceIllegalStateException() { + Java_FakeBluetoothAdapter_forceIllegalStateException( + AttachCurrentThread(), j_fake_bluetooth_adapter_.obj()); +} + void BluetoothTestAndroid::SimulateGattConnection(BluetoothDevice* device) { BluetoothDeviceAndroid* device_android = static_cast(device); diff --git a/device/bluetooth/test/bluetooth_test_android.h b/device/bluetooth/test/bluetooth_test_android.h index 7f39099..3fb3a33 100644 --- a/device/bluetooth/test/bluetooth_test_android.h +++ b/device/bluetooth/test/bluetooth_test_android.h @@ -71,6 +71,10 @@ class BluetoothTestAndroid : public BluetoothTestBase { void SimulateGattDescriptorWriteWillFailSynchronouslyOnce( BluetoothGattDescriptor* descriptor) override; + // Instruct the fake adapter to throw an IllegalStateException for + // startScan and stopScan. + void ForceIllegalStateException(); + // Records that Java FakeBluetoothDevice connectGatt was called. void OnFakeBluetoothDeviceConnectGattCalled( JNIEnv* env, -- cgit v1.1