BluetoothRemoteGATTServer: remove context observer and prefinalizer

With the default HeapMojoRemote setting there was a bad
interaction between service_ and the GATTServer cleanup. service_
would get cleaned up in its own OnContextDestroyed method but
then the GattServer would try to access it in Dispose. This
was worked around by using kForceWithoutContextObserver.

In BluetoothGATTServer there should not be any need for manual
cleanup: service_ can use the OnContextDestroyed method and the
other object being cleaned up, active_algorithms_, is a HeapHashSet
with its own associated garbage collection.

Bug: 1049056
Change-Id: Ia81e99743c1f8bdccf37b54d0761258571a3ad4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2275515Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Johann Koenig <johannkoenig@google.com>
Cr-Commit-Position: refs/heads/master@{#788852}

third_party/blink/renderer/modules/bluetooth/bluetooth.h

@@ -99,9 +99,9 @@ class Bluetooth final : public EventTargetWithInlineData,
                                 Bluetooth>
       client_receivers_;
 
-  HeapMojoRemote<mojom::blink::WebBluetoothService,
-                 HeapMojoWrapperMode::kForceWithoutContextObserver>
-      service_;
+  // HeapMojoRemote objects are associated with a ContextLifecycleNotifier and
+  // cleaned up automatically when it is destroyed.
+  HeapMojoRemote<mojom::blink::WebBluetoothService> service_;
 };
 
 }  // namespace blink

third_party/blink/renderer/modules/bluetooth/bluetooth_remote_gatt_server.cc

@@ -26,15 +26,12 @@ namespace blink {
 
 BluetoothRemoteGATTServer::BluetoothRemoteGATTServer(ExecutionContext* context,
                                                      BluetoothDevice* device)
-    : ExecutionContextLifecycleObserver(context),
+    :  // See https://bit.ly/2S0zRAS for task types.
+      task_runner_(context->GetTaskRunner(TaskType::kMiscPlatformAPI)),
       client_receivers_(this, context),
       device_(device),
       connected_(false) {}
 
-void BluetoothRemoteGATTServer::ContextDestroyed() {
-  Dispose();
-}
-
 void BluetoothRemoteGATTServer::GATTServerDisconnected() {
   DispatchDisconnected();
 }
@@ -54,20 +51,10 @@ bool BluetoothRemoteGATTServer::RemoveFromActiveAlgorithms(
   return true;
 }
 
-void BluetoothRemoteGATTServer::DisconnectIfConnected() {
-  if (connected_) {
-    SetConnected(false);
-    ClearActiveAlgorithms();
-    mojom::blink::WebBluetoothService* service =
-        device_->GetBluetooth()->Service();
-    service->RemoteServerDisconnect(device_->id());
-  }
-}
-
 void BluetoothRemoteGATTServer::CleanupDisconnectedDeviceAndFireEvent() {
   DCHECK(connected_);
-  SetConnected(false);
-  ClearActiveAlgorithms();
+  connected_ = false;
+  active_algorithms_.clear();
   device_->ClearAttributeInstanceMapAndFireEvent();
 }
 
@@ -78,16 +65,11 @@ void BluetoothRemoteGATTServer::DispatchDisconnected() {
   CleanupDisconnectedDeviceAndFireEvent();
 }
 
-void BluetoothRemoteGATTServer::Dispose() {
-  DisconnectIfConnected();
-}
-
 void BluetoothRemoteGATTServer::Trace(Visitor* visitor) const {
   visitor->Trace(client_receivers_);
   visitor->Trace(active_algorithms_);
   visitor->Trace(device_);
   ScriptWrappable::Trace(visitor);
-  ExecutionContextLifecycleObserver::Trace(visitor);
 }
 
 void BluetoothRemoteGATTServer::ConnectCallback(
@@ -98,7 +80,7 @@ void BluetoothRemoteGATTServer::ConnectCallback(
     return;
 
   if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
-    SetConnected(true);
+    connected_ = true;
     resolver->Resolve(this);
   } else {
     resolver->Reject(BluetoothError::CreateDOMException(result));
@@ -112,11 +94,8 @@ ScriptPromise BluetoothRemoteGATTServer::connect(ScriptState* script_state) {
   mojom::blink::WebBluetoothService* service =
       device_->GetBluetooth()->Service();
   mojo::PendingAssociatedRemote<mojom::blink::WebBluetoothServerClient> client;
-  // See https://bit.ly/2S0zRAS for task types.
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-      GetExecutionContext()->GetTaskRunner(TaskType::kMiscPlatformAPI);
   client_receivers_.Add(client.InitWithNewEndpointAndPassReceiver(),
-                        std::move(task_runner));
+                        task_runner_);
 
   service->RemoteServerConnect(
       device_->id(), std::move(client),

third_party/blink/renderer/modules/bluetooth/bluetooth_remote_gatt_server.h

@@ -27,23 +27,16 @@ class ScriptState;
 // bluetooth peripheral.
 class BluetoothRemoteGATTServer
     : public ScriptWrappable,
-      public ExecutionContextLifecycleObserver,
       public mojom::blink::WebBluetoothServerClient {
-  USING_PRE_FINALIZER(BluetoothRemoteGATTServer, Dispose);
   DEFINE_WRAPPERTYPEINFO();
   USING_GARBAGE_COLLECTED_MIXIN(BluetoothRemoteGATTServer);
 
  public:
   BluetoothRemoteGATTServer(ExecutionContext*, BluetoothDevice*);
 
-  // ExecutionContextLifecycleObserver:
-  void ContextDestroyed() override;
-
   // mojom::blink::WebBluetoothServerClient:
   void GATTServerDisconnected() override;
 
-  void SetConnected(bool connected) { connected_ = connected; }
-
   // The Active Algorithms set is maintained so that disconnection, i.e.
   // disconnect() method or the device disconnecting by itself, can be detected
   // by algorithms. They check via RemoveFromActiveAlgorithms that their
@@ -55,8 +48,6 @@ class BluetoothRemoteGATTServer
   // Removes |resolver| from the set of Active Algorithms if it was in the set
   // and returns true, false otherwise.
   bool RemoveFromActiveAlgorithms(ScriptPromiseResolver*);
-  // Removes all ScriptPromiseResolvers from the set of Active Algorithms.
-  void ClearActiveAlgorithms() { active_algorithms_.clear(); }
 
   // If gatt is connected then sets gatt.connected to false and disconnects.
   // This function only performs the necessary steps to ensure a device
@@ -70,10 +61,6 @@ class BluetoothRemoteGATTServer
 
   void DispatchDisconnected();
 
-  // USING_PRE_FINALIZER interface.
-  // Called before the object gets garbage collected.
-  void Dispose();
-
   // Interface required by Garbage Collectoin:
   void Trace(Visitor*) const override;
 
@@ -111,9 +98,9 @@ class BluetoothRemoteGATTServer
   // using this server’s connection.
   HeapHashSet<Member<ScriptPromiseResolver>> active_algorithms_;
 
+  const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   HeapMojoAssociatedReceiverSet<mojom::blink::WebBluetoothServerClient,
-                                BluetoothRemoteGATTServer,
-                                HeapMojoWrapperMode::kWithoutContextObserver>
+                                BluetoothRemoteGATTServer>
       client_receivers_;
 
   Member<BluetoothDevice> device_;