[Blobs] Add AsDataPipeGetter method to Blob mojo interface.

This makes it simpler to create a DataPipeGetter out of a Blob, and also
ensures that a blob that is used as the body of a request can still be
read even after the process that created the DataPipeGetter (i.e. the
renderer that initiated the fetch) dies.

Bug: 821878, 761117
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I519a6061900f4cb29b786b8445dc14f034959e27
Reviewed-on: https://chromium-review.googlesource.com/963088
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543807}

content/renderer/loader/web_url_request_util.cc

@@ -105,94 +105,6 @@ class HeaderFlattener : public blink::WebHTTPHeaderVisitor {
   std::string buffer_;
 };
 
-// Vends data pipes to read a Blob. It stays alive until all Mojo connections
-// close.
-class DataPipeGetter : public network::mojom::DataPipeGetter {
- public:
-  DataPipeGetter(blink::mojom::BlobPtr blob,
-                 network::mojom::DataPipeGetterRequest request) {
-    // If a sync XHR is doing the upload, then the main thread will be blocked.
-    // So we must bind on a background thread, otherwise the methods below will
-    // never be called and the process will hang.
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-        base::CreateSingleThreadTaskRunnerWithTraits(
-            {base::TaskPriority::USER_VISIBLE,
-             base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
-    task_runner->PostTask(
-        FROM_HERE,
-        base::BindOnce(&DataPipeGetter::BindInternal, base::Unretained(this),
-                       blob.PassInterface(), std::move(request)));
-  }
-  ~DataPipeGetter() override = default;
-
- private:
-  class BlobReaderClient : public blink::mojom::BlobReaderClient {
-   public:
-    explicit BlobReaderClient(ReadCallback callback)
-        : callback_(std::move(callback)) {
-      DCHECK(!callback_.is_null());
-    }
-    ~BlobReaderClient() override = default;
-
-    // blink::mojom::BlobReaderClient implementation:
-    void OnCalculatedSize(uint64_t total_size,
-                          uint64_t expected_content_size) override {
-      // Check if null since it's conceivable OnComplete() was already called
-      // with error.
-      if (!callback_.is_null())
-        std::move(callback_).Run(net::OK, total_size);
-    }
-    void OnComplete(int32_t status, uint64_t data_length) override {
-      // Check if null since OnCalculatedSize() may have already been called
-      // and an error occurred later.
-      if (!callback_.is_null() && status != net::OK) {
-        // On error, signal failure immediately. On success, OnCalculatedSize()
-        // is guaranteed to be called, and the result will be signaled from
-        // there.
-        std::move(callback_).Run(status, 0);
-      }
-    }
-
-   private:
-    ReadCallback callback_;
-
-    DISALLOW_COPY_AND_ASSIGN(BlobReaderClient);
-  };
-
-  void BindInternal(blink::mojom::BlobPtrInfo blob,
-                    network::mojom::DataPipeGetterRequest request) {
-    bindings_.set_connection_error_handler(base::BindRepeating(
-        &DataPipeGetter::OnConnectionError, base::Unretained(this)));
-    bindings_.AddBinding(this, std::move(request));
-    blob_.Bind(std::move(blob));
-  }
-
-  void OnConnectionError() {
-    if (bindings_.empty())
-      delete this;
-  }
-
-  // network::mojom::DataPipeGetter implementation:
-  void Read(mojo::ScopedDataPipeProducerHandle handle,
-            ReadCallback callback) override {
-    blink::mojom::BlobReaderClientPtr blob_reader_client_ptr;
-    mojo::MakeStrongBinding(
-        std::make_unique<BlobReaderClient>(std::move(callback)),
-        mojo::MakeRequest(&blob_reader_client_ptr));
-    blob_->ReadAll(std::move(handle), std::move(blob_reader_client_ptr));
-  }
-
-  void Clone(network::mojom::DataPipeGetterRequest request) override {
-    bindings_.AddBinding(this, std::move(request));
-  }
-
- private:
-  blink::mojom::BlobPtr blob_;
-  mojo::BindingSet<network::mojom::DataPipeGetter> bindings_;
-
-  DISALLOW_COPY_AND_ASSIGN(DataPipeGetter);
-};
-
 }  // namespace
 
 ResourceType WebURLRequestContextToResourceType(
@@ -480,9 +392,7 @@ scoped_refptr<network::ResourceRequestBody> GetRequestBodyForWebHTTPBody(
                                         blink::mojom::Blob::Version_));
 
           network::mojom::DataPipeGetterPtr data_pipe_getter_ptr;
-          // Object deletes itself.
-          new DataPipeGetter(std::move(blob_ptr),
-                             MakeRequest(&data_pipe_getter_ptr));
+          blob_ptr->AsDataPipeGetter(MakeRequest(&data_pipe_getter_ptr));
 
           request_body->AppendDataPipe(std::move(data_pipe_getter_ptr));
         } else {

storage/browser/blob/blob_impl.cc

@@ -39,6 +39,47 @@ class ReaderDelegate : public MojoBlobReader::Delegate {
  private:
   mojo::ScopedDataPipeProducerHandle handle_;
   blink::mojom::BlobReaderClientPtr client_;
+
+  DISALLOW_COPY_AND_ASSIGN(ReaderDelegate);
+};
+
+class DataPipeGetterReaderDelegate : public MojoBlobReader::Delegate {
+ public:
+  DataPipeGetterReaderDelegate(
+      mojo::ScopedDataPipeProducerHandle handle,
+      network::mojom::DataPipeGetter::ReadCallback callback)
+      : handle_(std::move(handle)), callback_(std::move(callback)) {}
+
+  mojo::ScopedDataPipeProducerHandle PassDataPipe() override {
+    return std::move(handle_);
+  }
+
+  MojoBlobReader::Delegate::RequestSideData DidCalculateSize(
+      uint64_t total_size,
+      uint64_t content_size) override {
+    // Check if null since it's conceivable OnComplete() was already called
+    // with error.
+    if (!callback_.is_null())
+      std::move(callback_).Run(net::OK, content_size);
+    return MojoBlobReader::Delegate::DONT_REQUEST_SIDE_DATA;
+  }
+
+  void OnComplete(net::Error result, uint64_t total_written_bytes) override {
+    // Check if null since DidCalculateSize() may have already been called
+    // and an error occurred later.
+    if (!callback_.is_null() && result != net::OK) {
+      // On error, signal failure immediately. On success, OnCalculatedSize()
+      // is guaranteed to be called, and the result will be signaled from
+      // there.
+      std::move(callback_).Run(result, 0);
+    }
+  }
+
+ private:
+  mojo::ScopedDataPipeProducerHandle handle_;
+  network::mojom::DataPipeGetter::ReadCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(DataPipeGetterReaderDelegate);
 };
 
 }  // namespace
