Changeset 205110 in webkit

Timestamp:

Aug 28, 2016 11:49:42 PM (8 years ago)

Author:

commit-queue@webkit.org

Message:

[Fetch API] Ensure response cloning works when data is loading
​https://bugs.webkit.org/show_bug.cgi?id=161137

Patch by Youenn Fablet <​youenn@apple.com> on 2016-08-28
Reviewed by Darin Adler.

LayoutTests/imported/w3c:

• web-platform-tests/fetch/api/response/response-clone-expected.txt:
• web-platform-tests/fetch/api/response/response-clone.html: New tests highly inspired from Chromium similar tests.

Source/WebCore:

Test: http/tests/fetch/clone-after-load-is-finished.html and updated wpt test

FetchBody is only able to clone data when requested by Response in case the data is already there.
If data is loaded from FetchLoader, each received chunk should be given to each cloned response.
The simplest approach is to create a ReadableStream body for the cloned Response and use it for the teeing.

• Modules/fetch/FetchResponse.cpp:

(WebCore::FetchResponse::BodyLoader::didSucceed): Postpone stream closing if there is still data to enqueue in the FetchBody.

• Modules/fetch/FetchResponse.h:
• Modules/fetch/FetchResponse.idl:
• Modules/fetch/FetchResponse.js:

(clone): If response is being loaded, create a ReadableStream body so that loaded data cloning is handled by ReadableStream tee.

• Modules/streams/ReadableStreamInternals.js: Restrict firstReadCallback to the case of a readable ReadableStream.

If stream is errored or closed, FetchResponse will already have disposed of its source.
(readFromReadableStreamDefaultReader):

• bindings/js/WebCoreBuiltinNames.h:

LayoutTests:

• http/tests/fetch/clone-response-body-expected.txt: Added.
• http/tests/fetch/clone-response-body.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/fetch/clone-response-body-expected.txt (added)
• LayoutTests/http/tests/fetch/clone-response-body.html (added)
• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/fetch/api/response/response-clone-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/fetch/api/response/response-clone.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/fetch/FetchResponse.cpp (modified) (1 diff)
• Source/WebCore/Modules/fetch/FetchResponse.h (modified) (1 diff)
• Source/WebCore/Modules/fetch/FetchResponse.idl (modified) (1 diff)
• Source/WebCore/Modules/fetch/FetchResponse.js (modified) (1 diff)
• Source/WebCore/Modules/streams/ReadableStreamInternals.js (modified) (1 diff)
• Source/WebCore/bindings/js/WebCoreBuiltinNames.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-08-28  Youenn Fablet  <youenn@apple.com>
+
+        [Fetch API] Ensure response cloning works when data is loading
+        https://bugs.webkit.org/show_bug.cgi?id=161137
+
+        Reviewed by Darin Adler.
+
+        * http/tests/fetch/clone-response-body-expected.txt: Added.
+        * http/tests/fetch/clone-response-body.html: Added.
+
 2016-08-28  Jiewen Tan  <jiewen_tan@apple.com>
 

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2016-08-28  Youenn Fablet  <youenn@apple.com>
+
+        [Fetch API] Ensure response cloning works when data is loading
+        https://bugs.webkit.org/show_bug.cgi?id=161137
+
+        Reviewed by Darin Adler.
+
+        * web-platform-tests/fetch/api/response/response-clone-expected.txt:
+        * web-platform-tests/fetch/api/response/response-clone.html: New tests highly inspired from Chromium similar tests.
+
 2016-08-27  Youenn Fablet  <youenn@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/response/response-clone-expected.txt

@@ -5 +5 @@
 PASS Check cloned response's body 
 PASS Cannot clone a disturbed response 
+PASS Cloned responses should provide the same data 
+PASS Cancelling stream should not affect cloned one 
 

trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/response/response-clone.html

@@ -62 +62 @@
         });
       }, "Cannot clone a disturbed response");
+
+    promise_test(function(t) {
+        var clone;
+        var result;
+        var response;
+        return fetch('../resources/trickle.py?count=2&delay=100').then(function(res) {
+            clone = res.clone();
+            response = res;
+            return clone.arrayBuffer();
+        }).then(function(r) {
+            assert_equals(r.byteLength, 26);
+            result = r;
+            return response.arrayBuffer();
+        }).then(function(r) {
+            assert_array_equals(r, result, "cloned responses should provide the same data");
+        });
+     }, 'Cloned responses should provide the same data');
+
+    promise_test(function(t) {
+        var clone;
+        return fetch('../resources/trickle.py?count=2&delay=100').then(function(res) {
+            clone = res.clone();
+            res.body.cancel();
+            assert_true(res.bodyUsed);
+            assert_false(clone.bodyUsed);
+            return clone.arrayBuffer();
+        }).then(function(r) {
+            assert_equals(r.byteLength, 26);
+            assert_true(clone.bodyUsed);
+        });
+    }, 'Cancelling stream should not affect cloned one');
+
     </script>
   </body>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-08-28  Youenn Fablet  <youenn@apple.com>
