Convert PushMessaging implementation and clients to new Mojo types

This converts the GetPushMessaging() method from DocumentInterfaceBroker
along with the relevant parts of the implementation of mojom::PushMessaging
at chrome::PushMessagingManager, and the two classes that use it from Blink,
that is, blink::PushProvider and blink::PushMessagingClient.

Note that, for now, this still relies on implicit conversions to get
a PendingReceiver<T> from InterfaceRequest<T> in PushMessagingManager,
which should go away once RenderProcessHostImpl has migrated away
from service_manager::BinderRegistry, to service_manager::BinderMap.

Also, it needs to keep using the InterfaceProvider in PushProvider to bind
the remote interface to avoid thread-related issues that would happen if
we tried to use service_manager::Connector now, since that requires being
in the main thread to run Connect(), and that code usually runs from the
ServiceWorker thread instead. This should stop being a problem once we
have a ServiceWorkers-based InterfaceBroker, at which point the code can
be fully migrated to service_manager::Connector.

Bug: 955171
Change-Id: I62fb6d8b8e1a5872a14740260ea2bca14daf1a15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1655536Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Cr-Commit-Position: refs/heads/master@{#669593}

content/browser/frame_host/render_frame_host_impl.cc

@@ -6083,7 +6083,7 @@ void RenderFrameHostImpl::GetCredentialManager(
 }
 
 void RenderFrameHostImpl::GetPushMessaging(
-    blink::mojom::PushMessagingRequest request) {
+    mojo::PendingReceiver<blink::mojom::PushMessaging> receiver) {
   if (!push_messaging_manager_) {
     push_messaging_manager_.reset(new PushMessagingManager(
         GetProcess()->GetID(), routing_id_,
@@ -6093,8 +6093,9 @@ void RenderFrameHostImpl::GetPushMessaging(
 
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&PushMessagingManager::BindRequest,
-                     push_messaging_manager_->AsWeakPtr(), std::move(request)));
+      base::BindOnce(&PushMessagingManager::AddPushMessagingReceiver,
+                     push_messaging_manager_->AsWeakPtr(),
+                     std::move(receiver)));
 }
 
 void RenderFrameHostImpl::GetVirtualAuthenticatorManager(

content/browser/frame_host/render_frame_host_impl.h

@@ -1407,7 +1407,8 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void GetCredentialManager(
       blink::mojom::CredentialManagerRequest request) override;
   void GetAuthenticator(blink::mojom::AuthenticatorRequest request) override;
-  void GetPushMessaging(blink::mojom::PushMessagingRequest request) override;
+  void GetPushMessaging(
+      mojo::PendingReceiver<blink::mojom::PushMessaging> receiver) override;
   void GetVirtualAuthenticatorManager(
       blink::test::mojom::VirtualAuthenticatorManagerRequest request) override;
   void RegisterAppCacheHost(blink::mojom::AppCacheHostRequest host_request,

content/browser/push_messaging/push_messaging_manager.cc

@@ -5,6 +5,7 @@
 #include "content/browser/push_messaging/push_messaging_manager.h"
 
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ -292,9 +293,15 @@ PushMessagingManager::PushMessagingManager(
 
 PushMessagingManager::~PushMessagingManager() {}
 
+void PushMessagingManager::AddPushMessagingReceiver(
+    mojo::PendingReceiver<blink::mojom::PushMessaging> receiver) {
+  receivers_.Add(this, std::move(receiver));
+}
+
 void PushMessagingManager::BindRequest(
     blink::mojom::PushMessagingRequest request) {
-  bindings_.AddBinding(this, std::move(request));
+  // Implicit conversion to mojo::PendingReceiver<blink::mojom::PushMessaging>.
+  AddPushMessagingReceiver(std::move(request));
 }
 
 // Subscribe methods on both IO and UI threads, merged in order of use from

content/browser/push_messaging/push_messaging_manager.h

@@ -14,7 +14,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "content/public/browser/browser_thread.h"
-#include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
 #include "third_party/blink/public/common/service_worker/service_worker_status_code.h"
 #include "third_party/blink/public/mojom/push_messaging/push_messaging.mojom.h"
 #include "url/gurl.h"
@@ -41,6 +41,11 @@ class PushMessagingManager : public blink::mojom::PushMessaging {
                        int render_frame_id,
                        ServiceWorkerContextWrapper* service_worker_context);
 
+  void AddPushMessagingReceiver(
+      mojo::PendingReceiver<blink::mojom::PushMessaging> receiver);
+
+  // Temporary method while RenderProcessHostImpl does not migrate from using
+  // service_manager::BinderRegistry to using service_manager::BinderMap.
   void BindRequest(blink::mojom::PushMessagingRequest request);
 
   base::WeakPtr<PushMessagingManager> AsWeakPtr() {
@@ -145,7 +150,7 @@ class PushMessagingManager : public blink::mojom::PushMessaging {
   GURL default_endpoint_;
   GURL web_push_protocol_endpoint_;
 
-  mojo::BindingSet<blink::mojom::PushMessaging> bindings_;
+  mojo::ReceiverSet<blink::mojom::PushMessaging> receivers_;
 
   base::WeakPtrFactory<PushMessagingManager> weak_factory_;
 

content/renderer/render_frame_impl_browsertest.cc

@@ -859,7 +859,8 @@ class TestSimpleDocumentInterfaceBrokerImpl
   void GetCredentialManager(
       blink::mojom::CredentialManagerRequest request) override {}
   void GetAuthenticator(blink::mojom::AuthenticatorRequest request) override {}
-  void GetPushMessaging(blink::mojom::PushMessagingRequest request) override {}
+  void GetPushMessaging(
+      mojo::PendingReceiver<blink::mojom::PushMessaging> receiver) override {}
   void GetVirtualAuthenticatorManager(
       blink::test::mojom::VirtualAuthenticatorManagerRequest request) override {
   }

third_party/blink/public/mojom/frame/document_interface_broker.mojom

@@ -20,7 +20,7 @@ interface DocumentInterfaceBroker {
   GetAuthenticator(blink.mojom.Authenticator& request);
   GetCredentialManager(blink.mojom.CredentialManager& request);
   GetFrameHostTestInterface(blink.mojom.FrameHostTestInterface& request);
-  GetPushMessaging(blink.mojom.PushMessaging& request);
+  GetPushMessaging(pending_receiver<blink.mojom.PushMessaging> request);
   GetVirtualAuthenticatorManager(blink.test.mojom.VirtualAuthenticatorManager& request);
   RegisterAppCacheHost(blink.mojom.AppCacheHost& host_request,
                        AppCacheFrontend frontend,

third_party/blink/public/platform/interface_provider.h

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_PUBLIC_PLATFORM_INTERFACE_PROVIDER_H_
 
 #include "mojo/public/cpp/bindings/interface_request.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/system/message_pipe.h"
 #include "third_party/blink/public/platform/web_common.h"
 
@@ -24,6 +25,11 @@ class BLINK_PLATFORM_EXPORT InterfaceProvider {
     GetInterface(Interface::Name_, ptr.PassMessagePipe());
   }
 
+  template <typename Interface>
+  void GetInterface(mojo::PendingReceiver<Interface> receiver) {
+    GetInterface(Interface::Name_, receiver.PassPipe());
+  }
+
   static InterfaceProvider* GetEmptyInterfaceProvider();
 };
 

third_party/blink/renderer/modules/push_messaging/push_messaging_client.cc

@@ -41,14 +41,11 @@ PushMessagingClient* PushMessagingClient::From(LocalFrame* frame) {
   return Supplement<LocalFrame>::From<PushMessagingClient>(frame);
 }
 
-mojom::blink::PushMessaging* PushMessagingClient::GetService() {
+mojom::blink::PushMessaging* PushMessagingClient::GetPushMessagingRemote() {
   if (!push_messaging_manager_) {
-    auto request = mojo::MakeRequest(
-        &push_messaging_manager_,
-        GetSupplementable()->GetTaskRunner(TaskType::kMiscPlatformAPI));
-
     GetSupplementable()->GetDocumentInterfaceBroker().GetPushMessaging(
-        std::move(request));
+        push_messaging_manager_.BindNewPipeAndPassReceiver(
+            GetSupplementable()->GetTaskRunner(TaskType::kMiscPlatformAPI)));
   }
 
   return push_messaging_manager_.get();
@@ -122,7 +119,7 @@ void PushMessagingClient::DoSubscribe(
     return;
   }
 
-  GetService()->Subscribe(
+  GetPushMessagingRemote()->Subscribe(
       service_worker_registration->RegistrationId(), std::move(options),
       user_gesture,
       WTF::Bind(&PushMessagingClient::DidSubscribe, WrapPersistent(this),

third_party/blink/renderer/modules/push_messaging/push_messaging_client.h

@@ -9,6 +9,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/mojom/manifest/manifest.mojom-blink.h"
 #include "third_party/blink/public/mojom/push_messaging/push_messaging.mojom-blink.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
@@ -46,7 +47,7 @@ class PushMessagingClient final
  private:
   // Returns an initialized PushMessaging service. A connection will be
   // established after the first call to this method.
-  mojom::blink::PushMessaging* GetService();
+  mojom::blink::PushMessaging* GetPushMessagingRemote();
 
   void DidGetManifest(ServiceWorkerRegistration* service_worker_registration,
                       mojom::blink::PushSubscriptionOptionsPtr options,
@@ -68,7 +69,7 @@ class PushMessagingClient final
                     const base::Optional<WTF::Vector<uint8_t>>& p256dh,
                     const base::Optional<WTF::Vector<uint8_t>>& auth);
 
-  mojom::blink::PushMessagingPtr push_messaging_manager_;
+  mojo::Remote<mojom::blink::PushMessaging> push_messaging_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(PushMessagingClient);
 };

third_party/blink/renderer/modules/push_messaging/push_provider.cc

@@ -23,9 +23,7 @@ namespace blink {
 const char PushProvider::kSupplementName[] = "PushProvider";
 
 PushProvider::PushProvider(ServiceWorkerRegistration& registration)
-    : Supplement<ServiceWorkerRegistration>(registration) {
-  GetInterface(mojo::MakeRequest(&push_messaging_manager_));
-}
+    : Supplement<ServiceWorkerRegistration>(registration) {}
 
 // static
 PushProvider* PushProvider::From(ServiceWorkerRegistration* registration) {
@@ -43,8 +41,13 @@ PushProvider* PushProvider::From(ServiceWorkerRegistration* registration) {
 }
 
 // static
-void PushProvider::GetInterface(mojom::blink::PushMessagingRequest request) {
-  Platform::Current()->GetInterfaceProvider()->GetInterface(std::move(request));
+mojom::blink::PushMessaging* PushProvider::GetPushMessagingRemote() {
+  if (!push_messaging_manager_) {
+    Platform::Current()->GetInterfaceProvider()->GetInterface(
+        push_messaging_manager_.BindNewPipeAndPassReceiver());
+  }
+
+  return push_messaging_manager_.get();
 }
 
 void PushProvider::Subscribe(
@@ -56,7 +59,7 @@ void PushProvider::Subscribe(
   mojom::blink::PushSubscriptionOptionsPtr content_options_ptr =
       mojom::blink::PushSubscriptionOptions::From(options);
 
-  push_messaging_manager_->Subscribe(
+  GetPushMessagingRemote()->Subscribe(
       GetSupplementable()->RegistrationId(), std::move(content_options_ptr),
       user_gesture,
       WTF::Bind(&PushProvider::DidSubscribe, WrapPersistent(this),
@@ -97,7 +100,7 @@ void PushProvider::Unsubscribe(
     std::unique_ptr<PushUnsubscribeCallbacks> callbacks) {
   DCHECK(callbacks);
 
-  push_messaging_manager_->Unsubscribe(
+  GetPushMessagingRemote()->Unsubscribe(
       GetSupplementable()->RegistrationId(),
       WTF::Bind(&PushProvider::DidUnsubscribe, WrapPersistent(this),
                 WTF::Passed(std::move(callbacks))));
@@ -122,7 +125,7 @@ void PushProvider::GetSubscription(
     std::unique_ptr<PushSubscriptionCallbacks> callbacks) {
   DCHECK(callbacks);
 
-  push_messaging_manager_->GetSubscription(
+  GetPushMessagingRemote()->GetSubscription(
       GetSupplementable()->RegistrationId(),
       WTF::Bind(&PushProvider::DidGetSubscription, WrapPersistent(this),
                 WTF::Passed(std::move(callbacks))));

third_party/blink/renderer/modules/push_messaging/push_provider.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/single_thread_task_runner.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/mojom/push_messaging/push_messaging.mojom-blink.h"
 #include "third_party/blink/public/mojom/push_messaging/push_messaging_status.mojom-blink.h"
 #include "third_party/blink/renderer/modules/push_messaging/push_subscription_callbacks.h"
@@ -47,7 +48,9 @@ class PushProvider final : public GarbageCollectedFinalized<PushProvider>,
   void GetSubscription(std::unique_ptr<PushSubscriptionCallbacks> callbacks);
 
  private:
-  static void GetInterface(mojom::blink::PushMessagingRequest request);
+  // Returns an initialized PushMessaging service. A connection will be
+  // established after the first call to this method.
+  mojom::blink::PushMessaging* GetPushMessagingRemote();
 
   void DidSubscribe(std::unique_ptr<PushSubscriptionCallbacks> callbacks,
                     mojom::blink::PushRegistrationStatus status,
@@ -68,7 +71,7 @@ class PushProvider final : public GarbageCollectedFinalized<PushProvider>,
                           const base::Optional<WTF::Vector<uint8_t>>& p256dh,
                           const base::Optional<WTF::Vector<uint8_t>>& auth);
 
-  mojom::blink::PushMessagingPtr push_messaging_manager_;
+  mojo::Remote<mojom::blink::PushMessaging> push_messaging_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(PushProvider);
 };