bluetooth: android: BluetoothRemoteGattServiceAndroid::GetUUID

Create a Java class ChromeBluetoothRemoteGattService to
implement BluetoothRemoteGattServiceAndroid, and implement
GetUUID.

Correct mistaken implementation of instanceID. Android's
getInstanceId is only unique for services with the same
UUID, so a device unique instance ID must join the UUID and
Android's getInstanceId.

Add the ability for tests to create services by providing a
vector of UUID strings.

BUG=541400

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

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

17 files changed, 189 insertions, 28 deletions

diff --git a/device/BUILD.gn b/device/BUILD.gn
index 6c8277a..32adecc 100644
--- a/device/BUILD.gn
+++ b/device/BUILD.gn
@@ -41,6 +41,7 @@ test("device_unittests") {
     "bluetooth/bluetooth_device_win_unittest.cc",
     "bluetooth/bluetooth_discovery_filter_unittest.cc",
     "bluetooth/bluetooth_gatt_chromeos_unittest.cc",
+    "bluetooth/bluetooth_gatt_service_unittest.cc",
     "bluetooth/bluetooth_low_energy_win_unittest.cc",
     "bluetooth/bluetooth_service_record_win_unittest.cc",
     "bluetooth/bluetooth_socket_chromeos_unittest.cc",
diff --git a/device/bluetooth/BUILD.gn b/device/bluetooth/BUILD.gn
index 2cb3417..a5ea564 100644
--- a/device/bluetooth/BUILD.gn
+++ b/device/bluetooth/BUILD.gn
@@ -200,6 +200,7 @@ if (is_android) {
   java_sources_needing_jni = [
     "android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java",
     "android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java",
+    "android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattService.java",
     "android/java/src/org/chromium/device/bluetooth/Wrappers.java",
   ]
 
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 037cd95..c55fb45 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
@@ -163,8 +163,12 @@ final class ChromeBluetoothDevice {
                     if (mNativeBluetoothDeviceAndroid != 0) {
                         for (Wrappers.BluetoothGattServiceWrapper service :
                                 mBluetoothGatt.getServices()) {
-                            nativeCreateGattRemoteService(mNativeBluetoothDeviceAndroid,
-                                    service.getInstanceId(), service);
+                            // Create a device unique service ID. getInstanceId only differs
+                            // between service instances with the same UUID.
+                            String serviceInstanceId =
+                                    service.getUuid().toString() + service.getInstanceId();
+                            nativeCreateGattRemoteService(
+                                    mNativeBluetoothDeviceAndroid, serviceInstanceId, service);
                         }
                     }
                 }
@@ -183,6 +187,6 @@ final class ChromeBluetoothDevice {
     // 'Object' type must be used for |bluetoothGattServiceWrapper| because inner class
     // Wrappers.BluetoothGattServiceWrapper reference is not handled by jni_generator.py JavaToJni.
     // http://crbug.com/505554
-    private native void nativeCreateGattRemoteService(
-            long nativeBluetoothDeviceAndroid, int instanceId, Object bluetoothGattServiceWrapper);
+    private native void nativeCreateGattRemoteService(long nativeBluetoothDeviceAndroid,
+            String instanceId, Object bluetoothGattServiceWrapper);
 }
diff --git a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattService.java b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattService.java
new file mode 100644
index 0000000..62c379a
--- /dev/null
+++ b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattService.java
@@ -0,0 +1,46 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.device.bluetooth;
+
+import org.chromium.base.Log;
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.base.annotations.JNINamespace;
+
+/**
+ * Exposes android.bluetooth.BluetoothGattService as necessary
+ * for C++ device::BluetoothRemoteGattServiceAndroid.
+ *
+ * Lifetime is controlled by
+ * device::BluetoothRemoteGattServiceAndroid.
+ */
+@JNINamespace("device")
+final class ChromeBluetoothRemoteGattService {
+    private static final String TAG = "Bluetooth";
+
+    final Wrappers.BluetoothGattServiceWrapper mService;
+
+    private ChromeBluetoothRemoteGattService(Wrappers.BluetoothGattServiceWrapper serviceWrapper) {
+        mService = serviceWrapper;
+        Log.v(TAG, "ChromeBluetoothRemoteGattService created.");
+    }
+
+    // ---------------------------------------------------------------------------------------------
+    // BluetoothRemoteGattServiceAndroid methods implemented in java:
+
+    // Implements BluetoothRemoteGattServiceAndroid::Create.
+    // 'Object' type must be used because inner class Wrappers.BluetoothGattServiceWrapper reference
+    // is not handled by jni_generator.py JavaToJni. http://crbug.com/505554
+    @CalledByNative
+    private static ChromeBluetoothRemoteGattService create(Object serviceWrapper) {
+        return new ChromeBluetoothRemoteGattService(
+                (Wrappers.BluetoothGattServiceWrapper) serviceWrapper);
+    }
+
+    // Implements BluetoothRemoteGattServiceAndroid::GetUUID.
+    @CalledByNative
+    private String getUUID() {
+        return mService.getUuid().toString();
+    }
+}
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 df54ee8..9f4d35f 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
@@ -28,6 +28,7 @@ import org.chromium.base.annotations.JNINamespace;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.UUID;
 
 /**
  * Wrapper classes around android.bluetooth.* classes that provide an
@@ -333,5 +334,9 @@ class Wrappers {
         public int getInstanceId() {
             return mService.getInstanceId();
         }
+
+        public UUID getUuid() {
+            return mService.getUuid();
+        }
     }
 }
diff --git a/device/bluetooth/bluetooth.gyp b/device/bluetooth/bluetooth.gyp
index c197d942..f99d017 100644
--- a/device/bluetooth/bluetooth.gyp
+++ b/device/bluetooth/bluetooth.gyp
@@ -256,6 +256,7 @@
           'sources': [
             'android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java',
             'android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java',
+            'android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattService.java',
             'android/java/src/org/chromium/device/bluetooth/Wrappers.java',
           ],
           'variables': {
diff --git a/device/bluetooth/bluetooth_device_android.cc b/device/bluetooth/bluetooth_device_android.cc
index 1066ac4..fccd24f 100644
--- a/device/bluetooth/bluetooth_device_android.cc
+++ b/device/bluetooth/bluetooth_device_android.cc
@@ -226,11 +226,12 @@ void BluetoothDeviceAndroid::OnConnectionStateChange(JNIEnv* env,
 void BluetoothDeviceAndroid::CreateGattRemoteService(
     JNIEnv* env,
     jobject caller,
-    int32_t instanceId,
+    const jstring& instanceId,
     jobject bluetooth_gatt_service_wrapper  // Java Type:
                                             // BluetoothGattServiceWrapper
     ) {
-  std::string instanceIdString = base::StringPrintf("%d", instanceId);
+  std::string instanceIdString =
+      base::android::ConvertJavaStringToUTF8(env, instanceId);
 
   if (gatt_services_.contains(instanceIdString))
     return;
diff --git a/device/bluetooth/bluetooth_device_android.h b/device/bluetooth/bluetooth_device_android.h
index e830e94..bed6780 100644
--- a/device/bluetooth/bluetooth_device_android.h
+++ b/device/bluetooth/bluetooth_device_android.h
@@ -99,7 +99,7 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDeviceAndroid final
   void CreateGattRemoteService(
       JNIEnv* env,
       jobject caller,
-      int32_t instanceId,
+      const jstring& instanceId,
       jobject bluetooth_gatt_service_wrapper);  // Java Type:
                                                 // BluetoothGattServiceWrapper
 
diff --git a/device/bluetooth/bluetooth_device_unittest.cc b/device/bluetooth/bluetooth_device_unittest.cc
index a89fc88..c6066ca 100644
--- a/device/bluetooth/bluetooth_device_unittest.cc
+++ b/device/bluetooth/bluetooth_device_unittest.cc
@@ -436,10 +436,13 @@ TEST_F(BluetoothTest, SimulateGattServicesDiscovered) {
   SimulateGattConnection(device);
   EXPECT_EQ(1, gatt_discovery_attempts_);
 
-  // TODO(scheib): Add more control over how many services are created and
-  // their properties. http://crbug.com/541400
-  SimulateGattServicesDiscovered(device);
-  EXPECT_EQ(2u, device->GetGattServices().size());
+  std::vector<std::string> services;
+  services.push_back("00000000-0000-1000-8000-00805f9b34fb");
+  // 2 duplicate UUIDs creating 2 instances.
+  services.push_back("00000001-0000-1000-8000-00805f9b34fb");
+  services.push_back("00000001-0000-1000-8000-00805f9b34fb");
+  SimulateGattServicesDiscovered(device, services);
+  EXPECT_EQ(3u, device->GetGattServices().size());
 }
 #endif  // defined(OS_ANDROID)
 
diff --git a/device/bluetooth/bluetooth_gatt_service_unittest.cc b/device/bluetooth/bluetooth_gatt_service_unittest.cc
new file mode 100644
index 0000000..91fd214
--- /dev/null
+++ b/device/bluetooth/bluetooth_gatt_service_unittest.cc
@@ -0,0 +1,45 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/bluetooth/bluetooth_gatt_service.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(OS_ANDROID)
+#include "device/bluetooth/test/bluetooth_test_android.h"
+#elif defined(OS_MACOSX)
+#include "device/bluetooth/test/bluetooth_test_mac.h"
+#endif
+
+namespace device {
+
+#if defined(OS_ANDROID)
+TEST_F(BluetoothTest, GetUUIDAndGetIdentifier) {
+  InitWithFakeAdapter();
+  StartDiscoverySession();
+  BluetoothDevice* device = DiscoverLowEnergyDevice(3);
+  device->CreateGattConnection(GetGattConnectionCallback(),
+                               GetConnectErrorCallback());
+  ResetEventCounts();
+  SimulateGattConnection(device);
+  EXPECT_EQ(1, gatt_discovery_attempts_);
+
+  // Create multiple instances with the same UUID.
+  BluetoothUUID uuid("00000000-0000-1000-8000-00805f9b34fb");
+  std::vector<std::string> services;
+  services.push_back(uuid.canonical_value());
+  services.push_back(uuid.canonical_value());
+  SimulateGattServicesDiscovered(device, services);
+
+  // Each has the same UUID.
+  EXPECT_EQ(uuid, device->GetGattServices()[0]->GetUUID());
+  EXPECT_EQ(uuid, device->GetGattServices()[1]->GetUUID());
+
+  // Instance IDs are unique.
+  EXPECT_NE(device->GetGattServices()[0]->GetIdentifier(),
+            device->GetGattServices()[1]->GetIdentifier());
+}
+#endif  // defined(OS_ANDROID)
+
+}  // namespace device
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_android.cc b/device/bluetooth/bluetooth_remote_gatt_service_android.cc
index aa25fec..c9caa3a 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_android.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_service_android.cc
@@ -4,8 +4,13 @@
 
 #include "device/bluetooth/bluetooth_remote_gatt_service_android.h"
 
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
 #include "device/bluetooth/bluetooth_adapter_android.h"
 #include "device/bluetooth/bluetooth_device_android.h"
+#include "jni/ChromeBluetoothRemoteGattService_jni.h"
+
+using base::android::AttachCurrentThread;
 
 namespace device {
 
@@ -18,16 +23,26 @@ BluetoothRemoteGattServiceAndroid* BluetoothRemoteGattServiceAndroid::Create(
   BluetoothRemoteGattServiceAndroid* service =
       new BluetoothRemoteGattServiceAndroid(adapter, device, instanceId);
 
+  service->j_service_.Reset(Java_ChromeBluetoothRemoteGattService_create(
+      AttachCurrentThread(), bluetooth_remote_gatt_service_wrapper));
+
   return service;
 }
 
+// static
+bool BluetoothRemoteGattServiceAndroid::RegisterJNI(JNIEnv* env) {
+  return RegisterNativesImpl(
+      env);  // Generated in ChromeBluetoothRemoteGattService_jni.h
+}
+
 std::string BluetoothRemoteGattServiceAndroid::GetIdentifier() const {
   return instanceId_;
 }
 
 device::BluetoothUUID BluetoothRemoteGattServiceAndroid::GetUUID() const {
-  NOTIMPLEMENTED();
-  return device::BluetoothUUID();
+  return device::BluetoothUUID(
+      ConvertJavaStringToUTF8(Java_ChromeBluetoothRemoteGattService_getUUID(
+          AttachCurrentThread(), j_service_.obj())));
 }
 
 bool BluetoothRemoteGattServiceAndroid::IsLocal() const {
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_android.h b/device/bluetooth/bluetooth_remote_gatt_service_android.h
index 6e0e702..5bc9e3d 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_android.h
+++ b/device/bluetooth/bluetooth_remote_gatt_service_android.h
@@ -41,6 +41,9 @@ class BluetoothRemoteGattServiceAndroid : public device::BluetoothGattService {
       // BluetoothRemoteGattServiceWrapper
       std::string instanceId);
 
+  // Register C++ methods exposed to Java using JNI.
+  static bool RegisterJNI(JNIEnv* env);
+
   // device::BluetoothGattService overrides.
   std::string GetIdentifier() const override;
   device::BluetoothUUID GetUUID() const override;
@@ -69,6 +72,9 @@ class BluetoothRemoteGattServiceAndroid : public device::BluetoothGattService {
                                     std::string instanceId);
   ~BluetoothRemoteGattServiceAndroid() override;
 
+  // Java object org.chromium.device.bluetooth.ChromeBluetoothRemoteGattService.
+  base::android::ScopedJavaGlobalRef<jobject> j_service_;
+
   // The adapter associated with this service. It's ok to store a raw pointer
   // here since |adapter_| indirectly owns this instance.
   BluetoothAdapterAndroid* adapter_;
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 062c3c8..2499c7b 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
@@ -17,7 +17,9 @@ import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.UUID;
 
 /**
  * Fake implementations of android.bluetooth.* classes for testing.
@@ -212,15 +214,20 @@ class Fakes {
         // Create a call to onServicesDiscovered on the |chrome_device| using parameter
         // |status|.
         @CalledByNative("FakeBluetoothDevice")
-        private static void servicesDiscovered(ChromeBluetoothDevice chromeDevice, int status) {
+        private static void servicesDiscovered(
+                ChromeBluetoothDevice chromeDevice, int status, String uuidsSpaceDelimited) {
             FakeBluetoothDevice fakeDevice = (FakeBluetoothDevice) chromeDevice.mDevice;
 
-            // TODO(scheib): Add more control over how many services are created and
-            // their properties. http://crbug.com/541400
             if (status == android.bluetooth.BluetoothGatt.GATT_SUCCESS) {
                 fakeDevice.mGatt.mServices.clear();
-                fakeDevice.mGatt.mServices.add(new FakeBluetoothGattService(0));
-                fakeDevice.mGatt.mServices.add(new FakeBluetoothGattService(1));
+                HashMap<String, Integer> uuidsToInstanceIdMap = new HashMap<String, Integer>();
+                for (String uuid : uuidsSpaceDelimited.split(" ")) {
+                    Integer previousId = uuidsToInstanceIdMap.get(uuid);
+                    int instanceId = (previousId == null) ? 0 : previousId + 1;
+                    uuidsToInstanceIdMap.put(uuid, instanceId);
+                    fakeDevice.mGatt.mServices.add(
+                            new FakeBluetoothGattService(UUID.fromString(uuid), instanceId));
+                }
             }
 
             fakeDevice.mGattCallback.onServicesDiscovered(status);
@@ -297,9 +304,11 @@ class Fakes {
      */
     static class FakeBluetoothGattService extends Wrappers.BluetoothGattServiceWrapper {
         final int mInstanceId;
+        final UUID mUuid;
 
-        public FakeBluetoothGattService(int instanceId) {
+        public FakeBluetoothGattService(UUID uuid, int instanceId) {
             super(null);
+            mUuid = uuid;
             mInstanceId = instanceId;
         }
 
@@ -307,6 +316,11 @@ class Fakes {
         public int getInstanceId() {
             return mInstanceId;
         }
+
+        @Override
+        public UUID getUuid() {
+            return mUuid;
+        }
     }
 
     // ---------------------------------------------------------------------------------------------
diff --git a/device/bluetooth/test/bluetooth_test.h b/device/bluetooth/test/bluetooth_test.h
index 43b47f2..11e6b6b 100644
--- a/device/bluetooth/test/bluetooth_test.h
+++ b/device/bluetooth/test/bluetooth_test.h
@@ -84,10 +84,12 @@ class BluetoothTestBase : public testing::Test {
   // Simulates GattConnection disconnecting.
   virtual void SimulateGattDisconnection(BluetoothDevice* device) {}
 
-  // Simulates success of discovering services. Two services are created.
-  // TODO(scheib): Add more control over how many services are created and
-  // their properties. http://crbug.com/541400
-  virtual void SimulateGattServicesDiscovered(BluetoothDevice* device) {}
+  // Simulates success of discovering services. |uuids| is used to create a
+  // service for each UUID string. Multiple UUIDs with the same value produce
+  // multiple service instances.
+  virtual void SimulateGattServicesDiscovered(
+      BluetoothDevice* device,
+      const std::vector<std::string>& uuids) {}
 
   // Simulates failure to discover services.
   virtual void SimulateGattServicesDiscoveryError(BluetoothDevice* device) {}
diff --git a/device/bluetooth/test/bluetooth_test_android.cc b/device/bluetooth/test/bluetooth_test_android.cc
index 01bc12f..48c2ab2 100644
--- a/device/bluetooth/test/bluetooth_test_android.cc
+++ b/device/bluetooth/test/bluetooth_test_android.cc
@@ -4,6 +4,10 @@
 
 #include "device/bluetooth/test/bluetooth_test_android.h"
 
+#include <iterator>
+#include <sstream>
+
+#include "base/android/jni_string.h"
 #include "base/logging.h"
 #include "device/bluetooth/android/wrappers.h"
 #include "device/bluetooth/bluetooth_adapter_android.h"
@@ -107,13 +111,22 @@ void BluetoothTestAndroid::SimulateGattDisconnection(BluetoothDevice* device) {
 }
 
 void BluetoothTestAndroid::SimulateGattServicesDiscovered(
-    BluetoothDevice* device) {
+    BluetoothDevice* device,
+    const std::vector<std::string>& uuids) {
   BluetoothDeviceAndroid* device_android =
       static_cast<BluetoothDeviceAndroid*>(device);
+  JNIEnv* env = base::android::AttachCurrentThread();
+
+  // Join UUID strings into a single string.
+  std::ostringstream uuids_space_delimited;
+  std::copy(uuids.begin(), uuids.end(),
+            std::ostream_iterator<std::string>(uuids_space_delimited, " "));
 
   Java_FakeBluetoothDevice_servicesDiscovered(
-      AttachCurrentThread(), device_android->GetJavaObject().obj(),
-      0);  // android.bluetooth.BluetoothGatt.GATT_SUCCESS
+      env, device_android->GetJavaObject().obj(),
+      0,  // android.bluetooth.BluetoothGatt.GATT_SUCCESS
+      base::android::ConvertUTF8ToJavaString(env, uuids_space_delimited.str())
+          .obj());
 }
 
 void BluetoothTestAndroid::SimulateGattServicesDiscoveryError(
@@ -123,7 +136,8 @@ void BluetoothTestAndroid::SimulateGattServicesDiscoveryError(
 
   Java_FakeBluetoothDevice_servicesDiscovered(
       AttachCurrentThread(), device_android->GetJavaObject().obj(),
-      0x00000101);  // android.bluetooth.BluetoothGatt.GATT_FAILURE
+      0x00000101,  // android.bluetooth.BluetoothGatt.GATT_FAILURE
+      nullptr);
 }
 
 void BluetoothTestAndroid::OnFakeBluetoothDeviceConnectGattCalled(
diff --git a/device/bluetooth/test/bluetooth_test_android.h b/device/bluetooth/test/bluetooth_test_android.h
index 4db263b..306d81a 100644
--- a/device/bluetooth/test/bluetooth_test_android.h
+++ b/device/bluetooth/test/bluetooth_test_android.h
@@ -31,7 +31,9 @@ class BluetoothTestAndroid : public BluetoothTestBase {
   void SimulateGattConnectionError(BluetoothDevice* device,
                                    BluetoothDevice::ConnectErrorCode) override;
   void SimulateGattDisconnection(BluetoothDevice* device) override;
-  void SimulateGattServicesDiscovered(BluetoothDevice* device) override;
+  void SimulateGattServicesDiscovered(
+      BluetoothDevice* device,
+      const std::vector<std::string>& uuids) override;
   void SimulateGattServicesDiscoveryError(BluetoothDevice* device) override;
 
   // Records that Java FakeBluetoothDevice connectGatt was called.
diff --git a/device/device_tests.gyp b/device/device_tests.gyp
index a645670..3cc4fc0 100644
--- a/device/device_tests.gyp
+++ b/device/device_tests.gyp
@@ -47,6 +47,7 @@
         'bluetooth/bluetooth_device_win_unittest.cc',
         'bluetooth/bluetooth_discovery_filter_unittest.cc',
         'bluetooth/bluetooth_gatt_chromeos_unittest.cc',
+        'bluetooth/bluetooth_gatt_service_unittest.cc',
         'bluetooth/bluetooth_low_energy_win_unittest.cc',
         'bluetooth/bluetooth_service_record_win_unittest.cc',
         'bluetooth/bluetooth_socket_chromeos_unittest.cc',