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/events/Event.h"
 #include "core/inspector/ConsoleMessage.h"
@@ skipping 12 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_clientBinding(this),
       m_stopped(false),
       m_device(device) {
   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::contextDestroyed(ExecutionContext*) {
+  notifyCharacteristicObjectRemoved();
+}
+
+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();
-}
-
 void BluetoothRemoteGATTCharacteristic::dispose() {
+  // The pipe to this object must be closed when is marked unreachable to

[ortuno, 2017/02/24 03:28:52]

notifyCharacteristicObjectRemoved is used so that when the context gets
destroyed e.g. when reloading the page or when the object is garbage collected
we stop notifications. If you only close the pipe when the object is garbage
collected i.e. dispose is called then the characteristic will still be
subscribed to notifications if you reload a page and GC hasn't run yet.

You should move this code to notifyCharacteristicObjectRemoved(). The binding
being bound will work as the m_stopped so you don't need it anymore.

[juncai, 2017/03/01 02:04:12]

Done.

+  // prevent messages from being dispatched before lazy sweeping.
+  if (m_clientBinding.is_bound())
+    m_clientBinding.Close();
   notifyCharacteristicObjectRemoved();
 }
 
 void BluetoothRemoteGATTCharacteristic::notifyCharacteristicObjectRemoved() {
   if (!m_stopped) {
     m_stopped = true;
-    m_device->bluetooth()->characteristicObjectRemoved(
-        m_characteristic->instance_id);
   }
 }
 
 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;
 
@@ skipping 111 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(BluetoothRemoteGATTUtils::CreateDOMException(
         BluetoothRemoteGATTUtils::ExceptionType::kGATTServerDisconnected));
     return;
   }
 
+  m_clientBinding.Bind(std::move(request));
+
   if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
     resolver->resolve(this);
   } else {
     resolver->reject(BluetoothError::take(resolver, result));
   }
 }
 
 ScriptPromise BluetoothRemoteGATTCharacteristic::startNotifications(
     ScriptState* scriptState) {
   if (!getGatt()->connected()) {
@@ skipping 45 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 106 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