Use a mojo::StrongBindingSet to manage PermissionServiceImpls

content/browser/permissions/permission_service_context.cc

 // Copyright 2014 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 "content/browser/permissions/permission_service_context.h"
 
 #include <utility>
 
 #include "content/browser/permissions/permission_service_impl.h"
 #include "content/public/browser/browser_context.h"
@@ skipping 54 matching lines @@
     RenderProcessHost* render_process_host)
     : WebContentsObserver(nullptr),
       render_frame_host_(nullptr),
       render_process_host_(render_process_host) {
 }
 
 PermissionServiceContext::~PermissionServiceContext() {
 }
 
 void PermissionServiceContext::CreateService(
-    mojo::InterfaceRequest<blink::mojom::PermissionService> request) {
-  services_.push_back(
-      base::MakeUnique<PermissionServiceImpl>(this, std::move(request)));
+    blink::mojom::PermissionServiceRequest request) {
+  services_.AddBinding(base::MakeUnique<PermissionServiceImpl>(this),
+                       std::move(request));
 }
 
 void PermissionServiceContext::CreateSubscription(
     PermissionType permission_type,
     const url::Origin& origin,
     PermissionObserverPtr observer) {
   BrowserContext* browser_context = GetBrowserContext();
   DCHECK(browser_context);
   if (!browser_context->GetPermissionManager())
     return;
 
   auto subscription =
       base::MakeUnique<PermissionSubscription>(this, std::move(observer));
   GURL requesting_origin(origin.Serialize());
   GURL embedding_origin = GetEmbeddingOrigin();
   int subscription_id =
       browser_context->GetPermissionManager()->SubscribePermissionStatusChange(
           permission_type, requesting_origin,
           // If the embedding_origin is empty, we'll use the |origin| instead.
           embedding_origin.is_empty() ? requesting_origin : embedding_origin,
           base::Bind(&PermissionSubscription::OnPermissionStatusChanged,
                      base::Unretained(subscription.get())));
   subscription->set_id(subscription_id);
   subscriptions_[subscription_id] = std::move(subscription);
 }
 
-void PermissionServiceContext::ServiceHadConnectionError(
-    PermissionServiceImpl* service) {
-  auto it = std::find_if(
-      services_.begin(), services_.end(),
-      [service](const std::unique_ptr<PermissionServiceImpl>& this_service) {
-        return service == this_service.get();
-      });
-  DCHECK(it != services_.end());
-  services_.erase(it);
-}
-
 void PermissionServiceContext::ObserverHadConnectionError(int subscription_id) {
   auto it = subscriptions_.find(subscription_id);
   DCHECK(it != subscriptions_.end());
   subscriptions_.erase(it);
 }
 
 void PermissionServiceContext::RenderFrameHostChanged(
     RenderFrameHost* old_host,
     RenderFrameHost* new_host) {
   CancelPendingOperations(old_host);
 }
 
 void PermissionServiceContext::FrameDeleted(
     RenderFrameHost* render_frame_host) {
   CancelPendingOperations(render_frame_host);
 }
 
 void PermissionServiceContext::DidFinishNavigation(
     NavigationHandle* navigation_handle) {
   if (!navigation_handle->HasCommitted() || navigation_handle->IsSameDocument())
     return;
 
   CancelPendingOperations(navigation_handle->GetRenderFrameHost());
 }
 
 void PermissionServiceContext::CancelPendingOperations(

[raymes, 2017/04/30 23:25:24]

Maybe we should call this CloseBindings (or something) instead of
CancelPendingOperations?

[Reilly Grant (use Gerrit), 2017/05/01 19:40:51]

Done.

     RenderFrameHost* render_frame_host) {
   DCHECK(render_frame_host_);
   if (render_frame_host != render_frame_host_)
     return;
 
-  for (const auto& service : services_)
-    service->CancelPendingOperations();

[raymes, 2017/04/28 02:46:00]

Please bear with my ignorance because my mojo-foo isn't very good.

Previously this looks like it would have cancelled the operations but the
interfaces would still have been bound to the message pipes? However now it
looks like the interfaces will be unbound. Is that significant?

[Reilly Grant (use Gerrit), 2017/04/28 17:52:22]

It's a good question. Yes, before the requests would be cancelled and now the
pipes are closed. The reason I did this is because mojo::BindingSet doesn't
provide a way to iterate over the interface implementations to run a method,
only to close the pipes. The reason I believe this is okay is that the
situations in which we call this method (RFH changed, RFH deleted and
navigation) are all cases in which the renderer side of the pipe will be closed
anyways.

-
+  services_.CloseAllBindings();
   subscriptions_.clear();
 }
 
 BrowserContext* PermissionServiceContext::GetBrowserContext() const {
   if (!web_contents()) {
     DCHECK(render_process_host_);
     return render_process_host_->GetBrowserContext();
   }
   return web_contents()->GetBrowserContext();
 }
 
 GURL PermissionServiceContext::GetEmbeddingOrigin() const {
   return web_contents() ? web_contents()->GetLastCommittedURL().GetOrigin()
                         : GURL();
 }
 
 RenderFrameHost* PermissionServiceContext::render_frame_host() const {
   return render_frame_host_;
 }
 
 } // namespace content