Refactor WebBluetoothServiceClient in the web_bluetooth.mojom

content/browser/bluetooth/web_bluetooth_service_impl.cc

Index: content/browser/bluetooth/web_bluetooth_service_impl.cc
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl.cc b/content/browser/bluetooth/web_bluetooth_service_impl.cc
index bb410e7e9f60ade0a48b30e3beb7af996a02bd76..16caad52b6366d65c7f7e419f3fda1b4184db494 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl.cc
+++ b/content/browser/bluetooth/web_bluetooth_service_impl.cc
@@ -233,9 +233,6 @@ void WebBluetoothServiceImpl::DeviceChanged(device::BluetoothAdapter* adapter,
     base::Optional<WebBluetoothDeviceId> device_id =
         connected_devices_->CloseConnectionToDeviceWithAddress(
             device->GetAddress());
-    if (device_id && client_) {
-      client_->GattServerDisconnected(device_id.value());
-    }
   }
 }
 
@@ -292,18 +289,13 @@ void WebBluetoothServiceImpl::GattCharacteristicValueChanged(
 void WebBluetoothServiceImpl::NotifyCharacteristicValueChanged(
     const std::string& characteristic_instance_id,
     const std::vector<uint8_t>& value) {
-  if (client_) {
-    client_->RemoteCharacteristicValueChanged(characteristic_instance_id,
-                                              value);
+  auto iter =
+      characteristic_id_to_notify_session_.find(characteristic_instance_id);

[ortuno, 2017/03/06 11:31:20]

You can move all this logic up to GattCharacteristicValueChanged now that reads
don't fire this event.

[juncai, 2017/03/09 07:30:57]

Done.

+  if (iter != characteristic_id_to_notify_session_.end()) {
+    iter->second.second->RemoteCharacteristicValueChanged(value);
   }
 }
 
-void WebBluetoothServiceImpl::SetClient(
-    blink::mojom::WebBluetoothServiceClientAssociatedPtrInfo client) {
-  DCHECK(!client_.get());
-  client_.Bind(std::move(client));
-}
-
 void WebBluetoothServiceImpl::RequestDevice(
     blink::mojom::WebBluetoothRequestDeviceOptionsPtr options,
     const RequestDeviceCallback& callback) {
@@ -338,13 +330,14 @@ void WebBluetoothServiceImpl::RemoteServerConnect(
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordConnectGATTOutcome(query_result.outcome);
-    callback.Run(query_result.GetWebResult());
+    callback.Run(query_result.GetWebResult(), nullptr /* client_request */);
     return;
   }
 
   if (connected_devices_->IsConnectedToDeviceWithId(device_id)) {
     DVLOG(1) << "Already connected.";
-    callback.Run(blink::mojom::WebBluetoothResult::SUCCESS);
+    callback.Run(blink::mojom::WebBluetoothResult::SUCCESS,
+                 nullptr /* client_request */);
     return;
   }
 
@@ -689,10 +682,11 @@ void WebBluetoothServiceImpl::RemoteCharacteristicStartNotifications(
   auto iter =
       characteristic_id_to_notify_session_.find(characteristic_instance_id);
   if (iter != characteristic_id_to_notify_session_.end() &&
-      iter->second->IsActive()) {
+      iter->second.first->IsActive()) {
     // If the frame has already started notifications and the notifications
     // are active we return SUCCESS.
-    callback.Run(blink::mojom::WebBluetoothResult::SUCCESS);
+    callback.Run(blink::mojom::WebBluetoothResult::SUCCESS,
+                 nullptr /* client_request */);
     return;
   }
 
@@ -705,7 +699,7 @@ void WebBluetoothServiceImpl::RemoteCharacteristicStartNotifications(
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordStartNotificationsOutcome(query_result.outcome);
-    callback.Run(query_result.GetWebResult());
+    callback.Run(query_result.GetWebResult(), nullptr /* client_request */);
     return;
   }
 
@@ -714,7 +708,8 @@ void WebBluetoothServiceImpl::RemoteCharacteristicStartNotifications(
       (device::BluetoothRemoteGattCharacteristic::PROPERTY_NOTIFY |
        device::BluetoothRemoteGattCharacteristic::PROPERTY_INDICATE);
   if (!notify_or_indicate) {
-    callback.Run(blink::mojom::WebBluetoothResult::GATT_NOT_SUPPORTED);
+    callback.Run(blink::mojom::WebBluetoothResult::GATT_NOT_SUPPORTED,
+                 nullptr /* client_request */);
     return;
   }
 
@@ -747,7 +742,7 @@ void WebBluetoothServiceImpl::RemoteCharacteristicStopNotifications(
     callback.Run();
     return;
   }
-  notify_session_iter->second->Stop(base::Bind(
+  notify_session_iter->second.first->Stop(base::Bind(
       &WebBluetoothServiceImpl::OnStopNotifySessionComplete,
       weak_ptr_factory_.GetWeakPtr(), characteristic_instance_id, callback));
 }
@@ -957,8 +952,18 @@ void WebBluetoothServiceImpl::OnCreateGATTConnectionSuccess(
   RecordConnectGATTTimeSuccess(base::TimeTicks::Now() - start_time);
   RecordConnectGATTOutcome(UMAConnectGATTOutcome::SUCCESS);
 
-  connected_devices_->Insert(device_id, std::move(connection));
-  callback.Run(blink::mojom::WebBluetoothResult::SUCCESS);
+  if (connected_devices_->IsConnectedToDeviceWithId(device_id)) {

[ortuno, 2017/03/06 11:31:20]

// It's possible for WebBluetoothServiceImpl to issue two successive
  // connection requests for which it would get two successive responses
  // and consequently try to insert two BluetoothGattConnections for the
  // same device. WebBluetoothServiceImpl should reject or queue connection
  // requests if there is a pending connection already, but the platform
  // abstraction doesn't currently support checking for pending connections.
  // TODO(crbug.com/583544): CHECK that this never happens once the platform
  // abstraction allows to check for pending connections.

I wonder if we should remove this comment from
frame_connected_bluetooth_devices.cc since we already perform this check here.
WDYT?

[juncai, 2017/03/09 07:30:57]

Done.

+    DVLOG(1) << "Already connected.";
+    callback.Run(blink::mojom::WebBluetoothResult::SUCCESS,
+                 nullptr /* client_request */);
+    return;
+  }
+
+  blink::mojom::WebBluetoothServerClientAssociatedPtr client;
+  callback.Run(blink::mojom::WebBluetoothResult::SUCCESS,
+               mojo::MakeRequest(&client));
+  connected_devices_->Insert(device_id, std::move(connection),
+                             std::move(client));
 }
 
 void WebBluetoothServiceImpl::OnCreateGATTConnectionFailed(
@@ -967,7 +972,8 @@ void WebBluetoothServiceImpl::OnCreateGATTConnectionFailed(
     device::BluetoothDevice::ConnectErrorCode error_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordConnectGATTTimeFailed(base::TimeTicks::Now() - start_time);
-  callback.Run(TranslateConnectErrorAndRecord(error_code));
+  callback.Run(TranslateConnectErrorAndRecord(error_code),
+               nullptr /* client_request */);
 }
 
 void WebBluetoothServiceImpl::OnCharacteristicReadValueSuccess(
@@ -1010,10 +1016,24 @@ void WebBluetoothServiceImpl::OnStartNotifySessionSuccess(
   // compilers may evaluate arguments in any order.
   std::string characteristic_instance_id =
       notify_session->GetCharacteristicIdentifier();
+
+  auto iter =
+      characteristic_id_to_notify_session_.find(characteristic_instance_id);
+  if (iter != characteristic_id_to_notify_session_.end() &&
+      iter->second.first->IsActive()) {
+    // If the frame has already started notifications and the notifications
+    // are active we return SUCCESS.
+    callback.Run(blink::mojom::WebBluetoothResult::SUCCESS,
+                 nullptr /* client_request */);
+    return;
+  }
+
+  blink::mojom::WebBluetoothCharacteristicClientAssociatedPtr client;
+  callback.Run(blink::mojom::WebBluetoothResult::SUCCESS,
+               mojo::MakeRequest(&client));
   // Saving the BluetoothGattNotifySession keeps notifications active.
   characteristic_id_to_notify_session_[characteristic_instance_id] =
-      std::move(notify_session);
-  callback.Run(blink::mojom::WebBluetoothResult::SUCCESS);
+      std::make_pair(std::move(notify_session), std::move(client));
 }
 
 void WebBluetoothServiceImpl::OnStartNotifySessionFailed(
@@ -1021,7 +1041,8 @@ void WebBluetoothServiceImpl::OnStartNotifySessionFailed(
     device::BluetoothRemoteGattService::GattErrorCode error_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   callback.Run(TranslateGATTErrorAndRecord(
-      error_code, UMAGATTOperation::START_NOTIFICATIONS));
+                   error_code, UMAGATTOperation::START_NOTIFICATIONS),
+               nullptr /* client_request */);
 }
 
 void WebBluetoothServiceImpl::OnStopNotifySessionComplete(