bluetooth: android: Crash fix: Check Characteristic isn't already destroyed.

Read and Write operations may not complete until after Chrome has destroyed
C++ and Java Chrome Characteristic objects, resulting in a null object when
looking up a Chrome Characteristic from the Android Wrapper.

BUG=559294

Review URL: https://codereview.chromium.org/1471173004

Cr-Commit-Position: refs/heads/master@{#363377}

7 files changed, 108 insertions, 15 deletions

diff --git a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java
index 6e20600..b11244e 100644
--- a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java
+++ b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java
@@ -186,10 +186,15 @@ final class ChromeBluetoothDevice {
             ThreadUtils.runOnUiThread(new Runnable() {
                 @Override
                 public void run() {
-                    ChromeBluetoothRemoteGattCharacteristic
-                            chromeBluetoothRemoteGattCharacteristic =
-                                    mWrapperToChromeCharacteristicsMap.get(characteristic);
-                    chromeBluetoothRemoteGattCharacteristic.onCharacteristicRead(status);
+                    ChromeBluetoothRemoteGattCharacteristic chromeCharacteristic =
+                            mWrapperToChromeCharacteristicsMap.get(characteristic);
+                    if (chromeCharacteristic == null) {
+                        // Android events arriving with no Chrome object is expected rarely: only
+                        // when the event races object destruction.
+                        Log.v(TAG, "onCharacteristicRead when chromeCharacteristic == null.");
+                    } else {
+                        chromeCharacteristic.onCharacteristicRead(status);
+                    }
                 }
             });
         }
@@ -201,10 +206,15 @@ final class ChromeBluetoothDevice {
             ThreadUtils.runOnUiThread(new Runnable() {
                 @Override
                 public void run() {
-                    ChromeBluetoothRemoteGattCharacteristic
-                            chromeBluetoothRemoteGattCharacteristic =
-                                    mWrapperToChromeCharacteristicsMap.get(characteristic);
-                    chromeBluetoothRemoteGattCharacteristic.onCharacteristicWrite(status);
+                    ChromeBluetoothRemoteGattCharacteristic chromeCharacteristic =
+                            mWrapperToChromeCharacteristicsMap.get(characteristic);
+                    if (chromeCharacteristic == null) {
+                        // Android events arriving with no Chrome object is expected rarely: only
+                        // when the event races object destruction.
+                        Log.v(TAG, "onCharacteristicWrite when chromeCharacteristic == null.");
+                    } else {
+                        chromeCharacteristic.onCharacteristicWrite(status);
+                    }
                 }
             });
         }
diff --git a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java
index dd0d261..3b64681 100644
--- a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java
+++ b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java
@@ -42,6 +42,7 @@ final class ChromeBluetoothRemoteGattCharacteristic {
      */
     @CalledByNative
     private void onBluetoothRemoteGattCharacteristicAndroidDestruction() {
+        Log.v(TAG, "ChromeBluetoothRemoteGattCharacteristic Destroyed.");
         mNativeBluetoothRemoteGattCharacteristicAndroid = 0;
         mChromeBluetoothDevice.mWrapperToChromeCharacteristicsMap.remove(mCharacteristic);
     }
diff --git a/device/bluetooth/bluetooth_gatt_characteristic_unittest.cc b/device/bluetooth/bluetooth_gatt_characteristic_unittest.cc
index 17e77e9..bbca500 100644
--- a/device/bluetooth/bluetooth_gatt_characteristic_unittest.cc
+++ b/device/bluetooth/bluetooth_gatt_characteristic_unittest.cc
@@ -214,6 +214,42 @@ TEST_F(BluetoothGattCharacteristicTest, WriteRemoteCharacteristic_Empty) {
 #endif  // defined(OS_ANDROID)
 
 #if defined(OS_ANDROID)
+// Tests ReadRemoteCharacteristic completing after Chrome objects are deleted.
+TEST_F(BluetoothGattCharacteristicTest, ReadRemoteCharacteristic_AfterDeleted) {
+  ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate());
+
+  characteristic1_->ReadRemoteCharacteristic(
+      GetReadValueCallback(Call::NOT_EXPECTED),
+      GetGattErrorCallback(Call::NOT_EXPECTED));
+
+  RememberCharacteristicForSubsequentAction(characteristic1_);
+  DeleteDevice(device_);
+
+  std::vector<uint8_t> empty_vector;
+  SimulateGattCharacteristicRead(/* use remembered characteristic */ nullptr,
+                                 empty_vector);
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests WriteRemoteCharacteristic completing after Chrome objects are deleted.
+TEST_F(BluetoothGattCharacteristicTest,
+       WriteRemoteCharacteristic_AfterDeleted) {
+  ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate());
+
+  std::vector<uint8_t> empty_vector;
+  characteristic1_->WriteRemoteCharacteristic(
+      empty_vector, GetCallback(Call::NOT_EXPECTED),
+      GetGattErrorCallback(Call::NOT_EXPECTED));
+
+  RememberCharacteristicForSubsequentAction(characteristic1_);
+  DeleteDevice(device_);
+
+  SimulateGattCharacteristicWrite(/* use remembered characteristic */ nullptr);
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
 // Tests ReadRemoteCharacteristic and GetValue with non-empty value buffer.
 TEST_F(BluetoothGattCharacteristicTest, ReadRemoteCharacteristic) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate());
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 d01cc1e..e9485e5 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
@@ -428,6 +428,7 @@ class Fakes {
         final int mProperties;
         final UUID mUuid;
         byte[] mValue;
+        static FakeBluetoothGattCharacteristic sRememberedCharacteristic;
 
         public FakeBluetoothGattCharacteristic(
                 FakeBluetoothGattService service, int instanceId, int properties, UUID uuid) {
@@ -441,10 +442,23 @@ class Fakes {
 
         // Simulate a value being read from a characteristic.
         @CalledByNative("FakeBluetoothGattCharacteristic")
+        private static void rememberCharacteristic(
+                ChromeBluetoothRemoteGattCharacteristic chromeCharacteristic) {
+            sRememberedCharacteristic =
+                    (FakeBluetoothGattCharacteristic) chromeCharacteristic.mCharacteristic;
+        }
+
+        // Simulate a value being read from a characteristic.
+        @CalledByNative("FakeBluetoothGattCharacteristic")
         private static void valueRead(ChromeBluetoothRemoteGattCharacteristic chromeCharacteristic,
                 int status, byte[] value) {
-            FakeBluetoothGattCharacteristic fakeCharacteristic =
-                    (FakeBluetoothGattCharacteristic) chromeCharacteristic.mCharacteristic;
+            if (chromeCharacteristic == null && sRememberedCharacteristic == null)
+                throw new IllegalArgumentException(
+                        "rememberCharacteristic wasn't called previously.");
+
+            FakeBluetoothGattCharacteristic fakeCharacteristic = (chromeCharacteristic == null)
+                    ? sRememberedCharacteristic
+                    : (FakeBluetoothGattCharacteristic) chromeCharacteristic.mCharacteristic;
 
             fakeCharacteristic.mValue = value;
             fakeCharacteristic.mService.mDevice.mGattCallback.onCharacteristicRead(
@@ -455,8 +469,13 @@ class Fakes {
         @CalledByNative("FakeBluetoothGattCharacteristic")
         private static void valueWrite(
                 ChromeBluetoothRemoteGattCharacteristic chromeCharacteristic, int status) {
-            FakeBluetoothGattCharacteristic fakeCharacteristic =
-                    (FakeBluetoothGattCharacteristic) chromeCharacteristic.mCharacteristic;
+            if (chromeCharacteristic == null && sRememberedCharacteristic == null)
+                throw new IllegalArgumentException(
+                        "rememberCharacteristic wasn't called previously.");
+
+            FakeBluetoothGattCharacteristic fakeCharacteristic = (chromeCharacteristic == null)
+                    ? sRememberedCharacteristic
+                    : (FakeBluetoothGattCharacteristic) chromeCharacteristic.mCharacteristic;
 
             fakeCharacteristic.mService.mDevice.mGattCallback.onCharacteristicWrite(
                     fakeCharacteristic, status);
diff --git a/device/bluetooth/test/bluetooth_test.h b/device/bluetooth/test/bluetooth_test.h
index a0066e9..3ddfec9 100644
--- a/device/bluetooth/test/bluetooth_test.h
+++ b/device/bluetooth/test/bluetooth_test.h
@@ -118,6 +118,14 @@ class BluetoothTestBase : public testing::Test {
                                           const std::string& uuid,
                                           int properties) {}
 
+  // Remembers |characteristic|'s platform specific object to be used in a
+  // subsequent call to methods such as SimulateGattCharacteristicRead that
+  // accept a nullptr value to select this remembered characteristic. This
+  // enables tests where the platform attempts to reference characteristic
+  // objects after the Chrome objects have been deleted, e.g. with DeleteDevice.
+  virtual void RememberCharacteristicForSubsequentAction(
+      BluetoothGattCharacteristic* characteristic) {}
+
   // Simulates a Characteristic Set Notify success.
   virtual void SimulateGattNotifySessionStarted(
       BluetoothGattCharacteristic* characteristic) {}
@@ -128,6 +136,8 @@ class BluetoothTestBase : public testing::Test {
       BluetoothGattCharacteristic* characteristic) {}
 
   // Simulates a Characteristic Read operation succeeding, returning |value|.
+  // If |characteristic| is null, acts upon the characteristic provided to
+  // RememberCharacteristicForSubsequentAction.
   virtual void SimulateGattCharacteristicRead(
       BluetoothGattCharacteristic* characteristic,
       const std::vector<uint8>& value) {}
@@ -143,6 +153,8 @@ class BluetoothTestBase : public testing::Test {
       BluetoothGattCharacteristic* characteristic) {}
 
   // Simulates a Characteristic Write operation succeeding, returning |value|.
+  // If |characteristic| is null, acts upon the characteristic provided to
+  // RememberCharacteristicForSubsequentAction.
   virtual void SimulateGattCharacteristicWrite(
       BluetoothGattCharacteristic* characteristic) {}
 
@@ -156,7 +168,7 @@ class BluetoothTestBase : public testing::Test {
   virtual void SimulateGattCharacteristicWriteWillFailSynchronouslyOnce(
       BluetoothGattCharacteristic* characteristic) {}
 
-  // Remove the device from the adapter and delete it.
+  // Removes the device from the adapter and deletes it.
   virtual void DeleteDevice(BluetoothDevice* device);
 
   // Callbacks that increment |callback_count_|, |error_callback_count_|:
diff --git a/device/bluetooth/test/bluetooth_test_android.cc b/device/bluetooth/test/bluetooth_test_android.cc
index bb55650..d822a7c 100644
--- a/device/bluetooth/test/bluetooth_test_android.cc
+++ b/device/bluetooth/test/bluetooth_test_android.cc
@@ -163,6 +163,16 @@ void BluetoothTestAndroid::SimulateGattCharacteristic(
       base::android::ConvertUTF8ToJavaString(env, uuid).obj(), properties);
 }
 
+void BluetoothTestAndroid::RememberCharacteristicForSubsequentAction(
+    BluetoothGattCharacteristic* characteristic) {
+  BluetoothRemoteGattCharacteristicAndroid* characteristic_android =
+      static_cast<BluetoothRemoteGattCharacteristicAndroid*>(characteristic);
+
+  Java_FakeBluetoothGattCharacteristic_rememberCharacteristic(
+      base::android::AttachCurrentThread(),
+      characteristic_android->GetJavaObject().obj());
+}
+
 void BluetoothTestAndroid::SimulateGattNotifySessionStarted(
     BluetoothGattCharacteristic* characteristic) {
   // Android doesn't provide any sort of callback for when notifications have
@@ -190,7 +200,9 @@ void BluetoothTestAndroid::SimulateGattCharacteristicRead(
   JNIEnv* env = base::android::AttachCurrentThread();
 
   Java_FakeBluetoothGattCharacteristic_valueRead(
-      env, characteristic_android->GetJavaObject().obj(),
+      env,
+      characteristic_android ? characteristic_android->GetJavaObject().obj()
+                             : nullptr,
       0,  // android.bluetooth.BluetoothGatt.GATT_SUCCESS
       base::android::ToJavaByteArray(env, value).obj());
 }
@@ -226,7 +238,8 @@ void BluetoothTestAndroid::SimulateGattCharacteristicWrite(
       static_cast<BluetoothRemoteGattCharacteristicAndroid*>(characteristic);
   Java_FakeBluetoothGattCharacteristic_valueWrite(
       base::android::AttachCurrentThread(),
-      characteristic_android->GetJavaObject().obj(),
+      characteristic_android ? characteristic_android->GetJavaObject().obj()
+                             : nullptr,
       0);  // android.bluetooth.BluetoothGatt.GATT_SUCCESS
 }
 
diff --git a/device/bluetooth/test/bluetooth_test_android.h b/device/bluetooth/test/bluetooth_test_android.h
index e4c1c82..664863a 100644
--- a/device/bluetooth/test/bluetooth_test_android.h
+++ b/device/bluetooth/test/bluetooth_test_android.h
@@ -39,6 +39,8 @@ class BluetoothTestAndroid : public BluetoothTestBase {
   void SimulateGattCharacteristic(BluetoothGattService* service,
                                   const std::string& uuid,
                                   int properties) override;
+  void RememberCharacteristicForSubsequentAction(
+      BluetoothGattCharacteristic* characteristic) override;
   void SimulateGattNotifySessionStarted(
       BluetoothGattCharacteristic* characteristic) override;
   void SimulateGattCharacteristicSetNotifyWillFailSynchronouslyOnce(