+
+        [Fetch API] Ensure response cloning works when data is loading
+        https://bugs.webkit.org/show_bug.cgi?id=161137
+
+        Reviewed by Darin Adler.
+
+        Test: http/tests/fetch/clone-after-load-is-finished.html and updated wpt test
+
+        FetchBody is only able to clone data when requested by Response in case the data is already there.
+        If data is loaded from FetchLoader, each received chunk should be given to each cloned response.
+        The simplest approach is to create a ReadableStream body for the cloned Response and use it for the teeing.
+
+        * Modules/fetch/FetchResponse.cpp:
+        (WebCore::FetchResponse::BodyLoader::didSucceed): Postpone stream closing if there is still data to enqueue in the FetchBody.
+        * Modules/fetch/FetchResponse.h:
+        * Modules/fetch/FetchResponse.idl:
+        * Modules/fetch/FetchResponse.js:
+        (clone): If response is being loaded, create a ReadableStream body so that loaded data cloning is handled by ReadableStream tee.
+        * Modules/streams/ReadableStreamInternals.js: Restrict firstReadCallback to the case of a readable ReadableStream.
+        If stream is errored or closed, FetchResponse will already have disposed of its source.
+        (readFromReadableStreamDefaultReader):
+        * bindings/js/WebCoreBuiltinNames.h:
+
 2016-08-28  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp

@@ -128 +128 @@
 {
     ASSERT(m_response.hasPendingActivity());
+    m_response.m_body.loadingSucceeded();
+
 #if ENABLE(STREAMS_API)
-    if (m_response.m_readableStreamSource) {
+    if (m_response.m_readableStreamSource && m_response.m_body.type() != FetchBody::Type::Loaded) {
+        // We only close the stream if FetchBody already enqueued data.
+        // Otherwise, FetchBody will close the stream when enqueuing data.
         m_response.m_readableStreamSource->close();
         m_response.m_readableStreamSource = nullptr;
     }
 #endif
-    m_response.m_body.loadingSucceeded();
 
     if (m_loader->isStarted())

trunk/Source/WebCore/Modules/fetch/FetchResponse.h

@@ -86 +86 @@
     void cancel();
 #endif
+    bool isLoading() const { return body().type() == FetchBody::Type::Loading; }
 
 private:

trunk/Source/WebCore/Modules/fetch/FetchResponse.idl

@@ -71 +71 @@
 
     [PrivateIdentifier] Promise consume(unsigned short type);
+    [PrivateIdentifier] boolean isLoading();
     [PrivateIdentifier, RaisesException] void setStatus(unsigned short status, DOMString statusText);
     [CallWith=ScriptState, PrivateIdentifier] void initializeWith(any body);

trunk/Source/WebCore/Modules/fetch/FetchResponse.js

@@ -88 +88 @@
 
     var cloned = @Response.prototype.@cloneForJS.@call(this);
+
+    // Let's create @body if response body is loading to provide data to both clones.
+    if (@Response.prototype.@isLoading.@call(this) && !this.@body) {
+        var source = @Response.prototype.@createReadableStreamSource.@call(this);
+        @assert(!!source);
+        this.@body = new @ReadableStream(source);
+    }
+
     if (this.@body) {
         var teedReadableStreams = @teeReadableStream(this.@body, false);

trunk/Source/WebCore/Modules/streams/ReadableStreamInternals.js

@@ -344 +344 @@
 
     // Native sources may want to start enqueueing at the time of the first read request.
-    if (!stream.@disturbed && stream.@underlyingSource.@firstReadCallback)
+    if (!stream.@disturbed && stream.@state === @streamReadable && stream.@underlyingSource.@firstReadCallback)
         stream.@underlyingSource.@firstReadCallback();
 

trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h

@@ -55 +55 @@
     macro(initializeWith) \
     macro(isDisturbed) \
+    macro(isLoading) \
     macro(localStreams) \
     macro(makeThisTypeError) \