Use blink::TransferableMessage in window and service worker postMessage.

The main thing this achieves is making sure that blobs that are sent in
messages are kept alive. Without this, especially when sending a blob to
a service worker that first has to start up, it would very well be possible
for the sending document to deref a blob before the receiving worker has a
chance to ref the blob.

Bug: 351753
Change-Id: I859a4a069ad6417a808c5711003ac72d580b431e
Reviewed-on: https://chromium-review.googlesource.com/887222
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534441}

content/browser/message_port_provider.cc

@@ -29,11 +29,14 @@ void PostMessageToFrameInternal(WebContents* web_contents,
 
   FrameMsg_PostMessage_Params params;
   params.is_data_raw_string = true;
-  params.data = data;
+  params.message = new base::RefCountedData<blink::TransferableMessage>();
+  params.message->data.encoded_message =
+      base::make_span(reinterpret_cast<const uint8_t*>(data.data()),
+                      data.size() * sizeof(base::char16));
+  params.message->data.ports = std::move(channels);
   params.source_routing_id = MSG_ROUTING_NONE;
   params.source_origin = source_origin;
   params.target_origin = target_origin;
-  params.message_ports = std::move(channels);
 
   RenderFrameHost* rfh = web_contents->GetMainFrame();
   rfh->Send(new FrameMsg_PostMessageEvent(rfh->GetRoutingID(), params));

content/browser/service_worker/service_worker_dispatcher_host.cc

@@ -228,9 +228,9 @@ ServiceWorkerDispatcherHost::AsWeakPtr() {
 void ServiceWorkerDispatcherHost::OnPostMessageToWorker(
     int handle_id,
     int provider_id,
-    const base::string16& message,
-    const url::Origin& source_origin,
-    const std::vector<MessagePortChannel>& sent_message_ports) {
+    const scoped_refptr<base::RefCountedData<blink::TransferableMessage>>&
+        message,
+    const url::Origin& source_origin) {
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerDispatcherHost::OnPostMessageToWorker");
   if (!GetContext())
@@ -250,17 +250,22 @@ void ServiceWorkerDispatcherHost::OnPostMessageToWorker(
     return;
   }
 
+  // When this method is called the encoded_message inside message could just
+  // point to the IPC message's buffer. But that buffer can become invalid
+  // before the message is passed on to the service worker, so make sure
+  // message owns its data.
+  message->data.EnsureDataIsOwned();
+
   DispatchExtendableMessageEvent(
-      base::WrapRefCounted(handle->version()), message, source_origin,
-      sent_message_ports, sender_provider_host,
+      base::WrapRefCounted(handle->version()), std::move(message->data),
+      source_origin, sender_provider_host,
       base::BindOnce(&ServiceWorkerUtils::NoOpStatusCallback));
 }
 
 void ServiceWorkerDispatcherHost::DispatchExtendableMessageEvent(
     scoped_refptr<ServiceWorkerVersion> worker,
-    const base::string16& message,
+    blink::TransferableMessage message,
     const url::Origin& source_origin,
-    const std::vector<MessagePortChannel>& sent_message_ports,
     ServiceWorkerProviderHost* sender_provider_host,
     StatusCallback callback) {
   switch (sender_provider_host->provider_type()) {
@@ -271,9 +276,8 @@ void ServiceWorkerDispatcherHost::DispatchExtendableMessageEvent(
           base::BindOnce(&ServiceWorkerDispatcherHost::
                              DispatchExtendableMessageEventInternal<
                                  blink::mojom::ServiceWorkerClientInfoPtr>,
-                         this, worker, message, source_origin,
-                         sent_message_ports, base::nullopt,
-                         std::move(callback)));
+                         this, worker, std::move(message), source_origin,
+                         base::nullopt, std::move(callback)));
       break;
     case blink::mojom::ServiceWorkerProviderType::kForServiceWorker: {
       // Clamp timeout to the sending worker's remaining timeout, to prevent
@@ -289,9 +293,9 @@ void ServiceWorkerDispatcherHost::DispatchExtendableMessageEvent(
           base::BindOnce(&ServiceWorkerDispatcherHost::
                              DispatchExtendableMessageEventInternal<
                                  blink::mojom::ServiceWorkerObjectInfoPtr>,
-                         this, worker, message, source_origin,
-                         sent_message_ports, base::make_optional(timeout),
-                         std::move(callback), std::move(worker_info)));
+                         this, worker, std::move(message), source_origin,
+                         base::make_optional(timeout), std::move(callback),
+                         std::move(worker_info)));
       break;
     }
     case blink::mojom::ServiceWorkerProviderType::kUnknown:
