Rewrite BlobBytesConsumer to not rely on blob URLs.

The existing implementation depended on the non-mojo blob URL code path,
and so would break soon after mojo blob URLs ship when the old code gets
deleted. So rewrite the implementation to read blobs more directly rather
than going through a blob URL.

Bug: 859109
Change-Id: Ia6f2432e48ac5304ea8f5598d293e31c2c1e1905
Reviewed-on: https://chromium-review.googlesource.com/1120679
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572363}

third_party/WebKit/LayoutTests/external/wpt/fetch/api/response/response-cancel-stream.html

@@ -30,7 +30,9 @@ promise_test(function(test) {
     var closedPromise = reader.closed.then(function() {
         return reader.cancel();
     });
-    reader.read();
+    reader.read().then(function readMore({done, value}) {
+      if (!done) return reader.read().then(readMore);
+    });
     return closedPromise;
 }, "Cancelling a closed blob Response stream");
 

third_party/blink/renderer/core/fetch/blob_bytes_consumer.h

@@ -8,28 +8,17 @@
 #include <memory>
 #include "base/memory/scoped_refptr.h"
 #include "third_party/blink/renderer/core/core_export.h"
-#include "third_party/blink/renderer/core/dom/context_lifecycle_observer.h"
 #include "third_party/blink/renderer/core/fetch/bytes_consumer.h"
