Make FetchDataLoader accept BytesConsumer

third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp

Index: third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp b/third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp
index 5a491d0024a46831b54bb985e05494a72e98b335..2ad908267c6922e683563ec4c4de66a95c31a424 100644
--- a/third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp
+++ b/third_party/WebKit/Source/modules/fetch/FetchDataLoader.cpp
@@ -5,6 +5,7 @@
 #include "modules/fetch/FetchDataLoader.h"
 
 #include "core/html/parser/TextResourceDecoder.h"
+#include "modules/fetch/BytesConsumer.h"
 #include "wtf/PtrUtil.h"
 #include "wtf/text/StringBuilder.h"
 #include "wtf/text/WTFString.h"
@@ -15,345 +16,265 @@ namespace blink {
 
 namespace {
 
-class FetchDataLoaderAsBlobHandle
-    : public FetchDataLoader
-    , public WebDataConsumerHandle::Client {
+class FetchDataLoaderAsBlobHandle final : public FetchDataLoader, public BytesConsumer::Client {
+    USING_GARBAGE_COLLECTED_MIXIN(FetchDataLoaderAsBlobHandle);
 public:
     explicit FetchDataLoaderAsBlobHandle(const String& mimeType)
-        : m_client(nullptr)
-        , m_mimeType(mimeType) { }
-
-    DEFINE_INLINE_VIRTUAL_TRACE()
+        : m_mimeType(mimeType)
     {
-        FetchDataLoader::trace(visitor);
-        visitor->trace(m_client);
     }
 
-private:
-    void start(FetchDataConsumerHandle* handle, FetchDataLoader::Client* client) override
+    void start(BytesConsumer* consumer, FetchDataLoader::Client* client) override
     {
-        ASSERT(!m_client);
-        ASSERT(!m_reader);
+        DCHECK(!m_client);
+        DCHECK(!m_consumer);
 
         m_client = client;
-        // Passing |this| here is safe because |this| owns |m_reader|.
-        m_reader = handle->obtainFetchDataReader(this);
-        RefPtr<BlobDataHandle> blobHandle = m_reader->drainAsBlobDataHandle();
+        m_consumer = consumer;
+
+        RefPtr<BlobDataHandle> blobHandle = m_consumer->drainAsBlobDataHandle();
         if (blobHandle) {
-            ASSERT(blobHandle->size() != UINT64_MAX);
-            m_reader.reset();
+            DCHECK_NE(UINT64_MAX, blobHandle->size());
             if (blobHandle->type() != m_mimeType) {
                 // A new BlobDataHandle is created to override the Blob's type.
                 m_client->didFetchDataLoadedBlobHandle(BlobDataHandle::create(blobHandle->uuid(), m_mimeType, blobHandle->size()));
             } else {
-                m_client->didFetchDataLoadedBlobHandle(blobHandle);
+                m_client->didFetchDataLoadedBlobHandle(std::move(blobHandle));
             }
-            m_client.clear();
             return;
         }
 
-        // We read data from |m_reader| and create a new blob.
         m_blobData = BlobData::create();
         m_blobData->setContentType(m_mimeType);
+        m_consumer->setClient(this);
+        onStateChange();
     }
 
-    void didGetReadable() override
+    void cancel() override
     {
-        ASSERT(m_client);
-        ASSERT(m_reader);
+        m_consumer->cancel();
+    }
 
+    void onStateChange() override
+    {
         while (true) {
-            const void* buffer;
+            const char* buffer;
             size_t available;
-            WebDataConsumerHandle::Result result = m_reader->beginRead(&buffer, WebDataConsumerHandle::FlagNone, &available);
-
-            switch (result) {
-            case WebDataConsumerHandle::Ok:
+            switch (m_consumer->beginRead(&buffer, &available)) {
+            case BytesConsumer::Result::Ok:
                 m_blobData->appendBytes(buffer, available);
-                m_reader->endRead(available);
+                m_consumer->endRead(available);
                 break;
-
-            case WebDataConsumerHandle::Done: {
-                m_reader.reset();
-                long long size = m_blobData->length();
+            case BytesConsumer::Result::ShouldWait:
+                return;
+            case BytesConsumer::Result::Done: {
+                auto size = m_blobData->length();
                 m_client->didFetchDataLoadedBlobHandle(BlobDataHandle::create(std::move(m_blobData), size));
-                m_client.clear();
                 return;
             }
-
-            case WebDataConsumerHandle::ShouldWait:
-                return;
-
-            case WebDataConsumerHandle::Busy:
-            case WebDataConsumerHandle::ResourceExhausted:
-            case WebDataConsumerHandle::UnexpectedError:
-                m_reader.reset();
-                m_blobData.reset();
+            case BytesConsumer::Result::Error:
                 m_client->didFetchDataLoadFailed();
-                m_client.clear();
                 return;
             }
         }
     }
 
-    void cancel() override
+    DEFINE_INLINE_TRACE()
     {
-        m_reader.reset();
-        m_blobData.reset();
-        m_client.clear();
+        visitor->trace(m_consumer);
+        visitor->trace(m_client);
+        FetchDataLoader::trace(visitor);
+        BytesConsumer::Client::trace(visitor);
     }
 
-    std::unique_ptr<FetchDataConsumerHandle::Reader> m_reader;
+private:
+    Member<BytesConsumer> m_consumer;
     Member<FetchDataLoader::Client> m_client;
 
     String m_mimeType;
     std::unique_ptr<BlobData> m_blobData;
 };
 
-class FetchDataLoaderAsArrayBuffer
-    : public FetchDataLoader
-    , public WebDataConsumerHandle::Client {
+class FetchDataLoaderAsArrayBuffer final : public FetchDataLoader, public BytesConsumer::Client {
+    USING_GARBAGE_COLLECTED_MIXIN(FetchDataLoaderAsArrayBuffer)
 public:
-    FetchDataLoaderAsArrayBuffer()
-        : m_client(nullptr) { }
-
-    DEFINE_INLINE_VIRTUAL_TRACE()
+    void start(BytesConsumer* consumer, FetchDataLoader::Client* client) override
     {
-        FetchDataLoader::trace(visitor);
-        visitor->trace(m_client);
+        DCHECK(!m_client);
+        DCHECK(!m_rawData);
+        DCHECK(!m_consumer);
+        m_client = client;
+        m_rawData = wrapUnique(new ArrayBufferBuilder());
+        m_consumer = consumer;
+        m_consumer->setClient(this);
+        onStateChange();
     }
 
-protected:
-    void start(FetchDataConsumerHandle* handle, FetchDataLoader::Client* client) override
+    void cancel() override
     {
-        ASSERT(!m_client);
-        ASSERT(!m_rawData);
-        ASSERT(!m_reader);
-        m_client = client;
-        m_rawData = wrapUnique(new ArrayBufferBuilder());
-        m_reader = handle->obtainFetchDataReader(this);
+        m_consumer->cancel();
     }
 
-    void didGetReadable() override
+    void onStateChange() override
     {
-        ASSERT(m_client);
-        ASSERT(m_rawData);
-        ASSERT(m_reader);
+        DCHECK(m_client);
+        DCHECK(m_rawData);
+        DCHECK(m_consumer);

[hiroshige, 2016/08/12 08:21:17]

This CL removes DCHECK(m_client) and DCHECK(m_consumer) in
FetchDataLoaderAsBlobHandle because we no longer explicitly clears these
pointers, while keeps the DCHECKs here. Can we make them consistent?

[yhirano, 2016/08/12 12:36:55]

Done.

 
         while (true) {
-            const void* buffer;
+            const char* buffer;
             size_t available;
-            WebDataConsumerHandle::Result result = m_reader->beginRead(&buffer, WebDataConsumerHandle::FlagNone, &available);
-
-            switch (result) {
-            case WebDataConsumerHandle::Ok:
+            switch (m_consumer->beginRead(&buffer, &available)) {
+            case BytesConsumer::Result::Ok:
                 if (available > 0) {
-                    unsigned bytesAppended = m_rawData->append(static_cast<const char*>(buffer), available);
+                    unsigned bytesAppended = m_rawData->append(buffer, available);
                     if (!bytesAppended) {
-                        m_reader->endRead(0);
-                        error();
+                        m_consumer->endRead(0);
+                        m_consumer->cancel();
+                        m_client->didFetchDataLoadFailed();
                         return;
                     }
-                    ASSERT(bytesAppended == available);
+                    DCHECK_EQ(bytesAppended, available);
                 }
-                m_reader->endRead(available);
+                m_consumer->endRead(available);
                 break;
-
-            case WebDataConsumerHandle::Done:
-                m_reader.reset();
-                m_client->didFetchDataLoadedArrayBuffer(DOMArrayBuffer::create(m_rawData->toArrayBuffer()));
-                m_rawData.reset();
-                m_client.clear();
+            case BytesConsumer::Result::ShouldWait:
                 return;
-
-            case WebDataConsumerHandle::ShouldWait:
+            case BytesConsumer::Result::Done:
+                m_client->didFetchDataLoadedArrayBuffer(DOMArrayBuffer::create(m_rawData->toArrayBuffer()));
                 return;
-
-            case WebDataConsumerHandle::Busy:
-            case WebDataConsumerHandle::ResourceExhausted:
-            case WebDataConsumerHandle::UnexpectedError:
-                error();
+            case BytesConsumer::Result::Error:
+                m_client->didFetchDataLoadFailed();
                 return;
             }
         }
     }
 
-    void error()
+    DEFINE_INLINE_TRACE()
     {
-        m_reader.reset();
-        m_rawData.reset();
-        m_client->didFetchDataLoadFailed();
-        m_client.clear();
-    }
-
-    void cancel() override
-    {
-        m_reader.reset();
-        m_rawData.reset();
-        m_client.clear();
+        visitor->trace(m_consumer);
+        visitor->trace(m_client);
+        FetchDataLoader::trace(visitor);
+        BytesConsumer::Client::trace(visitor);
     }
 
-    std::unique_ptr<FetchDataConsumerHandle::Reader> m_reader;
+private:
+    Member<BytesConsumer> m_consumer;
     Member<FetchDataLoader::Client> m_client;
 
     std::unique_ptr<ArrayBufferBuilder> m_rawData;
 };
 
-class FetchDataLoaderAsString
-    : public FetchDataLoader
-    , public WebDataConsumerHandle::Client {
+class FetchDataLoaderAsString final : public FetchDataLoader, public BytesConsumer::Client {
+    USING_GARBAGE_COLLECTED_MIXIN(FetchDataLoaderAsString);
 public:
-    FetchDataLoaderAsString()
-        : m_client(nullptr) { }
-
-    DEFINE_INLINE_VIRTUAL_TRACE()
-    {
-        FetchDataLoader::trace(visitor);
-        visitor->trace(m_client);
-    }
-
-protected:
-    void start(FetchDataConsumerHandle* handle, FetchDataLoader::Client* client) override
+    void start(BytesConsumer* consumer, FetchDataLoader::Client* client) override
     {
-        ASSERT(!m_client);
-        ASSERT(!m_decoder);
-        ASSERT(!m_reader);
+        DCHECK(!m_client);
+        DCHECK(!m_decoder);
+        DCHECK(!m_consumer);
         m_client = client;
         m_decoder = TextResourceDecoder::createAlwaysUseUTF8ForText();
-        m_reader = handle->obtainFetchDataReader(this);
+        m_consumer = consumer;
+        m_consumer->setClient(this);
+        onStateChange();
     }
 
-    void didGetReadable() override
+    void onStateChange() override
     {
-        ASSERT(m_client);
-        ASSERT(m_decoder);
-        ASSERT(m_reader);
+        DCHECK(m_client);
+        DCHECK(m_decoder);
+        DCHECK(m_consumer);

[hiroshige, 2016/08/12 08:21:17]

ditto.

[yhirano, 2016/08/12 12:36:55]

Done.

 
         while (true) {
-            const void* buffer;
+            const char* buffer;
             size_t available;
-            WebDataConsumerHandle::Result result = m_reader->beginRead(&buffer, WebDataConsumerHandle::FlagNone, &available);
-
-            switch (result) {
-            case WebDataConsumerHandle::Ok:
+            switch (m_consumer->beginRead(&buffer, &available)) {
+            case BytesConsumer::Result::Ok:
                 if (available > 0)
-                    m_builder.append(m_decoder->decode(static_cast<const char*>(buffer), available));
-                m_reader->endRead(available);
+                    m_builder.append(m_decoder->decode(buffer, available));
+                m_consumer->endRead(available);
                 break;
-
-            case WebDataConsumerHandle::Done:
-                m_reader.reset();
+            case BytesConsumer::Result::ShouldWait:
+                return;
+            case BytesConsumer::Result::Done:
                 m_builder.append(m_decoder->flush());
                 m_client->didFetchDataLoadedString(m_builder.toString());
-                m_builder.clear();
-                m_decoder.reset();
-                m_client.clear();
-                return;
-
-            case WebDataConsumerHandle::ShouldWait:
                 return;
-
-            case WebDataConsumerHandle::Busy:
-            case WebDataConsumerHandle::ResourceExhausted:
-            case WebDataConsumerHandle::UnexpectedError:
-                error();
+            case BytesConsumer::Result::Error:
+                m_client->didFetchDataLoadFailed();
                 return;
             }
         }
     }
 
-    void error()
+    void cancel() override
     {
-        m_reader.reset();
-        m_builder.clear();
-        m_decoder.reset();
-        m_client->didFetchDataLoadFailed();
-        m_client.clear();
+        m_consumer->cancel();
     }
 
-    void cancel() override
+    DEFINE_INLINE_TRACE()
     {
-        m_reader.reset();
-        m_builder.clear();
-        m_decoder.reset();
-        m_client.clear();
+        visitor->trace(m_consumer);
+        visitor->trace(m_client);
+        FetchDataLoader::trace(visitor);
+        BytesConsumer::Client::trace(visitor);
     }
 
-    std::unique_ptr<FetchDataConsumerHandle::Reader> m_reader;
+private:
+    Member<BytesConsumer> m_consumer;
     Member<FetchDataLoader::Client> m_client;
 
     std::unique_ptr<TextResourceDecoder> m_decoder;
     StringBuilder m_builder;
 };
 
-class FetchDataLoaderAsStream
-    : public FetchDataLoader
-    , public WebDataConsumerHandle::Client {
+class FetchDataLoaderAsStream final : public FetchDataLoader, public BytesConsumer::Client {
+    USING_GARBAGE_COLLECTED_MIXIN(FetchDataLoaderAsStream);
 public:
     explicit FetchDataLoaderAsStream(Stream* outStream)
-        : m_client(nullptr)
-        , m_outStream(outStream) { }
-
-    DEFINE_INLINE_VIRTUAL_TRACE()
+        : m_outStream(outStream)
     {
-        FetchDataLoader::trace(visitor);
-        visitor->trace(m_client);
-        visitor->trace(m_outStream);
     }
 
-protected:
-    void start(FetchDataConsumerHandle* handle, FetchDataLoader::Client* client) override
+    void start(BytesConsumer* consumer, FetchDataLoader::Client* client) override
     {
-        ASSERT(!m_client);
-        ASSERT(!m_reader);
+        DCHECK(!m_client);
+        DCHECK(!m_consumer);
         m_client = client;
-        m_reader = handle->obtainFetchDataReader(this);
+        m_consumer = consumer;
+        m_consumer->setClient(this);
+        onStateChange();
     }
 
-    void didGetReadable() override
+    void onStateChange() override
     {
-        ASSERT(m_client);
-        ASSERT(m_reader);
+        DCHECK(m_client);
+        DCHECK(m_consumer);

[hiroshige, 2016/08/12 08:21:17]

ditto.

[yhirano, 2016/08/12 12:36:55]

Done.

 
         bool needToFlush = false;
         while (true) {
-            const void* buffer;
+            const char* buffer;
             size_t available;
-            WebDataConsumerHandle::Result result = m_reader->beginRead(&buffer, WebDataConsumerHandle::FlagNone, &available);
-
-            switch (result) {
-            case WebDataConsumerHandle::Ok:
-                m_outStream->addData(static_cast<const char*>(buffer), available);
-                m_reader->endRead(available);
+            switch (m_consumer->beginRead(&buffer, &available)) {
+            case BytesConsumer::Result::Ok:
+                m_outStream->addData(buffer, available);
+                m_consumer->endRead(available);
                 needToFlush = true;
                 break;
-
-            case WebDataConsumerHandle::Done:
-                m_reader.reset();
+            case BytesConsumer::Result::ShouldWait:
                 if (needToFlush)
                     m_outStream->flush();
-                m_outStream->finalize();
-                m_client->didFetchDataLoadedStream();
-                cleanup();
                 return;
-
-            case WebDataConsumerHandle::ShouldWait:
+            case BytesConsumer::Result::Done:
                 if (needToFlush)
                     m_outStream->flush();
+                m_outStream->finalize();
+                m_client->didFetchDataLoadedStream();
                 return;
-
-            case WebDataConsumerHandle::Busy:
-            case WebDataConsumerHandle::ResourceExhausted:
-            case WebDataConsumerHandle::UnexpectedError:
-                // If the stream is aborted soon after the stream is registered
-                // to the StreamRegistry, ServiceWorkerURLRequestJob may not
-                // notice the error and continue waiting forever.
-                // FIXME: Add new message to report the error to the browser
-                // process.

[hiroshige, 2016/08/12 08:21:17]

Is this issue fixed?

[yhirano, 2016/08/12 12:36:55]

Oops, no.

-                m_reader.reset();
+            case BytesConsumer::Result::Error:
                 m_outStream->abort();
                 m_client->didFetchDataLoadFailed();
-                cleanup();
                 return;
             }
         }
@@ -361,23 +282,23 @@ protected:
 
     void cancel() override
     {
-        cleanup();
+        m_consumer->cancel();
     }
 
-    void cleanup()
+    DEFINE_INLINE_TRACE()
     {
-        m_reader.reset();
-        m_client.clear();
-        m_outStream.clear();
+        visitor->trace(m_consumer);
+        visitor->trace(m_client);
+        visitor->trace(m_outStream);
+        FetchDataLoader::trace(visitor);
+        BytesConsumer::Client::trace(visitor);
     }
 
-    std::unique_ptr<FetchDataConsumerHandle::Reader> m_reader;
+    Member<BytesConsumer> m_consumer;
     Member<FetchDataLoader::Client> m_client;
-
     Member<Stream> m_outStream;
 };
 
-
 } // namespace
 
 FetchDataLoader* FetchDataLoader::createLoaderAsBlobHandle(const String& mimeType)