@@ -54,6 +95,10 @@ void BlobImpl::Clone(blink::mojom::BlobRequest request) {
   bindings_.AddBinding(this, std::move(request));
 }
 
+void BlobImpl::AsDataPipeGetter(network::mojom::DataPipeGetterRequest request) {
+  data_pipe_getter_bindings_.AddBinding(this, std::move(request));
+}
+
 void BlobImpl::ReadRange(uint64_t offset,
                          uint64_t length,
                          mojo::ScopedDataPipeProducerHandle handle,
@@ -74,9 +119,21 @@ void BlobImpl::GetInternalUUID(GetInternalUUIDCallback callback) {
   std::move(callback).Run(handle_->uuid());
 }
 
+void BlobImpl::Clone(network::mojom::DataPipeGetterRequest request) {
+  data_pipe_getter_bindings_.AddBinding(this, std::move(request));
+}
+
+void BlobImpl::Read(mojo::ScopedDataPipeProducerHandle pipe,
+                    ReadCallback callback) {
+  MojoBlobReader::Create(handle_.get(), net::HttpByteRange(),
+                         std::make_unique<DataPipeGetterReaderDelegate>(
+                             std::move(pipe), std::move(callback)));
+}
+
 void BlobImpl::FlushForTesting() {
   bindings_.FlushForTesting();
-  if (bindings_.empty())
+  data_pipe_getter_bindings_.FlushForTesting();
+  if (bindings_.empty() && data_pipe_getter_bindings_.empty())
     delete this;
 }
 
@@ -85,8 +142,10 @@ BlobImpl::BlobImpl(std::unique_ptr<BlobDataHandle> handle,
     : handle_(std::move(handle)), weak_ptr_factory_(this) {
   DCHECK(handle_);
   bindings_.AddBinding(this, std::move(request));
-  bindings_.set_connection_error_handler(
-      base::Bind(&BlobImpl::OnConnectionError, base::Unretained(this)));
+  bindings_.set_connection_error_handler(base::BindRepeating(
+      &BlobImpl::OnConnectionError, base::Unretained(this)));
+  data_pipe_getter_bindings_.set_connection_error_handler(base::BindRepeating(
+      &BlobImpl::OnConnectionError, base::Unretained(this)));
 }
 
 BlobImpl::~BlobImpl() = default;
@@ -94,6 +153,8 @@ BlobImpl::~BlobImpl() = default;
 void BlobImpl::OnConnectionError() {
   if (!bindings_.empty())
     return;
+  if (!data_pipe_getter_bindings_.empty())
+    return;
   delete this;
 }
 

storage/browser/blob/blob_impl.h

@@ -6,6 +6,7 @@
 #define STORAGE_BROWSER_BLOB_BLOB_IMPL_H_
 
 #include "mojo/public/cpp/bindings/binding_set.h"
+#include "services/network/public/mojom/data_pipe_getter.mojom.h"
 #include "storage/browser/storage_browser_export.h"
 #include "third_party/WebKit/public/mojom/blob/blob.mojom.h"
 
@@ -14,12 +15,15 @@ namespace storage {
 class BlobDataHandle;
 
 // Self destroys when no more bindings exist.
-class STORAGE_EXPORT BlobImpl : public blink::mojom::Blob {
+class STORAGE_EXPORT BlobImpl : public blink::mojom::Blob,
+                                public network::mojom::DataPipeGetter {
  public:
   static base::WeakPtr<BlobImpl> Create(std::unique_ptr<BlobDataHandle> handle,
                                         blink::mojom::BlobRequest request);
 
+  // blink::mojom::Blob:
   void Clone(blink::mojom::BlobRequest request) override;
+  void AsDataPipeGetter(network::mojom::DataPipeGetterRequest request) override;
   void ReadRange(uint64_t offset,
                  uint64_t length,
                  mojo::ScopedDataPipeProducerHandle handle,
@@ -28,6 +32,11 @@ class STORAGE_EXPORT BlobImpl : public blink::mojom::Blob {
                blink::mojom::BlobReaderClientPtr client) override;
   void GetInternalUUID(GetInternalUUIDCallback callback) override;
 
+  // network::mojom::DataPipeGetter:
+  void Clone(network::mojom::DataPipeGetterRequest request) override;
+  void Read(mojo::ScopedDataPipeProducerHandle pipe,
+            ReadCallback callback) override;
+
   void FlushForTesting();
 
  private:
@@ -39,6 +48,7 @@ class STORAGE_EXPORT BlobImpl : public blink::mojom::Blob {
   std::unique_ptr<BlobDataHandle> handle_;
 
   mojo::BindingSet<blink::mojom::Blob> bindings_;
+  mojo::BindingSet<network::mojom::DataPipeGetter> data_pipe_getter_bindings_;
 
   base::WeakPtrFactory<BlobImpl> weak_ptr_factory_;
 

storage/browser/blob/blob_registry_impl_unittest.cc

@@ -47,6 +47,11 @@ class MockBlob : public blink::mojom::Blob {
                             std::move(request));
   }
 
+  void AsDataPipeGetter(
+      network::mojom::DataPipeGetterRequest request) override {
+    NOTREACHED();
+  }
+
   void ReadRange(uint64_t offset,
                  uint64_t size,
                  mojo::ScopedDataPipeProducerHandle,

third_party/WebKit/Source/platform/blob/testing/FakeBlob.cpp

@@ -15,6 +15,11 @@ void FakeBlob::Clone(mojom::blink::BlobRequest request) {
                           std::move(request));
 }
 
+void FakeBlob::AsDataPipeGetter(
+    network::mojom::blink::DataPipeGetterRequest request) {
+  NOTREACHED();
+}
+
 void FakeBlob::ReadRange(uint64_t offset,
                          uint64_t length,
                          mojo::ScopedDataPipeProducerHandle,

third_party/WebKit/Source/platform/blob/testing/FakeBlob.h

@@ -16,6 +16,7 @@ class FakeBlob : public mojom::blink::Blob {
   explicit FakeBlob(const String& uuid);
 
   void Clone(mojom::blink::BlobRequest) override;
+  void AsDataPipeGetter(network::mojom::blink::DataPipeGetterRequest) override;
   void ReadRange(uint64_t offset,
                  uint64_t length,
                  mojo::ScopedDataPipeProducerHandle,

third_party/WebKit/public/mojom/BUILD.gn

@@ -38,6 +38,10 @@ mojom("mojom_platform") {
     "//mojo/common:common_custom_types",
     "//mojo/public/mojom/base",
     "//services/network/public/mojom",
+
+    # TODO(https://crbug.com/822804): Remove when mojom bindings deps checks
+    # get fixed.
+    "//services/network/public/mojom:data_pipe_interfaces",
     "//ui/gfx/geometry/mojo",
     "//url/mojom:url_mojom_gurl",
     "//url/mojom:url_mojom_origin",

third_party/WebKit/public/mojom/blob/blob.mojom

@@ -4,6 +4,7 @@
 
 module blink.mojom;
 
+import "services/network/public/mojom/data_pipe_getter.mojom";
 import "services/network/public/mojom/http_request_headers.mojom";
 
 // Interface that can be implemented to be informed of certain information while
@@ -30,6 +31,9 @@ interface Blob {
   // Creates a copy of this Blob reference.
   Clone(Blob& blob);
 
+  // Creates a reference to this Blob as a DataPipeGetter.
+  AsDataPipeGetter(network.mojom.DataPipeGetter& data_pipe_getter);
+
   // Causes the entire contents of this blob to be written into the given data
   // pipe. An optional BlobReaderClient will be informed of the result of the
   // read operation.