[Chromecast] Add WebBluetooth support to Cast platforms (part 2 of N).

Introduce better lifetime management for BluetoothAdapterCast, allowing
the Cast embedder code to inject a factory for this class. This prevents
potential races at destruct time.

Also wires up the code to make Characteristic notifications work. With
this patch (and some hacks in content), a JS application can receive
notifications successfully.

BUG=827672

Bug: b/75967216
Test: test page
Change-Id: Idb8a1ba2c9ad94a80940083d31e307b3506f98e6
Reviewed-on: https://chromium-review.googlesource.com/988265
Commit-Queue: Stephen Lanham <slan@chromium.org>
Reviewed-by: Conley Owens <cco3@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547922}

chromecast/browser/BUILD.gn

@@ -127,6 +127,7 @@ cast_source_set("browser") {
     "//content/public/common",
     "//content/public/common:service_names",
     "//content/public/utility",
+    "//device/bluetooth",
     "//device/geolocation",
     "//gpu",
     "//ipc",

chromecast/browser/DEPS

@@ -23,6 +23,7 @@ include_rules = [
   "+content/public/browser",
   "+content/public/common",
   "+content/public/test",
+  "+device/bluetooth",
   "+device/geolocation",
   "+extensions/browser",
   "+extensions/common",

chromecast/browser/cast_browser_main_parts.cc

@@ -104,6 +104,10 @@
 #include "extensions/browser/extension_prefs.h"  // nogncheck
 #endif
 
+#if !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
+#include "device/bluetooth/cast/bluetooth_adapter_cast.h"
+#endif  // !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
+
 namespace {
 
 #if !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
@@ -469,6 +473,13 @@ int CastBrowserMainParts::PreCreateThreads() {
 void CastBrowserMainParts::PreMainMessageLoopRun() {
 #if !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
   memory_pressure_monitor_.reset(new CastMemoryPressureMonitor());
+
+  // base::Unretained() is safe because the browser client will outlive any
+  // component in the browser; this factory method will not be called after
+  // the browser starts to tear down.
+  device::BluetoothAdapterCast::SetFactory(base::BindRepeating(
+      &CastContentBrowserClient::CreateBluetoothAdapter,
+      base::Unretained(cast_browser_process_->browser_client())));
 #endif  // !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
 
   cast_browser_process_->SetNetLog(net_log_.get());

chromecast/browser/cast_content_browser_client.cc

@@ -249,6 +249,14 @@ media::MediaCapsImpl* CastContentBrowserClient::media_caps() {
   return cast_browser_main_parts_->media_caps();
 }
 
+#if !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
+base::WeakPtr<device::BluetoothAdapterCast>
+CastContentBrowserClient::CreateBluetoothAdapter() {
+  NOTREACHED() << "Bluetooth Adapter is not supported!";
+  return nullptr;
+}
+#endif  // !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
+
 void CastContentBrowserClient::SetMetricsClientId(
     const std::string& client_id) {}
 

chromecast/browser/cast_content_browser_client.h

@@ -25,6 +25,10 @@ namespace breakpad {
 class CrashHandlerHostLinux;
 }
 
+namespace device {
+class BluetoothAdapterCast;
+}
+
 namespace metrics {
 class MetricsService;
 }
@@ -96,6 +100,14 @@ class CastContentBrowserClient : public content::ContentBrowserClient {
 #endif  // BUILDFLAG(IS_CAST_USING_CMA_BACKEND)
   media::MediaCapsImpl* media_caps();
 
+#if !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
+  // Create a BluetoothAdapter for WebBluetooth support.
+  // TODO(slan): This further couples the browser to the Cast service. Remove
+  // this once the dedicated Bluetooth service has been implemented.
+  // (b/76155468)
+  virtual base::WeakPtr<device::BluetoothAdapterCast> CreateBluetoothAdapter();
+#endif  // !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
+
   // Invoked when the metrics client ID changes.
   virtual void SetMetricsClientId(const std::string& client_id);
 

chromecast/device/bluetooth/le/remote_characteristic_impl.cc

@@ -119,8 +119,11 @@ void RemoteCharacteristicImpl::SetRegisterNotification(bool enable,
 
   auto it = uuid_to_descriptor_.find(RemoteDescriptor::kCccdUuid);
   if (it == uuid_to_descriptor_.end()) {
-    LOG(ERROR) << "No CCCD found";
-    EXEC_CB_AND_RET(cb, false);
+    // If there is no CCCD on the remote characteristic, this is unusual, but
+    // not something that we can necessarily help. Log a warning and return
+    // success.
+    LOG(WARNING) << "No CCCD found on the remote characteristic.";
+    EXEC_CB_AND_RET(cb, true);
   }
 
   it->second->WriteAuth(bluetooth_v2_shlib::Gatt::Client::AUTH_REQ_NONE,

device/BUILD.gn

@@ -220,9 +220,16 @@ test("device_unittests") {
     deps += [ "//dbus" ]
   }
 
-  # BLE discovery: works on Linux.
   if (is_linux) {
+    # BLE discovery: works on Linux.
     sources += [ "fido/fido_ble_discovery_unittest.cc" ]
+
+    if (is_chromecast) {
+      sources += [
+        "bluetooth/cast/bluetooth_adapter_cast_unittest.cc",
+        "bluetooth/cast/bluetooth_utils_unittest.cc",
+      ]
+    }
   }
 
   if (is_mac) {

device/bluetooth/BUILD.gn

@@ -406,6 +406,12 @@ component("bluetooth") {
           "cast/bluetooth_remote_gatt_descriptor_cast.h",
           "cast/bluetooth_remote_gatt_service_cast.cc",
           "cast/bluetooth_remote_gatt_service_cast.h",
+          "cast/bluetooth_utils.cc",
+          "cast/bluetooth_utils.h",
+        ]
+
+        public_deps = [
+          "//chromecast/public",
         ]
 
         deps += [

device/bluetooth/cast/DEPS

 include_rules = [
-  "+chromecast/device/bluetooth"
+  "+chromecast/device/bluetooth",
+  "+chromecast/public/bluetooth"
 ]

device/bluetooth/cast/bluetooth_adapter_cast.cc

@@ -6,26 +6,26 @@
 
 #include "base/bind.h"
 #include "base/location.h"
+#include "base/no_destructor.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "chromecast/device/bluetooth/bluetooth_util.h"
 #include "chromecast/device/bluetooth/le/gatt_client_manager.h"
 #include "chromecast/device/bluetooth/le/le_scan_manager.h"
+#include "chromecast/device/bluetooth/le/remote_characteristic.h"
 #include "chromecast/device/bluetooth/le/remote_device.h"
+#include "chromecast/device/bluetooth/le/remote_service.h"
 #include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/bluetooth_discovery_session_outcome.h"
 #include "device/bluetooth/cast/bluetooth_device_cast.h"
+#include "device/bluetooth/cast/bluetooth_utils.h"
 
 namespace device {
 namespace {
 
-// The classes in //device/bluetooth expect addresses to be in the canonical
-// format: "AA:BB:CC:DD:EE:FF". Use this utility whenever an address from the
-// Cast stack is converted.
-std::string GetCanonicalAddress(
-    const chromecast::bluetooth_v2_shlib::Addr& addr) {
-  return BluetoothDevice::CanonicalizeAddress(
-      chromecast::bluetooth::util::AddrToString(addr));
+BluetoothAdapterCast::FactoryCb& GetFactory() {
+  static base::NoDestructor<BluetoothAdapterCast::FactoryCb> factory_cb;
+  return *factory_cb;
 }
 
 }  // namespace
@@ -201,13 +201,18 @@ void BluetoothAdapterCast::RemovePairingDelegateInternal(
   NOTIMPLEMENTED();
 }
 
+base::WeakPtr<BluetoothAdapterCast> BluetoothAdapterCast::GetWeakPtr() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return weak_factory_.GetWeakPtr();
+}
+
 void BluetoothAdapterCast::OnConnectChanged(
     scoped_refptr<chromecast::bluetooth::RemoteDevice> device,
     bool connected) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  auto address = GetCanonicalAddress(device->addr());
-  VLOG(1) << __func__ << " " << address << " connected: " << connected;
+  std::string address = GetCanonicalBluetoothAddress(device->addr());
+  DVLOG(1) << __func__ << " " << address << " connected: " << connected;
 
   // This method could be called before this device is detected in a scan and
   // GetDevice() is called. Add it if needed.
@@ -223,19 +228,26 @@ void BluetoothAdapterCast::OnMtuChanged(
     int mtu) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  DVLOG(3) << __func__ << " " << GetCanonicalAddress(device->addr())
+  DVLOG(3) << __func__ << " " << GetCanonicalBluetoothAddress(device->addr())
            << " mtu: " << mtu;
 }
 
 void BluetoothAdapterCast::OnServicesUpdated(
     scoped_refptr<chromecast::bluetooth::RemoteDevice> device,
     std::vector<scoped_refptr<chromecast::bluetooth::RemoteService>> services) {
+  DVLOG(1) << __func__;
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  auto it = devices_.find(GetCanonicalAddress(device->addr()));
-  if (it == devices_.end())
+  std::string address = GetCanonicalBluetoothAddress(device->addr());
+  BluetoothDeviceCast* cast_device = GetCastDevice(address);
+  if (!cast_device)
     return;
-  it->second->SetGattServicesDiscoveryComplete(true);
+
+  if (!cast_device->UpdateServices(services))
+    LOG(WARNING) << "The services were not updated. Alerting anyway.";
+
+  cast_device->SetGattServicesDiscoveryComplete(true);
+  NotifyGattServicesDiscovered(cast_device);
 }
 
 void BluetoothAdapterCast::OnCharacteristicNotification(
@@ -244,20 +256,23 @@ void BluetoothAdapterCast::OnCharacteristicNotification(
     std::vector<uint8_t> value) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  DVLOG(1) << __func__ << " " << GetCanonicalAddress(device->addr())
-           << " updated.";
+  std::string address = GetCanonicalBluetoothAddress(device->addr());
+  BluetoothDeviceCast* cast_device = GetCastDevice(address);
+  if (!cast_device)
+    return;
 
-  // TODO(slan): Add an Observer interface to RemoteCharacteristc so this can be
-  // wired directly to the BluetoothRemoteGattCharacteristicCast proxy, rather
-  // than by performing a search of the services on device for |characteristc|.
-  NOTIMPLEMENTED();
+  cast_device->UpdateCharacteristicValue(
+      std::move(characteristic), std::move(value),
+      base::BindOnce(
+          &BluetoothAdapterCast::NotifyGattCharacteristicValueChanged,
+          weak_factory_.GetWeakPtr()));
 }
 
 void BluetoothAdapterCast::OnNewScanResult(
     chromecast::bluetooth::LeScanManager::ScanResult result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  auto address = GetCanonicalAddress(result.addr);
+  std::string address = GetCanonicalBluetoothAddress(result.addr);
 
   // If we haven't created a BluetoothDeviceCast for this address yet, we need
   // to send an async request to |gatt_client_manager_| for a handle to the
@@ -300,7 +315,10 @@ void BluetoothAdapterCast::OnScanEnableChanged(bool enabled) {
 
 BluetoothDeviceCast* BluetoothAdapterCast::GetCastDevice(
     const std::string& address) {
-  return static_cast<BluetoothDeviceCast*>(devices_[address].get());
+  auto it = devices_.find(address);
+  return it == devices_.end()
+             ? nullptr
+             : static_cast<BluetoothDeviceCast*>(it->second.get());
 }
 
 void BluetoothAdapterCast::AddDevice(
@@ -309,7 +327,7 @@ void BluetoothAdapterCast::AddDevice(
 
   // This method should not be called if we already have a BluetoothDeviceCast
   // registered for this device.
-  auto address = GetCanonicalAddress(remote_device->addr());
+  std::string address = GetCanonicalBluetoothAddress(remote_device->addr());
   DCHECK(devices_.find(address) == devices_.end());
 
   devices_[address] =
@@ -346,7 +364,7 @@ void BluetoothAdapterCast::OnGetDevice(
     scoped_refptr<chromecast::bluetooth::RemoteDevice> remote_device) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  auto address = GetCanonicalAddress(remote_device->addr());
+  std::string address = GetCanonicalBluetoothAddress(remote_device->addr());
 
   // This callback could run before or after the device becomes connected and
   // OnConnectChanged() is called for a particular device. If that happened,
@@ -383,18 +401,24 @@ void BluetoothAdapterCast::InitializeAsynchronously(InitCallback callback) {
   std::move(callback).Run();
 }
 
+// static
+void BluetoothAdapterCast::SetFactory(FactoryCb factory_cb) {
+  FactoryCb& factory = GetFactory();
+  DCHECK(!factory);
+  factory = std::move(factory_cb);
+}
+
+// static
+void BluetoothAdapterCast::ResetFactoryForTest() {
+  GetFactory().Reset();
+}
+
 // static
 base::WeakPtr<BluetoothAdapter> BluetoothAdapterCast::Create(
     InitCallback callback) {
-  // TODO(slan): We need to figure out how to get these classes properly.
-  chromecast::bluetooth::GattClientManager* gatt_client_manager = nullptr;
-  chromecast::bluetooth::LeScanManager* le_scan_manager = nullptr;
-
-  // TODO(slan): Consider life-cycle management. Currently this just leaks.
-  auto* adapter =
-      new BluetoothAdapterCast(gatt_client_manager, le_scan_manager);
-  base::WeakPtr<BluetoothAdapterCast> weak_ptr =
-      adapter->weak_factory_.GetWeakPtr();
+  FactoryCb& factory = GetFactory();
+  DCHECK(factory) << "SetFactory() must be called before this method!";
+  base::WeakPtr<BluetoothAdapterCast> weak_ptr = factory.Run();
 
   // BluetoothAdapterFactory assumes that |init_callback| will be called
   // asynchronously the first time, and that IsInitialized() will return false.

device/bluetooth/cast/bluetooth_adapter_cast.h

@@ -16,6 +16,7 @@
 #include "chromecast/device/bluetooth/le/gatt_client_manager.h"
 #include "chromecast/device/bluetooth/le/le_scan_manager.h"
 #include "device/bluetooth/bluetooth_adapter.h"
+#include "device/bluetooth/bluetooth_export.h"
 
 namespace device {
 
@@ -31,10 +32,18 @@ class BluetoothDeviceCast;
 // This class is created and called on a single thread. It makes aysnchronous
 // calls to the Cast bluetooth stack, which may live on another thread. Unless
 // noted otherwise, callbacks will always be posted on the calling thread.
-class BluetoothAdapterCast : public BluetoothAdapter,
-                             chromecast::bluetooth::GattClientManager::Observer,
-                             chromecast::bluetooth::LeScanManager::Observer {
+class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterCast
+    : public BluetoothAdapter,
+      chromecast::bluetooth::GattClientManager::Observer,
+      chromecast::bluetooth::LeScanManager::Observer {
  public:
+  // Do not call this constructor directly; use CreateAdapter() instead. Neither
+  // |gatt_client_manager| nor |le_scan_manager| are owned by this class. Both
+  // must outlive |this|.
+  BluetoothAdapterCast(
+      chromecast::bluetooth::GattClientManager* gatt_client_manager,
+      chromecast::bluetooth::LeScanManager* le_scan_manager);
+
   // BluetoothAdapter implementation:
   std::string GetAddress() const override;
   std::string GetName() const override;
@@ -93,16 +102,30 @@ class BluetoothAdapterCast : public BluetoothAdapter,
   void RemovePairingDelegateInternal(
       BluetoothDevice::PairingDelegate* pairing_delegate) override;
 
-  // Creates a BluetoothAdapterCast. |callback| will be executed asynchronously
+  // Return a WeakPtr for this class. Must be called on the sequence on which
+  // this class was created.
+  // TODO(slan): Remove this once this class talks to a dedicated Bluetooth
+  // service (b/76155468)
+  base::WeakPtr<BluetoothAdapterCast> GetWeakPtr();
+
+  // |factory_cb| is used to inject a factory method from ChromecastService into
+  // this class. It will be invoked when Create() is called.
+  // TODO(slan): Remove this once this class talks to a dedicated Bluetooth
+  // service (b/76155468)
+  using FactoryCb =
+      base::RepeatingCallback<base::WeakPtr<BluetoothAdapterCast>()>;
+  static void SetFactory(FactoryCb factory_cb);
+
+  // Resets the factory callback for test scenarios.
+  static void ResetFactoryForTest();
+
+  // Creates a BluetoothAdapterCast using the |factory_cb| set in SetFactory().
+  // This method is intended to be called only by the WebBluetooth code in
+  // //device/blutooth/. |callback| will be executed asynchronously
   // on the calling sequence.
   static base::WeakPtr<BluetoothAdapter> Create(InitCallback callback);
 
  private:
-  // Neither |gatt_client_manager| nor |le_scan_manager| are owned by this
-  // class. Both must outlive |this|.
-  BluetoothAdapterCast(
-      chromecast::bluetooth::GattClientManager* gatt_client_manager,
-      chromecast::bluetooth::LeScanManager* le_scan_manager);
   ~BluetoothAdapterCast() override;
 
   // chromecast::bluetooth::GattClientManager::Observer implementation:

device/bluetooth/cast/bluetooth_adapter_cast_unittest.cc

+// Copyright 2018 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/cast/bluetooth_adapter_cast.h"
+
+#include "base/bind_helpers.h"
+#include "base/test/gtest_util.h"
+#include "base/test/mock_callback.h"
+#include "base/test/scoped_task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace device {
+
+class BluetoothAdapterCastTest : public testing::Test {
+ public:
+  BluetoothAdapterCastTest() = default;
+  ~BluetoothAdapterCastTest() override {
+    BluetoothAdapterCast::ResetFactoryForTest();
+  };
+
+ private:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+
+  DISALLOW_COPY_AND_ASSIGN(BluetoothAdapterCastTest);
+};
+
+TEST_F(BluetoothAdapterCastTest, TestSetFactory) {
+  // Test that the callback set with SetFactory() is called by Create().
+  base::MockCallback<BluetoothAdapterCast::FactoryCb> callback;
+  BluetoothAdapterCast::SetFactory(callback.Get());
+
+  // Call the method once.
+  EXPECT_CALL(callback, Run());
+  BluetoothAdapterCast::Create(base::DoNothing());
+
+  // Call it again.
+  EXPECT_CALL(callback, Run());
+  BluetoothAdapterCast::Create(base::DoNothing());
+}
+
+#if DCHECK_IS_ON()
+TEST_F(BluetoothAdapterCastTest, TestSetFactoryTwiceCrashes) {
+  // Test that calling SetFactory() more than once causes a crash.
+  base::MockCallback<BluetoothAdapterCast::FactoryCb> callback;
+  BluetoothAdapterCast::SetFactory(callback.Get());
+
+  // The factory has already been set. Crash.
+  EXPECT_DCHECK_DEATH(BluetoothAdapterCast::SetFactory(callback.Get()));
+}
+
+TEST_F(BluetoothAdapterCastTest, TestNoSetFactoryCrashes) {
+  // Test that calling BluetoothAdapterCast::Create() without calling
+  // SetFactory() causes a crash.
+  EXPECT_DCHECK_DEATH(BluetoothAdapterCast::Create(base::DoNothing()));
+}
+#endif  // DCHECK_IS_ON()
+
+}  // namespace device

device/bluetooth/cast/bluetooth_device_cast.cc

@@ -6,7 +6,11 @@
 
 #include "base/bind.h"
 #include "base/strings/stringprintf.h"
-#include "chromecast/device/bluetooth/bluetooth_util.h"
+#include "chromecast/device/bluetooth/le/remote_characteristic.h"
+#include "chromecast/device/bluetooth/le/remote_service.h"
+#include "device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.h"
+#include "device/bluetooth/cast/bluetooth_remote_gatt_service_cast.h"
+#include "device/bluetooth/cast/bluetooth_utils.h"
 
 namespace device {
 namespace {
@@ -52,8 +56,7 @@ BluetoothDeviceCast::BluetoothDeviceCast(
     scoped_refptr<chromecast::bluetooth::RemoteDevice> device)
     : BluetoothDevice(adapter),
       remote_device_(std::move(device)),
-      address_(CanonicalizeAddress(
-          chromecast::bluetooth::util::AddrToString(remote_device_->addr()))),
+      address_(GetCanonicalBluetoothAddress(remote_device_->addr())),
       weak_factory_(this) {}
 
 BluetoothDeviceCast::~BluetoothDeviceCast() {}
@@ -257,20 +260,84 @@ bool BluetoothDeviceCast::UpdateWithScanResult(
 
 bool BluetoothDeviceCast::SetConnected(bool connected) {
   DVLOG(2) << __func__ << " connected: " << connected;
-  bool changed = false;
-  if (!connected_ && connected) {
+  bool was_connected = connected_;
+
+  // Set the new state *before* calling the protected methods below. They may
+  // synchronously query the state of the device.
+  connected_ = connected;
+
+  // Update state in the base class. This will cause pending callbacks to be
+  // fired.
+  if (!was_connected && connected) {
     DidConnectGatt();
-    changed = true;
-  } else if (connected_ && !connected) {
+  } else if (was_connected && !connected) {
     DidDisconnectGatt();
+  }
+
+  // Return true if the value of |connected_| changed.
+  return was_connected != connected;
+}
+
+bool BluetoothDeviceCast::UpdateServices(
+    std::vector<scoped_refptr<chromecast::bluetooth::RemoteService>> services) {
+  DVLOG(2) << __func__;
+  bool changed = false;
+
+  // Create a look-up for the updated list of services.
+  std::unordered_set<std::string> new_service_uuids;
+  for (const auto& service : services)
+    new_service_uuids.insert(GetCanonicalBluetoothUuid(service->uuid()));
+
+  // Remove any services in |gatt_services_| that are not present in |services|.
+  for (auto it = gatt_services_.cbegin(); it != gatt_services_.cend();) {
+    if (new_service_uuids.find(it->first) == new_service_uuids.end()) {
+      gatt_services_.erase(it++);
+      changed = true;
+    } else {
+      ++it;
+    }
+  }
+
+  // Add new services.
+  for (auto& service : services) {
+    auto key = GetCanonicalBluetoothUuid(service->uuid());
+
+    if (gatt_services_.find(key) != gatt_services_.end())
+      continue;
+
+    auto cast_service = std::make_unique<BluetoothRemoteGattServiceCast>(
+        this, std::move(service));
+    DCHECK_EQ(key, cast_service->GetIdentifier());
+    gatt_services_[key] = std::move(cast_service);
     changed = true;
   }
 
-  connected_ = connected;
   return changed;
 }
 
+bool BluetoothDeviceCast::UpdateCharacteristicValue(
+    scoped_refptr<chromecast::bluetooth::RemoteCharacteristic> characteristic,
+    std::vector<uint8_t> value,
+    OnValueUpdatedCallback callback) {
+  auto uuid = UuidToBluetoothUUID(characteristic->uuid());
+  // TODO(slan): Consider using a look-up to find characteristics instead. This
+  // approach could be inefficient if a device has a lot of characteristics.
+  for (const auto& it : gatt_services_) {
+    for (auto* c : it.second->GetCharacteristics()) {
+      if (c->GetUUID() == uuid) {
+        static_cast<BluetoothRemoteGattCharacteristicCast*>(c)->SetValue(value);
+        std::move(callback).Run(c, value);
+        return true;
+      }
+    }
+  }
+  LOG(WARNING) << GetAddress() << " does not have a service with "
+               << " characteristic " << uuid.canonical_value();
+  return false;
+}
+
 void BluetoothDeviceCast::CreateGattConnectionImpl() {
+  DVLOG(2) << __func__ << " " << pending_connect_;
   if (pending_connect_)
     return;
   pending_connect_ = true;
@@ -279,6 +346,7 @@ void BluetoothDeviceCast::CreateGattConnectionImpl() {
 }
 
 void BluetoothDeviceCast::DisconnectGatt() {
+  DVLOG(2) << __func__ << " pending:" << pending_disconnect_;
   if (pending_disconnect_)
     return;
   pending_disconnect_ = true;
@@ -287,6 +355,7 @@ void BluetoothDeviceCast::DisconnectGatt() {
 }
 
 void BluetoothDeviceCast::OnConnect(bool success) {
+  DVLOG(2) << __func__ << " success:" << success;
   pending_connect_ = false;
   if (success)
     SetConnected(true);
@@ -295,6 +364,7 @@ void BluetoothDeviceCast::OnConnect(bool success) {
 }
 
 void BluetoothDeviceCast::OnDisconnect(bool success) {
+  DVLOG(2) << __func__ << " success:" << success;
   pending_disconnect_ = false;
   if (success)
     SetConnected(false);

device/bluetooth/cast/bluetooth_device_cast.h

@@ -97,6 +97,26 @@ class BluetoothDeviceCast : public BluetoothDevice {
   // connection state changed as a result.
   bool SetConnected(bool connected);
 
+  // Called by BluetoothAdapterCast when the GATT services for this device are
+  // updated. Updates the services in this devices to reflect |services|.
+  // Returns true if a service was added or removed.
+  bool UpdateServices(
+      std::vector<scoped_refptr<chromecast::bluetooth::RemoteService>>
+          services);
+
+  // Called by BluetoothAdapterCast when the value of a characteristic in one of
+  // this device's services has changed, resulting in a notification to the
+  // device. Locate the characteristc and update the underluing value. If the
+  // value is updated, run |callback| synchronously. Return true if that value
+  // changed.
+  using OnValueUpdatedCallback =
+      base::OnceCallback<void(BluetoothRemoteGattCharacteristic*,
+                              const std::vector<uint8_t>&)>;
+  bool UpdateCharacteristicValue(
+      scoped_refptr<chromecast::bluetooth::RemoteCharacteristic> characteristic,
+      std::vector<uint8_t> value,
+      OnValueUpdatedCallback callback);
+
  private:
   // Implements platform specific operations to initiate a GATT connection.
   // Subclasses must also call DidConnectGatt, DidFailToConnectGatt, or
@@ -129,4 +149,4 @@ class BluetoothDeviceCast : public BluetoothDevice {
 
 }  // namespace device
 
-#endif  // DEVICE_BLUETOOTH_CAST_BLUETOOTH_DEVICE_CAST_H_
+#endif  // DEVICE_BLUETOOTH_CAST_BLUETOOTH_DEVICE_CAST_H_
\ No newline at end of file

device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.cc

@@ -10,16 +10,67 @@
 #include "base/containers/queue.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "chromecast/device/bluetooth/bluetooth_util.h"
 #include "chromecast/device/bluetooth/le/remote_characteristic.h"
 #include "chromecast/device/bluetooth/le/remote_descriptor.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 #include "device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.h"
 #include "device/bluetooth/cast/bluetooth_remote_gatt_service_cast.h"
+#include "device/bluetooth/cast/bluetooth_utils.h"
 
 namespace device {
 namespace {
 
+BluetoothGattCharacteristic::Permissions ConvertPermissions(
+    chromecast::bluetooth_v2_shlib::Gatt::Permissions input) {
+  BluetoothGattCharacteristic::Permissions output =
+      BluetoothGattCharacteristic::PERMISSION_NONE;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PERMISSION_READ)
+    output |= BluetoothGattCharacteristic::PERMISSION_READ;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PERMISSION_READ_ENCRYPTED)
+    output |= BluetoothGattCharacteristic::PERMISSION_READ_ENCRYPTED;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PERMISSION_WRITE)
+    output |= BluetoothGattCharacteristic::PERMISSION_WRITE;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PERMISSION_WRITE_ENCRYPTED)
+    output |= BluetoothGattCharacteristic::PERMISSION_WRITE_ENCRYPTED;
+
+  // NOTE(slan): Determine the proper mapping for these.
+  // if (input & chromecast::bluetooth_v2_shlib::PERMISSION_READ_ENCRYPTED_MITM)
+  //   output |= BluetoothGattCharacteristic::PERMISSION_READ_ENCRYPTED_MITM;
+  // if (input &
+  // chromecast::bluetooth_v2_shlib::PERMISSION_WRITE_ENCRYPTED_MITM)
+  //   output |= BluetoothGattCharacteristic::PERMISSION_WRITE_ENCRYPTED_MITM;
+  // if (input & chromecast::bluetooth_v2_shlib::PERMISSION_WRITE_SIGNED)
+  //   output |= BluetoothGattCharacteristic::PERMISSION_WRITE_SIGNED;
+  // if (input & chromecast::bluetooth_v2_shlib::PERMISSION_WRITE_SIGNED_MITM)
+  //   output |= BluetoothGattCharacteristic::PERMISSION_WRITE_SIGNED_MITM;
+
+  return output;
+}
+
+BluetoothGattCharacteristic::Properties ConvertProperties(
+    chromecast::bluetooth_v2_shlib::Gatt::Properties input) {
+  BluetoothGattCharacteristic::Properties output =
+      BluetoothGattCharacteristic::PROPERTY_NONE;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PROPERTY_BROADCAST)
+    output |= BluetoothGattCharacteristic::PROPERTY_BROADCAST;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PROPERTY_READ)
+    output |= BluetoothGattCharacteristic::PROPERTY_READ;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PROPERTY_WRITE_NO_RESPONSE)
+    output |= BluetoothGattCharacteristic::PROPERTY_WRITE_WITHOUT_RESPONSE;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PROPERTY_WRITE)
+    output |= BluetoothGattCharacteristic::PROPERTY_WRITE;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PROPERTY_NOTIFY)
+    output |= BluetoothGattCharacteristic::PROPERTY_NOTIFY;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PROPERTY_INDICATE)
+    output |= BluetoothGattCharacteristic::PROPERTY_INDICATE;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PROPERTY_SIGNED_WRITE)
+    output |= BluetoothGattCharacteristic::PROPERTY_AUTHENTICATED_SIGNED_WRITES;
+  if (input & chromecast::bluetooth_v2_shlib::Gatt::PROPERTY_EXTENDED_PROPS)
+    output |= BluetoothGattCharacteristic::PROPERTY_EXTENDED_PROPERTIES;
+
+  return output;
+}
+
 // Called back when subscribing or unsubscribing to a remote characteristic.
 // If |success| is true, run |callback|. Otherwise run |error_callback|.
 void OnSubscribeOrUnsubscribe(
@@ -52,29 +103,21 @@ BluetoothRemoteGattCharacteristicCast::
     ~BluetoothRemoteGattCharacteristicCast() {}
 
 std::string BluetoothRemoteGattCharacteristicCast::GetIdentifier() const {
-  return chromecast::bluetooth::util::UuidToString(
-      remote_characteristic_->uuid());
+  return GetUUID().canonical_value();
 }
 
 BluetoothUUID BluetoothRemoteGattCharacteristicCast::GetUUID() const {
-  return BluetoothUUID(chromecast::bluetooth::util::UuidToString(
-      remote_characteristic_->uuid()));
+  return UuidToBluetoothUUID(remote_characteristic_->uuid());
 }
 
 BluetoothGattCharacteristic::Properties
 BluetoothRemoteGattCharacteristicCast::GetProperties() const {
-  // TODO(slan): Convert these from
-  // bluetooth_v2_shlib::Characteristic::properties.
-  NOTIMPLEMENTED();
-  return Properties();
+  return ConvertProperties(remote_characteristic_->properties());
 }
 
 BluetoothGattCharacteristic::Permissions
 BluetoothRemoteGattCharacteristicCast::GetPermissions() const {
-  // TODO(slan): Convert these from
-  // bluetooth_v2_shlib::Characteristic::permissions.
-  NOTIMPLEMENTED();
-  return Permissions();
+  return ConvertPermissions(remote_characteristic_->permissions());
 }
 
 const std::vector<uint8_t>& BluetoothRemoteGattCharacteristicCast::GetValue()
@@ -132,9 +175,11 @@ void BluetoothRemoteGattCharacteristicCast::SubscribeToNotifications(
     BluetoothRemoteGattDescriptor* ccc_descriptor,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
+  DVLOG(2) << __func__ << " " << GetIdentifier();
+
   // |remote_characteristic_| exposes a method which writes the CCCD after
   // subscribing the GATT client to the notification. This is syntactically
-  // nicer and saves us a thread-hop, so we can ignore |ccc_descriptor| for now.
+  // nicer and saves us a thread-hop, so we can ignore |ccc_descriptor|.
   (void)ccc_descriptor;
 
   remote_characteristic_->SetRegisterNotification(
@@ -146,14 +191,14 @@ void BluetoothRemoteGattCharacteristicCast::UnsubscribeFromNotifications(
     BluetoothRemoteGattDescriptor* ccc_descriptor,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
+  DVLOG(2) << __func__ << " " << GetIdentifier();
+
   // |remote_characteristic_| exposes a method which writes the CCCD after
-  // subscribing the GATT client to the notification. This is syntactically
-  // nicer and saves us a thread-hop, so we can ignore |ccc_descriptor| for now.
+  // unsubscribing the GATT client from the notification. This is syntactically
+  // nicer and saves us a thread-hop, so we can ignore |ccc_descriptor|.
   (void)ccc_descriptor;
 
-  // TODO(slan|bcf): Should we actually be using SetRegisterNotification() here
-  // to unset the CCCD bit on the peripheral? What does the standard say?
-  remote_characteristic_->SetNotification(
+  remote_characteristic_->SetRegisterNotification(
       false,
       base::BindOnce(&OnSubscribeOrUnsubscribe, callback, error_callback));
 }

device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.h

@@ -56,7 +56,7 @@ class BluetoothRemoteGattCharacteristicCast
 
   // Called by BluetoothAdapterCast to set the value when a new notification
   // comes in.
-  void SetValue(const std::vector<uint8_t>& value) { value_ = value; }
+  void SetValue(std::vector<uint8_t> value) { value_ = std::move(value); }
 
  private:
   // BluetoothRemoteGattCharacteristic implementation:

device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.cc

@@ -9,10 +9,10 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "chromecast/device/bluetooth/bluetooth_util.h"
 #include "chromecast/device/bluetooth/le/remote_descriptor.h"
 #include "device/bluetooth/bluetooth_gatt_service.h"
 #include "device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.h"
+#include "device/bluetooth/cast/bluetooth_utils.h"
 
 namespace device {
 
@@ -26,12 +26,11 @@ BluetoothRemoteGattDescriptorCast::BluetoothRemoteGattDescriptorCast(
 BluetoothRemoteGattDescriptorCast::~BluetoothRemoteGattDescriptorCast() {}
 
 std::string BluetoothRemoteGattDescriptorCast::GetIdentifier() const {
-  return chromecast::bluetooth::util::UuidToString(remote_descriptor_->uuid());
+  return GetUUID().canonical_value();
 }
 
 BluetoothUUID BluetoothRemoteGattDescriptorCast::GetUUID() const {
-  return BluetoothUUID(
-      chromecast::bluetooth::util::UuidToString(remote_descriptor_->uuid()));
+  return UuidToBluetoothUUID(remote_descriptor_->uuid());
 }
 
 BluetoothGattCharacteristic::Permissions

device/bluetooth/cast/bluetooth_remote_gatt_service_cast.cc

@@ -8,11 +8,11 @@
 #include <vector>
 
 #include "base/bind.h"
-#include "chromecast/device/bluetooth/bluetooth_util.h"
 #include "chromecast/device/bluetooth/le/remote_characteristic.h"
 #include "chromecast/device/bluetooth/le/remote_service.h"
 #include "device/bluetooth/cast/bluetooth_device_cast.h"
 #include "device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.h"
+#include "device/bluetooth/cast/bluetooth_utils.h"
 
 namespace device {
 
@@ -34,12 +34,11 @@ BluetoothRemoteGattServiceCast::BluetoothRemoteGattServiceCast(
 BluetoothRemoteGattServiceCast::~BluetoothRemoteGattServiceCast() {}
 
 std::string BluetoothRemoteGattServiceCast::GetIdentifier() const {
-  return chromecast::bluetooth::util::UuidToString(remote_service_->uuid());
+  return GetUUID().canonical_value();
 }
 
 BluetoothUUID BluetoothRemoteGattServiceCast::GetUUID() const {
-  return BluetoothUUID(
-      chromecast::bluetooth::util::UuidToString(remote_service_->uuid()));
+  return UuidToBluetoothUUID(remote_service_->uuid());
 }
 
 bool BluetoothRemoteGattServiceCast::IsPrimary() const {

device/bluetooth/cast/bluetooth_utils.cc

+// Copyright 2018 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/cast/bluetooth_utils.h"
+
+#include "chromecast/device/bluetooth/bluetooth_util.h"
+#include "device/bluetooth/bluetooth_device.h"
+
+namespace device {
+
+std::string GetCanonicalBluetoothAddress(
+    const chromecast::bluetooth_v2_shlib::Addr& addr) {
+  return device::BluetoothDevice::CanonicalizeAddress(
+      chromecast::bluetooth::util::AddrToString(addr));
+}
+
+BluetoothUUID UuidToBluetoothUUID(
+    const chromecast::bluetooth_v2_shlib::Uuid& uuid) {
+  return BluetoothUUID(chromecast::bluetooth::util::UuidToString(uuid));
+}
+
+std::string GetCanonicalBluetoothUuid(
+    const chromecast::bluetooth_v2_shlib::Uuid& uuid) {
+  return UuidToBluetoothUUID(uuid).canonical_value();
+}
+
+}  // namespace device
\ No newline at end of file

device/bluetooth/cast/bluetooth_utils.h

+// Copyright 2018 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.
+
+#ifndef DEVICE_BLUETOOTH_CAST_BLUETOOTH_UTILS_H_
+#define DEVICE_BLUETOOTH_CAST_BLUETOOTH_UTILS_H_
+
+#include <string>
+
+#include "chromecast/public/bluetooth/bluetooth_types.h"
+#include "device/bluetooth/bluetooth_export.h"
+#include "device/bluetooth/bluetooth_uuid.h"
+
+// This file contains common utilities for implementing Chromium bluetooth
+// interfaces with the Cast Bluetooth stack.
+namespace device {
+
+// Return |addr| in the canonical format used by Chromium Bluetooth code,
+// which is a 48-bit mac address with strictly uppercase digits (ex.
+// "AA:BB:CC:DD:EE:FF"). Any class implementing a Bluetooth interface which
+// needs to reference an address should use this function to obtain the correct
+// string.
+std::string DEVICE_BLUETOOTH_EXPORT
+GetCanonicalBluetoothAddress(const chromecast::bluetooth_v2_shlib::Addr& addr);
+
+// Convert |uuid| to BluetoothUUID, the type used by Chromium Bluetooth code.
+device::BluetoothUUID DEVICE_BLUETOOTH_EXPORT
+UuidToBluetoothUUID(const chromecast::bluetooth_v2_shlib::Uuid& uuid);
+
+// Return |uuid| in the canonical format used by Chromium Bluetooth code,
+// which is a 128-bit lowercase uuid:
+// This is the same as calling UuidToBluetoothUUID(uuid).canonical_value().
+std::string DEVICE_BLUETOOTH_EXPORT
+GetCanonicalBluetoothUuid(const chromecast::bluetooth_v2_shlib::Uuid& uuid);
+
+}  // namespace device
+
+#endif  // DEVICE_BLUETOOTH_CAST_BLUETOOTH_UTILS_H_
\ No newline at end of file

device/bluetooth/cast/bluetooth_utils_unittest.cc

+// Copyright 2018 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/cast/bluetooth_utils.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace device {
+
+TEST(BluetoothUtilsTest, TestGetCanonicalBluetoothAddress) {
+  // Test that the correct canonical address is returned for a variety of
+  // addresses.
+  ASSERT_EQ("AA:BB:CC:DD:EE:FF", GetCanonicalBluetoothAddress(
+                                     {{0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa}}));
+  ASSERT_EQ("44:55:66:77:88:99", GetCanonicalBluetoothAddress(
+                                     {{0x99, 0x88, 0x77, 0x66, 0x55, 0x44}}));
+  ASSERT_EQ("00:00:00:00:00:00", GetCanonicalBluetoothAddress(
+                                     {{0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}));
+}
+
+TEST(BluetoothUtilsTest, TestUuidToBluetoothUUID_128bit) {
+  // Test a 128-bit UUID.
+  BluetoothUUID uuid =
+      UuidToBluetoothUUID({{0x12, 0x3e, 0x45, 0x67, 0xe8, 0x9b, 0x12, 0xd3,
+                            0xa4, 0x56, 0x42, 0x66, 0x55, 0x44, 0x00, 0x00}});
+  ASSERT_TRUE(uuid.IsValid());
+  ASSERT_EQ(BluetoothUUID::kFormat128Bit, uuid.format());
+  ASSERT_EQ("123e4567-e89b-12d3-a456-426655440000", uuid.value());
+  ASSERT_EQ("123e4567-e89b-12d3-a456-426655440000", uuid.canonical_value());
+}
+
+TEST(BluetoothUtilsTest, TestUuidToBluetoothUUID_16bit) {
+  // Test a 16-bit UUID. Note that since chromecast::bluetooth_v2_shlib::Uuid
+  // always has 128 bits, the underlying value of every BluetoothUUID returned
+  // from this function will look like "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx".
+  // For information on how a 16-bit UUID is represented as a 128-bit UUID, see
+  // http://www.argenox.com/a-ble-advertising-primer.
+
+  // Get BluetoothUUID for 0xFE34.
+  BluetoothUUID uuid =
+      UuidToBluetoothUUID({{0x00, 0x00, 0xfe, 0x34, 0x00, 0x00, 0x10, 0x00,
+                            0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB}});
+  ASSERT_TRUE(uuid.IsValid());
+  ASSERT_EQ(BluetoothUUID::kFormat128Bit, uuid.format());
+  ASSERT_EQ("0000fe34-0000-1000-8000-00805f9b34fb", uuid.value());
+  ASSERT_EQ("0000fe34-0000-1000-8000-00805f9b34fb", uuid.canonical_value());
+}
+
+TEST(BluetoothUtilsTest, TestGetCanonicalBluetoothUuid) {
+  std::string uuid = GetCanonicalBluetoothUuid(
+      {{0x12, 0x3e, 0x45, 0x67, 0xe8, 0x9b, 0x12, 0xd3, 0xa4, 0x56, 0x42, 0x66,
+        0x55, 0x44, 0x00, 0x00}});
+  ASSERT_EQ("123e4567-e89b-12d3-a456-426655440000", uuid);
+}
+
+}  // namespace device