Refactor WebBluetoothServiceClient in the web_bluetooth.mojom

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

Index: third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp
index c462463ac6a8bed03d4d50ec643c311a70d3615c..9efeb90cbf9a8d15fe8b2984f430f8378690ecdd 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp
@@ -30,6 +30,7 @@ BluetoothRemoteGATTCharacteristic::BluetoothRemoteGATTCharacteristic(
     : ContextLifecycleObserver(context),
       m_characteristic(std::move(characteristic)),
       m_service(service),
+      m_clientBinding(this),
       m_stopped(false),
       m_device(device) {
   m_properties =
@@ -49,27 +50,29 @@ 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);
   }
 }
 
@@ -87,12 +90,6 @@ 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(
@@ -224,7 +221,8 @@ ScriptPromise BluetoothRemoteGATTCharacteristic::writeValue(
 
 void BluetoothRemoteGATTCharacteristic::NotificationsCallback(
     ScriptPromiseResolver* resolver,
-    mojom::blink::WebBluetoothResult result) {
+    mojom::blink::WebBluetoothResult result,
+    mojom::blink::WebBluetoothCharacteristicClientAssociatedRequest request) {
   if (!resolver->getExecutionContext() ||
       resolver->getExecutionContext()->isContextDestroyed())
     return;
@@ -236,6 +234,8 @@ void BluetoothRemoteGATTCharacteristic::NotificationsCallback(
     return;
   }
 
+  m_clientBinding.Bind(std::move(request));
+
   if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
     resolver->resolve(this);
   } else {
@@ -301,7 +301,7 @@ ScriptPromise BluetoothRemoteGATTCharacteristic::stopNotifications(
       convertToBaseCallback(
           WTF::bind(&BluetoothRemoteGATTCharacteristic::NotificationsCallback,
                     wrapPersistent(this), wrapPersistent(resolver),
-                    mojom::blink::WebBluetoothResult::SUCCESS)));
+                    mojom::blink::WebBluetoothResult::SUCCESS, nullptr)));
   return promise;
 }