Refactor WebBluetoothServiceClient in the web_bluetooth.mojom

third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp

 // 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.
 
 #include "modules/bluetooth/BluetoothRemoteGATTCharacteristic.h"
 
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMException.h"
 #include "core/events/Event.h"
@@ skipping 13 matching lines @@
 namespace blink {
 
 BluetoothRemoteGATTCharacteristic::BluetoothRemoteGATTCharacteristic(
     ExecutionContext* context,
     mojom::blink::WebBluetoothRemoteGATTCharacteristicPtr characteristic,
     BluetoothRemoteGATTService* service,
     BluetoothDevice* device)
     : ContextLifecycleObserver(context),
       m_characteristic(std::move(characteristic)),
       m_service(service),
-      m_stopped(false),
-      m_device(device) {
+      m_device(device),
+      m_clientBinding(this) {
   m_properties =
       BluetoothCharacteristicProperties::create(m_characteristic->properties);
 }
 
 BluetoothRemoteGATTCharacteristic* BluetoothRemoteGATTCharacteristic::create(
     ExecutionContext* context,
     mojom::blink::WebBluetoothRemoteGATTCharacteristicPtr characteristic,
     BluetoothRemoteGATTService* service,
     BluetoothDevice* device) {
   return new BluetoothRemoteGATTCharacteristic(
       context, std::move(characteristic), service, device);
 }
 
 void BluetoothRemoteGATTCharacteristic::setValue(DOMDataView* domDataView) {
   m_value = domDataView;
 }
 
-void BluetoothRemoteGATTCharacteristic::dispatchCharacteristicValueChanged(
-    const Vector<uint8_t>& value) {
+void BluetoothRemoteGATTCharacteristic::RemoteCharacteristicValueChanged(
+    const WTF::Vector<uint8_t>& value) {
   if (!getGatt()->connected())
     return;
   this->setValue(BluetoothRemoteGATTUtils::ConvertWTFVectorToDataView(value));
   dispatchEvent(Event::create(EventTypeNames::characteristicvaluechanged));
 }
 
 void BluetoothRemoteGATTCharacteristic::contextDestroyed(ExecutionContext*) {
-  notifyCharacteristicObjectRemoved();
+  dispose();
 }
 
 void BluetoothRemoteGATTCharacteristic::dispose() {
-  notifyCharacteristicObjectRemoved();
-}
-
-void BluetoothRemoteGATTCharacteristic::notifyCharacteristicObjectRemoved() {
-  if (!m_stopped) {
-    m_stopped = true;
-    m_device->bluetooth()->characteristicObjectRemoved(
-        m_characteristic->instance_id);
-  }
+  if (m_clientBinding.is_bound())
+    m_clientBinding.Close();
 }
 
 const WTF::AtomicString& BluetoothRemoteGATTCharacteristic::interfaceName()
     const {
   return EventTargetNames::BluetoothRemoteGATTCharacteristic;
 }
 
 ExecutionContext* BluetoothRemoteGATTCharacteristic::getExecutionContext()
     const {
   return ContextLifecycleObserver::getExecutionContext();
 }
 
 void BluetoothRemoteGATTCharacteristic::addedEventListener(
     const AtomicString& eventType,
     RegisteredEventListener& registeredListener) {
   EventTargetWithInlineData::addedEventListener(eventType, registeredListener);
-  // We will also need to unregister a characteristic once all the event
-  // listeners have been removed. See http://crbug.com/541390
-  if (eventType == EventTypeNames::characteristicvaluechanged) {
-    m_device->bluetooth()->registerCharacteristicObject(
-        m_characteristic->instance_id, this);
-  }
 }
 
 void BluetoothRemoteGATTCharacteristic::ReadValueCallback(
     ScriptPromiseResolver* resolver,
     mojom::blink::WebBluetoothResult result,
     const Optional<Vector<uint8_t>>& value) {
   if (!resolver->getExecutionContext() ||
       resolver->getExecutionContext()->isContextDestroyed())
     return;
 
   // If the device is disconnected, reject.
   if (!getGatt()->RemoveFromActiveAlgorithms(resolver)) {
     resolver->reject(BluetoothError::createDOMException(
         blink::mojom::WebBluetoothResult::GATT_SERVER_DISCONNECTED));
     return;
   }
 
   if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
     DCHECK(value);
     DOMDataView* domDataView =
         BluetoothRemoteGATTUtils::ConvertWTFVectorToDataView(value.value());
     setValue(domDataView);
+    dispatchEvent(Event::create(EventTypeNames::characteristicvaluechanged));
     resolver->resolve(domDataView);
   } else {
     resolver->reject(BluetoothError::createDOMException(result));
   }
 }
 
 ScriptPromise BluetoothRemoteGATTCharacteristic::readValue(
     ScriptState* scriptState) {
   if (!getGatt()->connected()) {
     return ScriptPromise::rejectWithDOMException(
@@ skipping 85 matching lines @@
       m_characteristic->instance_id, valueVector,
       convertToBaseCallback(WTF::bind(
           &BluetoothRemoteGATTCharacteristic::WriteValueCallback,
           wrapPersistent(this), wrapPersistent(resolver), valueVector)));
 
   return promise;
 }
 
 void BluetoothRemoteGATTCharacteristic::NotificationsCallback(
     ScriptPromiseResolver* resolver,
-    mojom::blink::WebBluetoothResult result) {
+    mojom::blink::WebBluetoothResult result,
+    mojom::blink::WebBluetoothCharacteristicClientAssociatedRequest request) {
   if (!resolver->getExecutionContext() ||
       resolver->getExecutionContext()->isContextDestroyed())
     return;
 
   // If the device is disconnected, reject.
   if (!getGatt()->RemoveFromActiveAlgorithms(resolver)) {
     resolver->reject(BluetoothError::createDOMException(
         blink::mojom::WebBluetoothResult::GATT_SERVER_DISCONNECTED));
     return;
   }
 
+  m_clientBinding.Bind(std::move(request));

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

Wouldn't we bind an invalid request for:

startNotifications();
startNotifications();

? We should probably add a test if there is a bug here.

Also please move inside SUCCESS if statement, so that it's obvious that we only
bind for success.

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

Done.

+
   if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
     resolver->resolve(this);
   } else {
     resolver->reject(BluetoothError::createDOMException(result));
   }
 }
 
 ScriptPromise BluetoothRemoteGATTCharacteristic::startNotifications(
     ScriptState* scriptState) {
   if (!getGatt()->connected()) {
@@ skipping 41 matching lines @@
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
   getGatt()->AddToActiveAlgorithms(resolver);
 
   mojom::blink::WebBluetoothService* service = m_device->bluetooth()->service();
   service->RemoteCharacteristicStopNotifications(
       m_characteristic->instance_id,
       convertToBaseCallback(
           WTF::bind(&BluetoothRemoteGATTCharacteristic::NotificationsCallback,
                     wrapPersistent(this), wrapPersistent(resolver),
-                    mojom::blink::WebBluetoothResult::SUCCESS)));
+                    mojom::blink::WebBluetoothResult::SUCCESS, nullptr)));
   return promise;
 }
 
 ScriptPromise BluetoothRemoteGATTCharacteristic::getDescriptor(
     ScriptState* scriptState,
     const StringOrUnsignedLong& descriptorUUID,
     ExceptionState& exceptionState) {
   String descriptor =
       BluetoothUUID::getDescriptor(descriptorUUID, exceptionState);
   if (exceptionState.hadException())
@@ skipping 121 matching lines @@
 DEFINE_TRACE(BluetoothRemoteGATTCharacteristic) {
   visitor->trace(m_service);
   visitor->trace(m_properties);
   visitor->trace(m_value);
   visitor->trace(m_device);
   EventTargetWithInlineData::trace(visitor);
   ContextLifecycleObserver::trace(visitor);
 }
 
 }  // namespace blink