@@ -363,9 +367,8 @@ void ServiceWorkerDispatcherHost::OnProviderCreated(
 template <typename SourceInfoPtr>
 void ServiceWorkerDispatcherHost::DispatchExtendableMessageEventInternal(
     scoped_refptr<ServiceWorkerVersion> worker,
-    const base::string16& message,
+    blink::TransferableMessage message,
     const url::Origin& source_origin,
-    const std::vector<MessagePortChannel>& sent_message_ports,
     const base::Optional<base::TimeDelta>& timeout,
     StatusCallback callback,
     SourceInfoPtr source_info) {
@@ -387,17 +390,16 @@ void ServiceWorkerDispatcherHost::DispatchExtendableMessageEventInternal(
       base::BindOnce(
           &ServiceWorkerDispatcherHost::
               DispatchExtendableMessageEventAfterStartWorker<SourceInfoPtr>,
-          this, worker, message, source_origin, sent_message_ports,
+          this, worker, std::move(message), source_origin,
           std::move(source_info), timeout, std::move(callback)));
 }
 
 template <typename SourceInfoPtr>
 void ServiceWorkerDispatcherHost::
     DispatchExtendableMessageEventAfterStartWorker(
         scoped_refptr<ServiceWorkerVersion> worker,
-        const base::string16& message,
+        blink::TransferableMessage message,
         const url::Origin& source_origin,
-        const std::vector<MessagePortChannel>& sent_message_ports,
         SourceInfoPtr source_info,
         const base::Optional<base::TimeDelta>& timeout,
         StatusCallback callback,
@@ -419,9 +421,8 @@ void ServiceWorkerDispatcherHost::
   }
 
   mojom::ExtendableMessageEventPtr event = mojom::ExtendableMessageEvent::New();
-  event->message = message;
+  event->message = std::move(message);
   event->source_origin = source_origin;
-  event->message_ports = MessagePortChannel::ReleaseHandles(sent_message_ports);
   SetMessageEventSource(&event, std::move(source_info));
 
   worker->event_dispatcher()->DispatchExtendableMessageEvent(

content/browser/service_worker/service_worker_dispatcher_host.h

@@ -24,10 +24,6 @@
 #include "mojo/public/cpp/bindings/strong_associated_binding_set.h"
 #include "third_party/WebKit/common/service_worker/service_worker_registration.mojom.h"
 
-namespace blink {
-class MessagePortChannel;
-}
-
 namespace url {
 class Origin;
 }  // namespace url
@@ -144,34 +140,31 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost
   void OnPostMessageToWorker(
       int handle_id,
       int provider_id,
-      const base::string16& message,
-      const url::Origin& source_origin,
-      const std::vector<blink::MessagePortChannel>& sent_message_ports);
+      const scoped_refptr<base::RefCountedData<blink::TransferableMessage>>&
+          message,
+      const url::Origin& source_origin);
 
   void OnTerminateWorker(int handle_id);
 
   void DispatchExtendableMessageEvent(
       scoped_refptr<ServiceWorkerVersion> worker,
-      const base::string16& message,
+      blink::TransferableMessage message,
       const url::Origin& source_origin,
-      const std::vector<blink::MessagePortChannel>& sent_message_ports,
       ServiceWorkerProviderHost* sender_provider_host,
       StatusCallback callback);
   template <typename SourceInfoPtr>
   void DispatchExtendableMessageEventInternal(
       scoped_refptr<ServiceWorkerVersion> worker,
-      const base::string16& message,
+      blink::TransferableMessage message,
       const url::Origin& source_origin,
-      const std::vector<blink::MessagePortChannel>& sent_message_ports,
       const base::Optional<base::TimeDelta>& timeout,
       StatusCallback callback,
       SourceInfoPtr source_info);
   template <typename SourceInfoPtr>
   void DispatchExtendableMessageEventAfterStartWorker(
       scoped_refptr<ServiceWorkerVersion> worker,
-      const base::string16& message,
+      blink::TransferableMessage message,
       const url::Origin& source_origin,
-      const std::vector<blink::MessagePortChannel>& sent_message_ports,
       SourceInfoPtr source_info,
       const base::Optional<base::TimeDelta>& timeout,
       StatusCallback callback,

content/browser/service_worker/service_worker_dispatcher_host_unittest.cc

@@ -230,13 +230,12 @@ class ServiceWorkerDispatcherHostTest : public testing::Test {
 
   void DispatchExtendableMessageEvent(
       scoped_refptr<ServiceWorkerVersion> worker,
-      const base::string16& message,
+      blink::TransferableMessage message,
       const url::Origin& source_origin,
-      const std::vector<MessagePortChannel>& sent_message_ports,
       ServiceWorkerProviderHost* sender_provider_host,
       ServiceWorkerDispatcherHost::StatusCallback callback) {
     dispatcher_host_->DispatchExtendableMessageEvent(
-        std::move(worker), message, source_origin, sent_message_ports,
+        std::move(worker), std::move(message), source_origin,
         sender_provider_host, std::move(callback));
   }
 
@@ -423,13 +422,13 @@ TEST_F(ServiceWorkerDispatcherHostTest, DispatchExtendableMessageEvent) {
   EXPECT_EQ(base::TimeDelta::FromSeconds(6), remaining_time);
 
   // Dispatch ExtendableMessageEvent.
-  std::vector<MessagePortChannel> ports;
-  SetUpDummyMessagePort(&ports);
+  blink::TransferableMessage message;
+  SetUpDummyMessagePort(&message.ports);
   called = false;
   status = SERVICE_WORKER_ERROR_MAX_VALUE;
   DispatchExtendableMessageEvent(
-      version_, base::string16(),
-      url::Origin::Create(version_->scope().GetOrigin()), ports, provider_host_,
+      version_, std::move(message),
+      url::Origin::Create(version_->scope().GetOrigin()), provider_host_,
       base::BindOnce(&SaveStatusCallback, &called, &status));
   // The 2nd reference to |sender_worker_handle| has been passed via the above
   // dispatch of ExtendableMessageEvent.
@@ -458,13 +457,13 @@ TEST_F(ServiceWorkerDispatcherHostTest, DispatchExtendableMessageEvent_Fail) {
 
   // Try to dispatch ExtendableMessageEvent. This should fail to start the
   // worker and to dispatch the event.
-  std::vector<MessagePortChannel> ports;
-  SetUpDummyMessagePort(&ports);
+  blink::TransferableMessage message;
+  SetUpDummyMessagePort(&message.ports);
   bool called = false;
   ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE;
   DispatchExtendableMessageEvent(
-      version_, base::string16(),
-      url::Origin::Create(version_->scope().GetOrigin()), ports, provider_host_,
+      version_, std::move(message),
+      url::Origin::Create(version_->scope().GetOrigin()), provider_host_,
       base::BindOnce(&SaveStatusCallback, &called, &status));
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(called);

content/browser/service_worker/service_worker_provider_host.cc

@@ -545,16 +545,13 @@ bool ServiceWorkerProviderHost::CanAssociateRegistration(
 
 void ServiceWorkerProviderHost::PostMessageToClient(
     ServiceWorkerVersion* version,
-    const base::string16& message,
-    const std::vector<blink::MessagePortChannel>& sent_message_ports) {
+    blink::TransferableMessage message) {
   DCHECK(IsProviderForClient());
   if (!dispatcher_host_)
     return;
 
-  auto message_pipes =
-      blink::MessagePortChannel::ReleaseHandles(sent_message_ports);
   container_->PostMessageToClient(GetOrCreateServiceWorkerHandle(version),
-                                  message, std::move(message_pipes));
+                                  std::move(message));
 }
 
 void ServiceWorkerProviderHost::CountFeature(uint32_t feature) {

content/browser/service_worker/service_worker_provider_host.h

@@ -36,10 +36,6 @@
 #include "third_party/WebKit/common/service_worker/service_worker_provider_type.mojom.h"
 #include "third_party/WebKit/common/service_worker/service_worker_registration.mojom.h"
 
-namespace blink {
-class MessagePortChannel;
-}
-
 namespace network {
 class ResourceRequestBody;
 }
@@ -296,10 +292,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   bool IsContextAlive();
 
   // Dispatches message event to the document.
-  void PostMessageToClient(
-      ServiceWorkerVersion* version,
-      const base::string16& message,
-      const std::vector<blink::MessagePortChannel>& sent_message_ports);
+  void PostMessageToClient(ServiceWorkerVersion* version,
+                           blink::TransferableMessage message);
 
   // Notifies the client that its controller used a feature, for UseCounter
   // purposes. This can only be called if IsProviderForClient() is true.

content/browser/service_worker/service_worker_provider_host_unittest.cc

@@ -384,10 +384,8 @@ class MockServiceWorkerContainer : public mojom::ServiceWorkerContainer {
                      bool should_notify_controllerchange) override {
     was_set_controller_called_ = true;
   }
-  void PostMessageToClient(
-      blink::mojom::ServiceWorkerObjectInfoPtr controller,
-      const base::string16& message,
-      std::vector<mojo::ScopedMessagePipeHandle> message_pipes) override {}
+  void PostMessageToClient(blink::mojom::ServiceWorkerObjectInfoPtr controller,
+                           blink::TransferableMessage message) override {}
   void CountFeature(blink::mojom::WebFeature feature) override {}
 
   bool was_set_controller_called() const { return was_set_controller_called_; }

content/browser/service_worker/service_worker_version.cc

@@ -1295,8 +1295,8 @@ bool ServiceWorkerVersion::IsInstalled(ServiceWorkerVersion::Status status) {
 
 void ServiceWorkerVersion::OnPostMessageToClient(
     const std::string& client_uuid,
-    const base::string16& message,
-    const std::vector<blink::MessagePortChannel>& sent_message_ports) {
+    const scoped_refptr<base::RefCountedData<blink::TransferableMessage>>&
+        message) {
   if (!context_)
     return;
   TRACE_EVENT1("ServiceWorker",
@@ -1313,7 +1313,7 @@ void ServiceWorkerVersion::OnPostMessageToClient(
     // possibly due to timing issue or bad message.
     return;
   }
-  provider_host->PostMessageToClient(this, message, sent_message_ports);
+  provider_host->PostMessageToClient(this, std::move(message->data));
 }
 
 void ServiceWorkerVersion::OnFocusClient(int request_id,

content/browser/service_worker/service_worker_version.h

@@ -49,10 +49,6 @@
 #include "url/gurl.h"
 #include "url/origin.h"
 
-namespace blink {
-class MessagePortChannel;
-}
-
 namespace net {
 class HttpResponseInfo;
 }
@@ -650,8 +646,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
 
   void OnPostMessageToClient(
       const std::string& client_uuid,
-      const base::string16& message,
-      const std::vector<blink::MessagePortChannel>& sent_message_ports);
+      const scoped_refptr<base::RefCountedData<blink::TransferableMessage>>&
+          message);
   void OnFocusClient(int request_id, const std::string& client_uuid);
   void OnNavigateClient(int request_id,
                         const std::string& client_uuid,

content/common/BUILD.gn

@@ -604,7 +604,7 @@ mojom("mojo_bindings") {
     "//services/video_capture/public/interfaces",
     "//services/viz/public/interfaces",
     "//skia/public/interfaces",
-    "//third_party/WebKit/common:mojo_platform_bindings",
+    "//third_party/WebKit/common:mojo_bindings",
     "//third_party/WebKit/public:mojo_bindings",
     "//ui/base/mojo:mojo_bindings",
     "//ui/gfx/geometry/mojo",
@@ -613,7 +613,7 @@ mojom("mojo_bindings") {
     "//url/mojom:url_mojom_origin",
   ]
 
-  overridden_deps = [ "//third_party/WebKit/common:mojo_platform_bindings" ]
+  overridden_deps = [ "//third_party/WebKit/common:mojo_bindings" ]
   component_deps = [ "//third_party/WebKit/common:blink_common" ]
 
   component_output_prefix = "content_common_mojo_bindings"