Convert ServiceWorkerStreamCallback to new Mojo types

This CL converts ServiceWorkerStreamCallback{Ptr, Request} in
content and blink to the new Mojo types, and uses
pending_receiver<ServiceWorkerStreamCallback> in
service_worker_stream_handle.mojom.

Bug: 955171, 978694
Change-Id: I0eb07b02a08965124864022c2e1a9403efa9b0d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1767470
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690298}

content/browser/service_worker/service_worker_navigation_loader.cc

@@ -24,6 +24,8 @@
 #include "content/common/service_worker/service_worker_loader_helpers.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/browser_thread.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/mojom/fetch_api.mojom.h"
 
@@ -50,12 +52,11 @@ std::string ComposeFetchEventResultString(
 class ServiceWorkerNavigationLoader::StreamWaiter
     : public blink::mojom::ServiceWorkerStreamCallback {
  public:
-  StreamWaiter(
-      ServiceWorkerNavigationLoader* owner,
-      blink::mojom::ServiceWorkerStreamCallbackRequest callback_request)
-      : owner_(owner),
-        binding_(this, std::move(callback_request)) {
-    binding_.set_connection_error_handler(
+  StreamWaiter(ServiceWorkerNavigationLoader* owner,
+               mojo::PendingReceiver<blink::mojom::ServiceWorkerStreamCallback>
+                   callback_receiver)
+      : owner_(owner), receiver_(this, std::move(callback_receiver)) {
+    receiver_.set_disconnect_handler(
         base::BindOnce(&StreamWaiter::OnAborted, base::Unretained(this)));
   }
 
@@ -71,7 +72,7 @@ class ServiceWorkerNavigationLoader::StreamWaiter
 
  private:
   ServiceWorkerNavigationLoader* owner_;
-  mojo::Binding<blink::mojom::ServiceWorkerStreamCallback> binding_;
+  mojo::Receiver<blink::mojom::ServiceWorkerStreamCallback> receiver_;
 
   DISALLOW_COPY_AND_ASSIGN(StreamWaiter);
 };
@@ -377,7 +378,7 @@ void ServiceWorkerNavigationLoader::StartResponse(
                            TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT,
                            "result", "stream response");
     stream_waiter_ = std::make_unique<StreamWaiter>(
-        this, std::move(body_as_stream->callback_request));
+        this, std::move(body_as_stream->callback_receiver));
     CommitResponseBody(std::move(body_as_stream->stream));
     // StreamWaiter will call CommitCompleted() when done.
     return;

content/browser/service_worker/service_worker_navigation_loader.h

@@ -16,7 +16,7 @@
 #include "content/browser/service_worker/embedded_worker_status.h"
 #include "content/browser/service_worker/service_worker_fetch_dispatcher.h"
 #include "content/browser/url_loader_factory_getter.h"
-#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/system/data_pipe.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
 #include "third_party/blink/public/common/service_worker/service_worker_status_code.h"

content/browser/service_worker/service_worker_navigation_loader_unittest.cc

@@ -24,6 +24,7 @@
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/mock_render_process_host.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/system/data_pipe_utils.h"
@@ -102,11 +103,12 @@ class FetchEventServiceWorker : public FakeServiceWorker {
 
   // Tells this worker to respond to fetch events with the specified stream.
   void RespondWithStream(
-      blink::mojom::ServiceWorkerStreamCallbackRequest callback_request,
+      mojo::PendingReceiver<blink::mojom::ServiceWorkerStreamCallback>
+          callback_receiver,
       mojo::ScopedDataPipeConsumerHandle consumer_handle) {
     response_mode_ = ResponseMode::kStream;
     stream_handle_ = blink::mojom::ServiceWorkerStreamHandle::New();
-    stream_handle_->callback_request = std::move(callback_request);
+    stream_handle_->callback_receiver = std::move(callback_receiver);
     stream_handle_->stream = std::move(consumer_handle);
   }
 
@@ -640,10 +642,11 @@ TEST_F(ServiceWorkerNavigationLoaderTest, StreamResponse) {
 
   // Construct the Stream to respond with.
   const char kResponseBody[] = "Here is sample text for the Stream.";
-  blink::mojom::ServiceWorkerStreamCallbackPtr stream_callback;
+  mojo::Remote<blink::mojom::ServiceWorkerStreamCallback> stream_callback;
   mojo::DataPipe data_pipe;
-  service_worker_->RespondWithStream(mojo::MakeRequest(&stream_callback),
-                                     std::move(data_pipe.consumer_handle));
+  service_worker_->RespondWithStream(
+      stream_callback.BindNewPipeAndPassReceiver(),
+      std::move(data_pipe.consumer_handle));
 
   // Perform the request.
   StartRequest(CreateRequest());
@@ -687,10 +690,11 @@ TEST_F(ServiceWorkerNavigationLoaderTest, StreamResponse_Abort) {
 
   // Construct the Stream to respond with.
   const char kResponseBody[] = "Here is sample text for the Stream.";
-  blink::mojom::ServiceWorkerStreamCallbackPtr stream_callback;
+  mojo::Remote<blink::mojom::ServiceWorkerStreamCallback> stream_callback;
   mojo::DataPipe data_pipe;
-  service_worker_->RespondWithStream(mojo::MakeRequest(&stream_callback),
-                                     std::move(data_pipe.consumer_handle));
+  service_worker_->RespondWithStream(
+      stream_callback.BindNewPipeAndPassReceiver(),
+      std::move(data_pipe.consumer_handle));
 
   // Perform the request.
   StartRequest(CreateRequest());
@@ -736,10 +740,11 @@ TEST_F(ServiceWorkerNavigationLoaderTest, StreamResponseAndCancel) {
 
   // Construct the Stream to respond with.
   const char kResponseBody[] = "Here is sample text for the Stream.";
-  blink::mojom::ServiceWorkerStreamCallbackPtr stream_callback;
+  mojo::Remote<blink::mojom::ServiceWorkerStreamCallback> stream_callback;
   mojo::DataPipe data_pipe;
-  service_worker_->RespondWithStream(mojo::MakeRequest(&stream_callback),
-                                     std::move(data_pipe.consumer_handle));
+  service_worker_->RespondWithStream(
+      stream_callback.BindNewPipeAndPassReceiver(),
+      std::move(data_pipe.consumer_handle));
 
   // Perform the request.
   StartRequest(CreateRequest());

content/renderer/service_worker/service_worker_subresource_loader.cc

@@ -22,7 +22,9 @@
 #include "content/renderer/renderer_blink_platform_impl.h"
 #include "content/renderer/service_worker/controller_service_worker_connector.h"
 #include "mojo/public/cpp/bindings/binding.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "net/base/net_errors.h"
 #include "net/url_request/redirect_util.h"
@@ -129,11 +131,12 @@ class HeaderRewritingURLLoaderClient : public network::mojom::URLLoaderClient {
 class ServiceWorkerSubresourceLoader::StreamWaiter
     : public blink::mojom::ServiceWorkerStreamCallback {
  public:
-  StreamWaiter(ServiceWorkerSubresourceLoader* owner,
-               blink::mojom::ServiceWorkerStreamCallbackRequest request)
-      : owner_(owner), binding_(this, std::move(request)) {
+  StreamWaiter(
+      ServiceWorkerSubresourceLoader* owner,
+      mojo::PendingReceiver<blink::mojom::ServiceWorkerStreamCallback> receiver)
+      : owner_(owner), receiver_(this, std::move(receiver)) {
     DCHECK(owner_);
-    binding_.set_connection_error_handler(
+    receiver_.set_disconnect_handler(
         base::BindOnce(&StreamWaiter::OnAborted, base::Unretained(this)));
   }
 
@@ -143,7 +146,7 @@ class ServiceWorkerSubresourceLoader::StreamWaiter
 
  private:
   ServiceWorkerSubresourceLoader* owner_;
-  mojo::Binding<blink::mojom::ServiceWorkerStreamCallback> binding_;
+  mojo::Receiver<blink::mojom::ServiceWorkerStreamCallback> receiver_;
 
   DISALLOW_COPY_AND_ASSIGN(StreamWaiter);
 };
@@ -483,7 +486,7 @@ void ServiceWorkerSubresourceLoader::StartResponse(
     DCHECK(!response->blob);
     DCHECK(url_loader_client_.is_bound());
     stream_waiter_ = std::make_unique<StreamWaiter>(
-        this, std::move(body_as_stream->callback_request));
+        this, std::move(body_as_stream->callback_receiver));
     CommitResponseBody(std::move(body_as_stream->stream));
     return;
   }

content/renderer/service_worker/service_worker_subresource_loader_unittest.cc

@@ -24,6 +24,7 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "mojo/public/cpp/system/data_pipe_utils.h"
 #include "net/http/http_util.h"
@@ -141,11 +142,12 @@ class FakeControllerServiceWorker
 
   // Tells this controller to respond to fetch events with the specified stream.
   void RespondWithStream(
-      blink::mojom::ServiceWorkerStreamCallbackRequest callback_request,
+      mojo::PendingReceiver<blink::mojom::ServiceWorkerStreamCallback>
+          callback_receiver,
       mojo::ScopedDataPipeConsumerHandle consumer_handle) {
     response_mode_ = ResponseMode::kStream;
     stream_handle_ = blink::mojom::ServiceWorkerStreamHandle::New();
-    stream_handle_->callback_request = std::move(callback_request);
+    stream_handle_->callback_receiver = std::move(callback_receiver);
     stream_handle_->stream = std::move(consumer_handle);
   }
 
@@ -864,10 +866,11 @@ TEST_F(ServiceWorkerSubresourceLoaderTest, StreamResponse) {
 
   // Construct the Stream to respond with.
   const char kResponseBody[] = "Here is sample text for the Stream.";
-  blink::mojom::ServiceWorkerStreamCallbackPtr stream_callback;
+  mojo::Remote<blink::mojom::ServiceWorkerStreamCallback> stream_callback;
   mojo::DataPipe data_pipe;
-  fake_controller_.RespondWithStream(mojo::MakeRequest(&stream_callback),
-                                     std::move(data_pipe.consumer_handle));
+  fake_controller_.RespondWithStream(
+      stream_callback.BindNewPipeAndPassReceiver(),
+      std::move(data_pipe.consumer_handle));
   fake_controller_.SetResponseSource(
       network::mojom::FetchResponseSource::kNetwork);
 
@@ -924,10 +927,11 @@ TEST_F(ServiceWorkerSubresourceLoaderTest, StreamResponse_Abort) {
 
   // Construct the Stream to respond with.
   const char kResponseBody[] = "Here is sample text for the Stream.";
-  blink::mojom::ServiceWorkerStreamCallbackPtr stream_callback;
+  mojo::Remote<blink::mojom::ServiceWorkerStreamCallback> stream_callback;
   mojo::DataPipe data_pipe;
-  fake_controller_.RespondWithStream(mojo::MakeRequest(&stream_callback),
-                                     std::move(data_pipe.consumer_handle));
+  fake_controller_.RespondWithStream(
+      stream_callback.BindNewPipeAndPassReceiver(),
+      std::move(data_pipe.consumer_handle));
 
   network::mojom::URLLoaderFactoryPtr factory =
       CreateSubresourceLoaderFactory();
@@ -1228,10 +1232,11 @@ TEST_F(ServiceWorkerSubresourceLoaderTest, RedirectResponse) {
 
   // Give the final response.
   const char kResponseBody[] = "Here is sample text for the Stream.";
-  blink::mojom::ServiceWorkerStreamCallbackPtr stream_callback;
+  mojo::Remote<blink::mojom::ServiceWorkerStreamCallback> stream_callback;
   mojo::DataPipe data_pipe;
-  fake_controller_.RespondWithStream(mojo::MakeRequest(&stream_callback),
-                                     std::move(data_pipe.consumer_handle));
+  fake_controller_.RespondWithStream(
+      stream_callback.BindNewPipeAndPassReceiver(),
+      std::move(data_pipe.consumer_handle));
   loader->FollowRedirect({}, {}, base::nullopt);
   client->RunUntilResponseReceived();
 

third_party/blink/public/mojom/service_worker/service_worker_stream_handle.mojom

@@ -13,5 +13,5 @@ interface ServiceWorkerStreamCallback {
 
 struct ServiceWorkerStreamHandle {
   handle<data_pipe_consumer> stream;
-  ServiceWorkerStreamCallback& callback_request;
+  pending_receiver<ServiceWorkerStreamCallback> callback_receiver;
 };

third_party/blink/renderer/modules/service_worker/fetch_respond_with_observer.cc

@@ -10,6 +10,8 @@
 #include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "services/network/public/mojom/fetch_api.mojom-blink.h"
 #include "third_party/blink/public/mojom/devtools/console_message.mojom-blink.h"
 #include "third_party/blink/public/mojom/loader/request_context_frame_type.mojom-blink.h"
@@ -120,7 +122,7 @@ bool IsClientRequest(network::mojom::RequestContextFrameType frame_type,
          request_context == mojom::RequestContextType::WORKER;
 }
 
-// Notifies the result of FetchDataLoader to |callback_ptr_|, the other endpoint
+// Notifies the result of FetchDataLoader to |callback_|, the other endpoint
 // for which is passed to the browser process via
 // blink.mojom.ServiceWorkerFetchResponseCallback.OnResponseStream().
 class FetchLoaderClient final
@@ -132,10 +134,10 @@ class FetchLoaderClient final
   FetchLoaderClient(
       std::unique_ptr<ServiceWorkerTimeoutTimer::StayAwakeToken> token)
       : token_(std::move(token)) {
-    // We need to make |callback_ptr_| callable in the first place because some
+    // We need to make |callback_| callable in the first place because some
     // DidFetchDataLoadXXX() accessing it may be called synchronously from
     // StartLoading().
-    callback_request_ = mojo::MakeRequest(&callback_ptr_);
+    callback_receiver_ = callback_.BindNewPipeAndPassReceiver();
   }
 
   void DidFetchDataStartedDataPipe(
@@ -145,18 +147,18 @@ class FetchLoaderClient final
     body_stream_ = std::move(pipe);
   }
   void DidFetchDataLoadedDataPipe() override {
-    callback_ptr_->OnCompleted();
+    callback_->OnCompleted();
     token_.reset();
   }
   void DidFetchDataLoadFailed() override {
-    callback_ptr_->OnAborted();
+    callback_->OnAborted();
     token_.reset();
   }
   void Abort() override {
     // A fetch() aborted via AbortSignal in the ServiceWorker will just look
     // like an ordinary failure to the page.
     // TODO(ricea): Should a fetch() on the page get an AbortError instead?
-    callback_ptr_->OnAborted();
+    callback_->OnAborted();
     token_.reset();
   }
 
@@ -164,7 +166,7 @@ class FetchLoaderClient final
     if (!body_stream_.is_valid())
       return nullptr;
     return mojom::blink::ServiceWorkerStreamHandle::New(
-        std::move(body_stream_), std::move(callback_request_));
+        std::move(body_stream_), std::move(callback_receiver_));
   }
 
   void Trace(blink::Visitor* visitor) override {
@@ -173,9 +175,10 @@ class FetchLoaderClient final
 
  private:
   mojo::ScopedDataPipeConsumerHandle body_stream_;
-  mojom::blink::ServiceWorkerStreamCallbackRequest callback_request_;
+  mojo::PendingReceiver<mojom::blink::ServiceWorkerStreamCallback>
+      callback_receiver_;
 
-  mojom::blink::ServiceWorkerStreamCallbackPtr callback_ptr_;
+  mojo::Remote<mojom::blink::ServiceWorkerStreamCallback> callback_;
   std::unique_ptr<ServiceWorkerTimeoutTimer::StayAwakeToken> token_;
 
   DISALLOW_COPY_AND_ASSIGN(FetchLoaderClient);