[Fetch API] Response bodyUsed should check for its body disturbed state
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Aug 2016 16:36:57 +0000 (16:36 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Aug 2016 16:36:57 +0000 (16:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161429

Patch by Youenn Fablet <youenn@apple.com> on 2016-08-31
Reviewed by Alex Christensen.

Source/WebCore:

Test: http/tests/fetch/bodyUsed-worker.html

Implementing bodyUsed as a JS Builtin.
This allows using directly @isReadableStreamDisturbed if response has a body.

Renaming isDisturbed to isDisturbedOrLocked to better match fetch spec terminology.

* Modules/fetch/FetchBodyOwner.cpp:
(WebCore::FetchBodyOwner::isDisturbedOrLocked): Renaming isDisturbed to isDisturbedOrLocked.
(WebCore::FetchBodyOwner::arrayBuffer): Ditto.
(WebCore::FetchBodyOwner::blob): Ditto.
(WebCore::FetchBodyOwner::formData): Ditto.
(WebCore::FetchBodyOwner::json): Ditto.
(WebCore::FetchBodyOwner::text): Ditto.
(WebCore::FetchBodyOwner::isDisturbed): Ditto.
* Modules/fetch/FetchBodyOwner.h:
(WebCore::FetchBodyOwner::isDisturbed): Ditto.
* Modules/fetch/FetchRequest.cpp:
(WebCore::FetchRequest::initializeWith): Ditto.
(WebCore::FetchRequest::clone): Ditto.
* Modules/fetch/FetchResponse.cpp:
(WebCore::FetchResponse::cloneForJS): Ditto.
(WebCore::FetchResponse::createReadableStreamSource): Only asserting for isDisturbed.
* Modules/fetch/FetchResponse.idl: Marking bodyUsed as JS builtin.
* Modules/fetch/FetchResponse.js: Adding bodyUsed.
(bodyUsed):
(clone):

LayoutTests:

* http/tests/fetch/bodyUsed-expected.txt: Added.
* http/tests/fetch/bodyUsed-worker-expected.txt: Added.
* http/tests/fetch/bodyUsed-worker.html: Added.
* http/tests/fetch/bodyUsed.js: Added.
* http/tests/fetch/window/body-mixin-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@205251 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/fetch/bodyUsed-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/fetch/bodyUsed-worker-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/fetch/bodyUsed-worker.html [new file with mode: 0644]
LayoutTests/http/tests/fetch/bodyUsed.html [new file with mode: 0644]
LayoutTests/http/tests/fetch/bodyUsed.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/fetch/FetchBodyOwner.cpp
Source/WebCore/Modules/fetch/FetchBodyOwner.h
Source/WebCore/Modules/fetch/FetchRequest.cpp
Source/WebCore/Modules/fetch/FetchResponse.cpp
Source/WebCore/Modules/fetch/FetchResponse.idl
Source/WebCore/Modules/fetch/FetchResponse.js

index 1ca11f9..6d5fe74 100644 (file)
@@ -1,5 +1,18 @@
 2016-08-31  Youenn Fablet  <youenn@apple.com>
 
+        [Fetch API] Response bodyUsed should check for its body disturbed state
+        https://bugs.webkit.org/show_bug.cgi?id=161429
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/fetch/bodyUsed-expected.txt: Added.
+        * http/tests/fetch/bodyUsed-worker-expected.txt: Added.
+        * http/tests/fetch/bodyUsed-worker.html: Added.
+        * http/tests/fetch/bodyUsed.js: Added.
+        * http/tests/fetch/window/body-mixin-expected.txt:
+
+2016-08-31  Youenn Fablet  <youenn@apple.com>
+
         [Fetch API] Blob type should be correctly set in case of empty body
         https://bugs.webkit.org/show_bug.cgi?id=161431
 
diff --git a/LayoutTests/http/tests/fetch/bodyUsed-expected.txt b/LayoutTests/http/tests/fetch/bodyUsed-expected.txt
new file mode 100644 (file)
index 0000000..f8429e6
--- /dev/null
@@ -0,0 +1,6 @@
+
+PASS BodyUsedShouldNotBeSetForNullBody 
+PASS BodyUsedShouldBeSetForEmptyBody 
+PASS BodyUsedShouldBeSetWhenRead 
+PASS BodyUsedShouldBeSetWhenCancelled 
+
diff --git a/LayoutTests/http/tests/fetch/bodyUsed-worker-expected.txt b/LayoutTests/http/tests/fetch/bodyUsed-worker-expected.txt
new file mode 100644 (file)
index 0000000..f8429e6
--- /dev/null
@@ -0,0 +1,6 @@
+
+PASS BodyUsedShouldNotBeSetForNullBody 
+PASS BodyUsedShouldBeSetForEmptyBody 
+PASS BodyUsedShouldBeSetWhenRead 
+PASS BodyUsedShouldBeSetWhenCancelled 
+
diff --git a/LayoutTests/http/tests/fetch/bodyUsed-worker.html b/LayoutTests/http/tests/fetch/bodyUsed-worker.html
new file mode 100644 (file)
index 0000000..72afce7
--- /dev/null
@@ -0,0 +1,17 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Response body used tests</title>
+        <meta name="help" href="https://fetch.spec.whatwg.org/#response">
+        <meta name="help" href="https://fetch.spec.whatwg.org/#body-mixin">
+        <script src="/resources/testharness.js"></script>
+        <script src="/resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <script>
+            fetch_tests_from_worker(new Worker("bodyUsed.js"));
+        </script>
+    </body>
+</html>
+
diff --git a/LayoutTests/http/tests/fetch/bodyUsed.html b/LayoutTests/http/tests/fetch/bodyUsed.html
new file mode 100644 (file)
index 0000000..85bde8c
--- /dev/null
@@ -0,0 +1,14 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Response body used tests</title>
+        <meta name="help" href="https://fetch.spec.whatwg.org/#response">
+        <meta name="help" href="https://fetch.spec.whatwg.org/#body-mixin">
+        <script src="/resources/testharness.js"></script>
+        <script src="/resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <script src="bodyUsed.js"></script>
+    </body>
+</html>
diff --git a/LayoutTests/http/tests/fetch/bodyUsed.js b/LayoutTests/http/tests/fetch/bodyUsed.js
new file mode 100644 (file)
index 0000000..27ba229
--- /dev/null
@@ -0,0 +1,38 @@
+if (this.document === undefined) {
+      importScripts("/resources/testharness.js");
+}
+
+// Four following tests are taken from Chromium fetch tests, see body-mixin.js file
+test(t => {
+    var req = new Request('/');
+    assert_false(req.bodyUsed);
+    req.text();
+    assert_false(req.bodyUsed);
+}, 'BodyUsedShouldNotBeSetForNullBody');
+
+test(t => {
+    var req = new Request('/', {method: 'POST', body: ''});
+    assert_false(req.bodyUsed);
+    req.text();
+    assert_true(req.bodyUsed);
+}, 'BodyUsedShouldBeSetForEmptyBody');
+
+test(t => {
+    var res = new Response('');
+    assert_false(res.bodyUsed);
+    var reader = res.body.getReader();
+    assert_false(res.bodyUsed);
+    reader.read();
+    assert_true(res.bodyUsed);
+}, 'BodyUsedShouldBeSetWhenRead');
+
+test(t => {
+    var res = new Response('');
+    assert_false(res.bodyUsed);
+    var reader = res.body.getReader();
+    assert_false(res.bodyUsed);
+    reader.cancel();
+    assert_true(res.bodyUsed);
+}, 'BodyUsedShouldBeSetWhenCancelled');
+
+done();
index 1e8e48d..3b79471 100644 (file)
@@ -1,5 +1,40 @@
 2016-08-31  Youenn Fablet  <youenn@apple.com>
 
+        [Fetch API] Response bodyUsed should check for its body disturbed state
+        https://bugs.webkit.org/show_bug.cgi?id=161429
+
+        Reviewed by Alex Christensen.
+
+        Test: http/tests/fetch/bodyUsed-worker.html
+
+        Implementing bodyUsed as a JS Builtin.
+        This allows using directly @isReadableStreamDisturbed if response has a body.
+
+        Renaming isDisturbed to isDisturbedOrLocked to better match fetch spec terminology.
+
+        * Modules/fetch/FetchBodyOwner.cpp:
+        (WebCore::FetchBodyOwner::isDisturbedOrLocked): Renaming isDisturbed to isDisturbedOrLocked.
+        (WebCore::FetchBodyOwner::arrayBuffer): Ditto.
+        (WebCore::FetchBodyOwner::blob): Ditto.
+        (WebCore::FetchBodyOwner::formData): Ditto.
+        (WebCore::FetchBodyOwner::json): Ditto.
+        (WebCore::FetchBodyOwner::text): Ditto.
+        (WebCore::FetchBodyOwner::isDisturbed): Ditto.
+        * Modules/fetch/FetchBodyOwner.h:
+        (WebCore::FetchBodyOwner::isDisturbed): Ditto.
+        * Modules/fetch/FetchRequest.cpp:
+        (WebCore::FetchRequest::initializeWith): Ditto.
+        (WebCore::FetchRequest::clone): Ditto.
+        * Modules/fetch/FetchResponse.cpp:
+        (WebCore::FetchResponse::cloneForJS): Ditto.
+        (WebCore::FetchResponse::createReadableStreamSource): Only asserting for isDisturbed.
+        * Modules/fetch/FetchResponse.idl: Marking bodyUsed as JS builtin.
+        * Modules/fetch/FetchResponse.js: Adding bodyUsed.
+        (bodyUsed):
+        (clone):
+
+2016-08-31  Youenn Fablet  <youenn@apple.com>
+
         [Fetch API] Blob type should be correctly set in case of empty body
         https://bugs.webkit.org/show_bug.cgi?id=161431
 
index 1d0333b..4fd0cf5 100644 (file)
@@ -58,7 +58,7 @@ void FetchBodyOwner::stop()
     ASSERT(!m_blobLoader);
 }
 
-bool FetchBodyOwner::isDisturbed() const
+bool FetchBodyOwner::isDisturbedOrLocked() const
 {
     if (m_isDisturbed)
         return true;
@@ -77,7 +77,7 @@ void FetchBodyOwner::arrayBuffer(DeferredWrapper&& promise)
         fulfillPromiseWithArrayBuffer(promise, nullptr, 0);
         return;
     }
-    if (isDisturbed()) {
+    if (isDisturbedOrLocked()) {
         promise.reject(TypeError);
         return;
     }
@@ -91,7 +91,7 @@ void FetchBodyOwner::blob(DeferredWrapper&& promise)
         promise.resolve(Blob::create(Vector<uint8_t>(), Blob::normalizedContentType(extractMIMETypeFromMediaType(m_body.contentType()))));
         return;
     }
-    if (isDisturbed()) {
+    if (isDisturbedOrLocked()) {
         promise.reject(TypeError);
         return;
     }
@@ -105,7 +105,7 @@ void FetchBodyOwner::formData(DeferredWrapper&& promise)
         promise.reject(0);
         return;
     }
-    if (isDisturbed()) {
+    if (isDisturbedOrLocked()) {
         promise.reject(TypeError);
         return;
     }
@@ -119,7 +119,7 @@ void FetchBodyOwner::json(DeferredWrapper&& promise)
         promise.reject(SYNTAX_ERR);
         return;
     }
-    if (isDisturbed()) {
+    if (isDisturbedOrLocked()) {
         promise.reject(TypeError);
         return;
     }
@@ -133,7 +133,7 @@ void FetchBodyOwner::text(DeferredWrapper&& promise)
         promise.resolve(String());
         return;
     }
-    if (isDisturbed()) {
+    if (isDisturbedOrLocked()) {
         promise.reject(TypeError);
         return;
     }
index a60b1ad..a7f4847 100644 (file)
@@ -44,7 +44,7 @@ public:
     FetchBodyOwner(ScriptExecutionContext&, FetchBody&&);
 
     // Exposed Body API
-    bool isDisturbed() const;
+    bool isDisturbed() const { return m_isDisturbed; };
 
     void arrayBuffer(DeferredWrapper&&);
     void blob(DeferredWrapper&&);
@@ -52,6 +52,8 @@ public:
     void json(DeferredWrapper&&);
     void text(DeferredWrapper&&);
 
+    bool isDisturbedOrLocked() const;
+
     void loadBlob(Blob&, FetchBodyConsumer*);
 
     bool isActive() const { return !!m_blobLoader; }
index 185adcc..1379f44 100644 (file)
@@ -243,7 +243,7 @@ FetchHeaders* FetchRequest::initializeWith(const String& url, const Dictionary&
 
 FetchHeaders* FetchRequest::initializeWith(FetchRequest& input, const Dictionary& init, ExceptionCode& ec)
 {
-    if (input.isDisturbed()) {
+    if (input.isDisturbedOrLocked()) {
         ec = TypeError;
         return nullptr;
     }
@@ -308,7 +308,7 @@ ResourceRequest FetchRequest::internalRequest() const
 
 RefPtr<FetchRequest> FetchRequest::clone(ScriptExecutionContext& context, ExceptionCode& ec)
 {
-    if (isDisturbed()) {
+    if (isDisturbedOrLocked()) {
         ec = TypeError;
         return nullptr;
     }
index 89e5aa9..c6e3e69 100644 (file)
@@ -97,7 +97,7 @@ FetchResponse::FetchResponse(ScriptExecutionContext& context, FetchBody&& body,
 Ref<FetchResponse> FetchResponse::cloneForJS()
 {
     ASSERT(scriptExecutionContext());
-    ASSERT(!isDisturbed());
+    ASSERT(!isDisturbedOrLocked());
     return adoptRef(*new FetchResponse(*scriptExecutionContext(), FetchBody(m_body), FetchHeaders::create(headers()), ResourceResponse(m_response)));
 }
 
@@ -270,7 +270,7 @@ void FetchResponse::consumeBodyAsStream()
 ReadableStreamSource* FetchResponse::createReadableStreamSource()
 {
     ASSERT(!m_readableStreamSource);
-    ASSERT(!isDisturbed());
+    ASSERT(!m_isDisturbed);
 
     if (body().isEmpty())
         return nullptr;
index 9b4589c..4019b83 100644 (file)
@@ -54,7 +54,7 @@ interface FetchResponse {
     [JSBuiltin] readonly attribute ReadableStream? body;
 
     // Copy of FetchBody IDL as we want to implement some of these as built-ins.
-    [ImplementedAs=isDisturbed] readonly attribute boolean bodyUsed;
+    [JSBuiltin] readonly attribute boolean bodyUsed;
     [JSBuiltin] Promise arrayBuffer();
     [JSBuiltin] Promise blob();
     [JSBuiltin] Promise formData();
index 8eb0ab3..9b05ed3 100644 (file)
@@ -60,6 +60,17 @@ function initializeFetchResponse(body, init)
     return this;
 }
 
+function bodyUsed()
+{
+   if (!(this instanceof @Response))
+        throw @makeGetterTypeError("Response", "bodyUsed");
+
+    if (this.@body)
+        return @isReadableStreamDisturbed(this.@body);
+
+    return @Response.prototype.@isDisturbed.@call(this);
+}
+
 function body()
 {
     if (!(this instanceof @Response))
@@ -83,7 +94,7 @@ function clone()
     if (!(this instanceof @Response))
         throw @makeThisTypeError("Response", "clone");
 
-    if (@Response.prototype.@isDisturbed.@call(this))
+    if (@Response.prototype.@isDisturbed.@call(this) || (this.@body && @isReadableStreamLocked(this.@body)))
         throw new @TypeError("Cannot clone a disturbed Response");
 
     var cloned = @Response.prototype.@cloneForJS.@call(this);