[fetch] Request constructor should throw an error if ReadableStream is locked.

This CL make Request constructor follow
https://fetch.spec.whatwg.org/#dom-request:
"38. If inputBody is body and input is disturbed or locked, then throw a
TypeError."

This CL also refactors the constructor around body extracting following
the last spec.

Bug: 1105704
Change-Id: Ie2ba890828e2091dbc0c55fa7120fb7ec37ebc13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2309369Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792488}

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

@@ -220,24 +220,6 @@ Request* Request::CreateRequestWithRequestOrString(
     const String& input_string,
     const RequestInit* init,
     ExceptionState& exception_state) {
-  // Setup RequestInit's body first
-  // - "If |input| is a Request object and it is disturbed, throw a
-  //   TypeError."
-  if (input_request && input_request->IsBodyUsed()) {
-    exception_state.ThrowTypeError(
-        "Cannot construct a Request with a Request object that has already "
-        "been used.");
-    return nullptr;
-  }
-
-  // - "Let |temporaryBody| be |input|'s request's body if |input| is a
-  //   Request object, and null otherwise."
-  BodyStreamBuffer* temporary_body =
-      input_request ? input_request->BodyBuffer() : nullptr;
-
-  // "Let |request| be |input|'s request, if |input| is a Request object,
-  // and a new request otherwise."
-
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
   scoped_refptr<const SecurityOrigin> origin =
       execution_context->GetSecurityOrigin();
@@ -621,11 +603,16 @@ Request* Request::CreateRequestWithRequestOrString(
   if (exception_state.HadException())
     return nullptr;
 
-  // "If either |init|'s body member is present or |temporaryBody| is
+  // "Let |inputBody| be |input|'s request's body if |input| is a
+  //   Request object, and null otherwise."
+  BodyStreamBuffer* input_body =
+      input_request ? input_request->BodyBuffer() : nullptr;
+
+  // "If either |init|["body"] exists and is non-null or |inputBody| is
   // non-null, and |request|'s method is `GET` or `HEAD`, throw a TypeError.
   v8::Local<v8::Value> init_body =
       init->hasBody() ? init->body().V8Value() : v8::Local<v8::Value>();
-  if ((!init_body.IsEmpty() && !init_body->IsNull()) || temporary_body) {
+  if ((!init_body.IsEmpty() && !init_body->IsNull()) || input_body) {
     if (request->Method() == http_names::kGET ||
         request->Method() == http_names::kHEAD) {
       exception_state.ThrowTypeError(
@@ -634,7 +621,10 @@ Request* Request::CreateRequestWithRequestOrString(
     }
   }
 
-  // "If |init|’s body member is present and is non-null, then:"
+  // "Let |body| be |inputBody|."
+  BodyStreamBuffer* body = input_body;
+
+  // "If |init|["body"] exists and is non-null, then:"
   if (!init_body.IsEmpty() && !init_body->IsNull()) {
     // - If |init|["keepalive"] exists and is true, then set |body| and
     //   |Content-Type| to the result of extracting |init|["body"], with the
@@ -648,17 +638,13 @@ Request* Request::CreateRequestWithRequestOrString(
       return nullptr;
     }
 
-    // Perform the following steps:
-    // - "Let |stream| and |Content-Type| be the result of extracting
-    //   |init|'s body member."
-    // - "Set |temporaryBody| to |stream|.
-    // - "If |Content-Type| is non-null and |r|'s request's header list
-    //   contains no header named `Content-Type`, append
-    //   `Content-Type`/|Content-Type| to |r|'s Headers object. Rethrow any
-    //   exception."
+    // "Otherwise, set |body| and |Content-Type| to the result of extracting
+    //  init["body"]."
     String content_type;
-    temporary_body =
-        ExtractBody(script_state, exception_state, init_body, content_type);
+    body = ExtractBody(script_state, exception_state, init_body, content_type);
+    // "If |Content-Type| is non-null and |this|'s header's header list
+    //  does not contain `Content-Type`, then append
+    //   `Content-Type`/|Content-Type| to |this|'s headers object.
     if (!content_type.IsEmpty() &&
         !r->getHeaders()->has(http_names::kContentType, exception_state)) {
       r->getHeaders()->append(http_names::kContentType, content_type,
@@ -668,10 +654,10 @@ Request* Request::CreateRequestWithRequestOrString(
       return nullptr;
   }
 
-  // If body is non-null and body’s source is null, then:
-  if (temporary_body && temporary_body->IsMadeFromReadableStream()) {
-    // If r’s request’s mode is neither "same-origin" nor "cors", then throw a
-    // TypeError.
+  // "If |body| is non-null and |body|’s source is null, then:"
+  if (body && body->IsMadeFromReadableStream()) {
+    // "If |this|’s request’s mode is neither "same-origin" nor "cors", then
+    // throw a TypeError."
     if (request->Mode() != network::mojom::RequestMode::kSameOrigin &&
         request->Mode() != network::mojom::RequestMode::kCors) {
       exception_state.ThrowTypeError(
@@ -679,13 +665,23 @@ Request* Request::CreateRequestWithRequestOrString(
           "\"same-origin\" or \"cors\"");
       return nullptr;
     }
-    // Set r’s request’s use-CORS-preflight flag.
+    // "Set this’s request’s use-CORS-preflight flag."
     request->SetMode(network::mojom::RequestMode::kCorsWithForcedPreflight);
   }
 
-  // "Set |r|'s request's body to |temporaryBody|.
-  if (temporary_body)
-    r->request_->SetBuffer(temporary_body);
+  // "If |inputBody| is |body| and |input| is disturbed or locked, then throw a
+  // TypeError."
+  if (input_body == body && input_request &&
+      (input_request->IsBodyUsed() || input_request->IsBodyLocked())) {
+    exception_state.ThrowTypeError(
+        "Cannot construct a Request with a Request object that has already "
+        "been used.");
+    return nullptr;
+  }
+
+  // "Set |this|'s request's body to |body|.
+  if (body)
+    r->request_->SetBuffer(body);
 
   // "Set |r|'s MIME type to the result of extracting a MIME type from |r|'s
   // request's header list."

third_party/blink/web_tests/external/wpt/fetch/api/request/request-disturbed-expected.txt

@@ -3,7 +3,7 @@ PASS Request's body: initial state
 PASS Request without body cannot be disturbed
 PASS Check cloning a disturbed request
 PASS Check creating a new request from a disturbed request
-FAIL Check creating a new request with a new body from a disturbed request Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used.
+PASS Check creating a new request with a new body from a disturbed request
 FAIL Input request used for creating new request became disturbed assert_equals: body should not change expected object "[object ReadableStream]" but got object "[object ReadableStream]"
 FAIL Input request used for creating new request became disturbed even if body is not used assert_equals: body should not change expected object "[object ReadableStream]" but got object "[object ReadableStream]"
 PASS Check consuming a disturbed request

third_party/blink/web_tests/external/wpt/fetch/api/request/request-init-stream.any-expected.txt

-This is a testharness.js-based test.
-PASS Constructing a Request with a stream holds the original object.
-PASS Constructing a Request with a stream on which getReader() is called
-PASS Constructing a Request with a stream on which read() is called
-PASS Constructing a Request with a stream on which read() and releaseLock() are called
-FAIL Constructing a Request with a Request on which body.getReader() is called assert_throws_js: new Request() function "() => new Request(input, init)" did not throw
-FAIL Constructing a Request with a Request on which body.getReader().read() is called promise_test: Unhandled rejection with value: object "TypeError: Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used."
-FAIL Constructing a Request with a Request on which read() and releaseLock() are called promise_test: Unhandled rejection with value: object "TypeError: Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used."
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/fetch/api/request/request-init-stream.any.serviceworker-expected.txt

-This is a testharness.js-based test.
-PASS Constructing a Request with a stream holds the original object.
-PASS Constructing a Request with a stream on which getReader() is called
-PASS Constructing a Request with a stream on which read() is called
-PASS Constructing a Request with a stream on which read() and releaseLock() are called
-FAIL Constructing a Request with a Request on which body.getReader() is called assert_throws_js: new Request() function "() => new Request(input, init)" did not throw
-FAIL Constructing a Request with a Request on which body.getReader().read() is called promise_test: Unhandled rejection with value: object "TypeError: Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used."
-FAIL Constructing a Request with a Request on which read() and releaseLock() are called promise_test: Unhandled rejection with value: object "TypeError: Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used."
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/fetch/api/request/request-init-stream.any.sharedworker-expected.txt

-This is a testharness.js-based test.
-PASS Constructing a Request with a stream holds the original object.
-PASS Constructing a Request with a stream on which getReader() is called
-PASS Constructing a Request with a stream on which read() is called
-PASS Constructing a Request with a stream on which read() and releaseLock() are called
-FAIL Constructing a Request with a Request on which body.getReader() is called assert_throws_js: new Request() function "() => new Request(input, init)" did not throw
-FAIL Constructing a Request with a Request on which body.getReader().read() is called promise_test: Unhandled rejection with value: object "TypeError: Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used."
-FAIL Constructing a Request with a Request on which read() and releaseLock() are called promise_test: Unhandled rejection with value: object "TypeError: Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used."
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/fetch/api/request/request-init-stream.any.worker-expected.txt

-This is a testharness.js-based test.
-PASS Constructing a Request with a stream holds the original object.
-PASS Constructing a Request with a stream on which getReader() is called
-PASS Constructing a Request with a stream on which read() is called
-PASS Constructing a Request with a stream on which read() and releaseLock() are called
-FAIL Constructing a Request with a Request on which body.getReader() is called assert_throws_js: new Request() function "() => new Request(input, init)" did not throw
-FAIL Constructing a Request with a Request on which body.getReader().read() is called promise_test: Unhandled rejection with value: object "TypeError: Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used."
-FAIL Constructing a Request with a Request on which read() and releaseLock() are called promise_test: Unhandled rejection with value: object "TypeError: Failed to construct 'Request': Cannot construct a Request with a Request object that has already been used."
-Harness: the test ran to completion.
-