[Onion Soup] Move WebBluetoothImpl from //content/renderer/bluetooth to Blink's bluetooth module

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 229ef2de46b0675a62dea0a7bffbcbd01827c64c..9127934166c309654cabfc18a0b3327dc0f9c231 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -4,6 +4,7 @@
 
 #include "content/browser/frame_host/render_frame_host_impl.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ -3229,19 +3230,28 @@ void RenderFrameHostImpl::AXContentTreeDataToAXTreeData(
 
 WebBluetoothServiceImpl* RenderFrameHostImpl::CreateWebBluetoothService(
     blink::mojom::WebBluetoothServiceRequest request) {
-  DCHECK(!web_bluetooth_service_);
-  web_bluetooth_service_.reset(
-      new WebBluetoothServiceImpl(this, std::move(request)));
-  // RFHI owns web_bluetooth_service_ and web_bluetooth_service owns the
-  // binding_ which may run the error handler. binding_ can't run the error
+  // RFHI owns |web_bluetooth_services_| and |web_bluetooth_service| owns the
+  // |binding_| which may run the error handler. |binding_| can't run the error
   // handler after it's destroyed so it can't run after the RFHI is destroyed.
-  web_bluetooth_service_->SetClientConnectionErrorHandler(base::Bind(
-      &RenderFrameHostImpl::DeleteWebBluetoothService, base::Unretained(this)));
-  return web_bluetooth_service_.get();
-}
-
-void RenderFrameHostImpl::DeleteWebBluetoothService() {
-  web_bluetooth_service_.reset();
+  std::unique_ptr<WebBluetoothServiceImpl> web_bluetooth_service =

[dcheng, 2016/12/27 20:37:37]

Nit: OK to use auto here.

[juncai, 2016/12/27 21:55:50]

Done.

+      base::MakeUnique<WebBluetoothServiceImpl>(this, std::move(request));
+  web_bluetooth_service->SetClientConnectionErrorHandler(
+      base::Bind(&RenderFrameHostImpl::DeleteWebBluetoothService,
+                 base::Unretained(this), web_bluetooth_service.get()));
+  web_bluetooth_services_.push_back(std::move(web_bluetooth_service));

[dcheng, 2016/12/27 20:37:37]

That being said, I've read the code a bit more: I'm wondering why this isn't
just using strong binding? Why does the RFH need to maintain a list of these?

Is it because there are concerns about the lifetimes of the bluetooth service vs
the RFH?

[Reilly Grant (use Gerrit), 2016/12/27 20:45:47]

Yes, this is a common pattern for Mojo service implementations that hold a
RenderFrameHost* (or RenderProcessHost*).

+  return web_bluetooth_services_.back().get();
+}
+
+void RenderFrameHostImpl::DeleteWebBluetoothService(
+    WebBluetoothServiceImpl* web_bluetooth_service) {
+  auto it = std::find_if(
+      web_bluetooth_services_.begin(), web_bluetooth_services_.end(),
+      [web_bluetooth_service](
+          const std::unique_ptr<WebBluetoothServiceImpl>& service) {
+        return web_bluetooth_service == service.get();
+      });
+  DCHECK(it != web_bluetooth_services_.end());
+  web_bluetooth_services_.erase(it);
 }
 
 void RenderFrameHostImpl::Create(