-#include "third_party/blink/renderer/core/loader/threadable_loader_client.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 
 namespace blink {
 
 class BlobDataHandle;
-class EncodedFormData;
 class ExecutionContext;
-class ThreadableLoader;
-class WebDataConsumerHandle;
 
 // A BlobBytesConsumer is created from a blob handle and it will
 // return a valid handle from drainAsBlobDataHandle as much as possible.
-class CORE_EXPORT BlobBytesConsumer final : public BytesConsumer,
-                                            public ContextLifecycleObserver,
-                                            public BytesConsumer::Client,
-                                            public ThreadableLoaderClient {
-  USING_GARBAGE_COLLECTED_MIXIN(BlobBytesConsumer);
-  USING_PRE_FINALIZER(BlobBytesConsumer, Cancel);
-
+class CORE_EXPORT BlobBytesConsumer final : public BytesConsumer {
  public:
   // |handle| can be null. In that case this consumer gets closed.
   BlobBytesConsumer(ExecutionContext*,
@@ -48,47 +37,13 @@ class CORE_EXPORT BlobBytesConsumer final : public BytesConsumer,
   Error GetError() const override;
   String DebugName() const override { return "BlobBytesConsumer"; }
 
-  // ContextLifecycleObserver implementation
-  void ContextDestroyed(ExecutionContext*) override;
-
-  // BytesConsumer::Client implementation
-  void OnStateChange() override;
-
-  // ThreadableLoaderClient implementation
-  void DidReceiveResponse(unsigned long identifier,
-                          const ResourceResponse&,
-                          std::unique_ptr<WebDataConsumerHandle>) override;
-  void DidFinishLoading(unsigned long identifier) override;
-  void DidFail(const ResourceError&) override;
-  void DidFailRedirectCheck() override;
-
   void Trace(blink::Visitor*) override;
 
-  static BlobBytesConsumer* CreateForTesting(ExecutionContext*,
-                                             scoped_refptr<BlobDataHandle>,
-                                             ThreadableLoader*);
-
  private:
-  BlobBytesConsumer(ExecutionContext*,
-                    scoped_refptr<BlobDataHandle>,
-                    ThreadableLoader*);
-  ThreadableLoader* CreateLoader();
-  void DidFailInternal();
-  bool IsClean() const { return blob_data_handle_.get(); }
-  void Close();
-  void GetError();
-  void Clear();
-
-  KURL blob_url_;
+  Member<ExecutionContext> execution_context_;
   scoped_refptr<BlobDataHandle> blob_data_handle_;
-  Member<BytesConsumer> body_;
+  Member<BytesConsumer> nested_consumer_;
   Member<BytesConsumer::Client> client_;
-  Member<ThreadableLoader> loader_;
-
-  PublicState state_ = PublicState::kReadableOrWaiting;
-  // These two booleans are meaningful only when m_state is ReadableOrWaiting.
-  bool has_seen_end_of_data_ = false;
-  bool has_finished_loading_ = false;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/fetch/form_data_bytes_consumer.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/fetch/form_data_bytes_consumer.h"
 
 #include "third_party/blink/public/platform/platform.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/fetch/blob_bytes_consumer.h"
 #include "third_party/blink/renderer/core/fetch/bytes_consumer_for_data_consumer_handle.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
@@ -319,7 +320,6 @@ class DataPipeAndDataBytesConsumer final : public BytesConsumer {
       case PublicState::kErrored:
         return;
       case PublicState::kClosed:
-        NOTREACHED();
         return;
       case PublicState::kReadableOrWaiting:
         break;

third_party/blink/renderer/platform/blob/blob_data.cc

@@ -362,6 +362,18 @@ BlobPtr BlobDataHandle::CloneBlobPtr() {
   return blob_clone;
 }
 
+network::mojom::blink::DataPipeGetterPtr BlobDataHandle::AsDataPipeGetter() {
+  MutexLocker locker(blob_info_mutex_);
+  if (!blob_info_.is_valid())
+    return nullptr;
+  network::mojom::blink::DataPipeGetterPtr result;
+  BlobPtr blob;
+  blob.Bind(std::move(blob_info_));
+  blob->AsDataPipeGetter(MakeRequest(&result));
+  blob_info_ = blob.PassInterface();
+  return result;
+}
+
 void BlobDataHandle::ReadAll(mojo::ScopedDataPipeProducerHandle pipe,
                              mojom::blink::BlobReaderClientPtr client) {
   MutexLocker locker(blob_info_mutex_);

third_party/blink/renderer/platform/blob/blob_data.h

@@ -192,6 +192,7 @@ class PLATFORM_EXPORT BlobDataHandle
   ~BlobDataHandle();
 
   mojom::blink::BlobPtr CloneBlobPtr();
+  network::mojom::blink::DataPipeGetterPtr AsDataPipeGetter();
 
   void ReadAll(mojo::ScopedDataPipeProducerHandle,
                mojom::blink::BlobReaderClientPtr);

third_party/blink/renderer/platform/blob/blob_registry.h

@@ -38,7 +38,6 @@
 
 namespace blink {
 
-class BlobBytesConsumer;
 class BlobDataHandle;
 class BlobURLRegistry;
 class KURL;
@@ -49,12 +48,11 @@ class PLATFORM_EXPORT BlobRegistry {
   STATIC_ONLY(BlobRegistry);
 
   // Calling methods in this class directly won't work when Blob URL management
-  // is switched to mojo. Instead codew should call PublicURLManager methods to
+  // is switched to mojo. Instead code should call PublicURLManager methods to
   // create/revoke blob URLs.
   // To avoid new usage of these methods, mark all as private with friends for
   // existing usage.
  private:
-  friend class BlobBytesConsumer;
   friend class BlobURLRegistry;
 
   // Methods for controlling Blob URLs.

third_party/blink/renderer/platform/blob/testing/fake_blob.cc

@@ -5,19 +5,52 @@
 #include "third_party/blink/renderer/platform/blob/testing/fake_blob.h"
 
 #include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/system/data_pipe_utils.h"
+#include "third_party/blink/public/platform/web_string.h"
 
 namespace blink {
+namespace {
 
-FakeBlob::FakeBlob(const String& uuid) : uuid_(uuid) {}
+class SimpleDataPipeGetter : public network::mojom::blink::DataPipeGetter {
+ public:
+  SimpleDataPipeGetter(const String& str) : str_(str) {}
+  ~SimpleDataPipeGetter() override = default;
+
+  // network::mojom::DataPipeGetter implementation:
+  void Read(mojo::ScopedDataPipeProducerHandle handle,
+            ReadCallback callback) override {
+    std::move(callback).Run(0 /* OK */, str_.length());
+    bool result = mojo::BlockingCopyFromString(WebString(str_).Utf8(), handle);
+    DCHECK(result);
+  }
+
+  void Clone(network::mojom::blink::DataPipeGetterRequest request) override {
+    mojo::MakeStrongBinding(std::make_unique<SimpleDataPipeGetter>(str_),
+                            std::move(request));
+  }
+
+ private:
+  String str_;
+
+  DISALLOW_COPY_AND_ASSIGN(SimpleDataPipeGetter);
+};
+
+}  // namespace
+
+FakeBlob::FakeBlob(const String& uuid, const String& body, State* state)
+    : uuid_(uuid), body_(body), state_(state) {}
 
 void FakeBlob::Clone(mojom::blink::BlobRequest request) {
-  mojo::MakeStrongBinding(std::make_unique<FakeBlob>(uuid_),
+  mojo::MakeStrongBinding(std::make_unique<FakeBlob>(uuid_, body_, state_),
                           std::move(request));
 }
 
 void FakeBlob::AsDataPipeGetter(
     network::mojom::blink::DataPipeGetterRequest request) {
-  NOTREACHED();
+  if (state_)
+    state_->did_initiate_read_operation = true;
+  mojo::MakeStrongBinding(std::make_unique<SimpleDataPipeGetter>(body_),
+                          std::move(request));
 }
 
 void FakeBlob::ReadRange(uint64_t offset,
@@ -27,9 +60,16 @@ void FakeBlob::ReadRange(uint64_t offset,
   NOTREACHED();
 }
 
-void FakeBlob::ReadAll(mojo::ScopedDataPipeProducerHandle,
-                       mojom::blink::BlobReaderClientPtr) {
-  NOTREACHED();
+void FakeBlob::ReadAll(mojo::ScopedDataPipeProducerHandle handle,
+                       mojom::blink::BlobReaderClientPtr client) {
+  if (state_)
+    state_->did_initiate_read_operation = true;
+  if (client)
+    client->OnCalculatedSize(body_.length(), body_.length());
+  bool result = mojo::BlockingCopyFromString(WebString(body_).Utf8(), handle);
+  DCHECK(result);
+  if (client)
+    client->OnComplete(0 /* OK */, body_.length());
 }
 
 void FakeBlob::ReadSideData(ReadSideDataCallback callback) {

third_party/blink/renderer/platform/blob/testing/fake_blob.h

@@ -9,11 +9,17 @@
 
 namespace blink {
 
-// Mocked Blob implementation for testing. You can't read from a FakeBlob, but
-// it does have a UUID.
+// Mocked Blob implementation for testing. Implements all methods except for
+// ReadRange and ReadSideData.
 class FakeBlob : public mojom::blink::Blob {
  public:
-  explicit FakeBlob(const String& uuid);
+  struct State {
+    bool did_initiate_read_operation = false;
+  };
+
+  FakeBlob(const String& uuid,
+           const String& body = String(),
+           State* state = nullptr);
 
   void Clone(mojom::blink::BlobRequest) override;
   void AsDataPipeGetter(network::mojom::blink::DataPipeGetterRequest) override;
@@ -25,9 +31,10 @@ class FakeBlob : public mojom::blink::Blob {
                mojom::blink::BlobReaderClientPtr) override;
   void ReadSideData(ReadSideDataCallback) override;
   void GetInternalUUID(GetInternalUUIDCallback) override;
-
  private:
   String uuid_;
+  String body_;
+  State* state_;
 };
 
 }  // namespace blink