Bug 1073231 Implement Request and Response Clone() methods. r=nsm r=baku

dom/bindings/Errors.msg

@@ -62,7 +62,7 @@ MSG_DEF(MSG_INVALID_HEADER_SEQUENCE, 0, JSEXN_TYPEERR, "Headers require name/val
 MSG_DEF(MSG_PERMISSION_DENIED_TO_PASS_ARG, 1, JSEXN_TYPEERR, "Permission denied to pass cross-origin object as {0}.")
 MSG_DEF(MSG_MISSING_REQUIRED_DICTIONARY_MEMBER, 1, JSEXN_TYPEERR, "Missing required {0}.")
 MSG_DEF(MSG_INVALID_REQUEST_METHOD, 1, JSEXN_TYPEERR, "Invalid request method {0}.")
-MSG_DEF(MSG_REQUEST_BODY_CONSUMED_ERROR, 0, JSEXN_TYPEERR, "Request body has already been consumed.")
+MSG_DEF(MSG_FETCH_BODY_CONSUMED_ERROR, 0, JSEXN_TYPEERR, "Body has already been consumed.")
 MSG_DEF(MSG_RESPONSE_INVALID_STATUSTEXT_ERROR, 0, JSEXN_TYPEERR, "Response statusText may not contain newline or carriage return.")
 MSG_DEF(MSG_FETCH_FAILED, 0, JSEXN_TYPEERR, "NetworkError when attempting to fetch resource.")
 MSG_DEF(MSG_NO_BODY_ALLOWED_FOR_GET_AND_HEAD, 0, JSEXN_TYPEERR, "HEAD or GET Request cannot have a body.")

dom/fetch/Fetch.cpp

@@ -1164,7 +1164,7 @@ FetchBody<Derived>::ConsumeBody(ConsumeType aType, ErrorResult& aRv)
 {
   mConsumeType = aType;
   if (BodyUsed()) {
-    aRv.ThrowTypeError(MSG_REQUEST_BODY_CONSUMED_ERROR);
+    aRv.ThrowTypeError(MSG_FETCH_BODY_CONSUMED_ERROR);
     return nullptr;
   }
 

dom/fetch/Fetch.h

@@ -99,7 +99,7 @@ template <class Derived>
 class FetchBody {
 public:
   bool
-  BodyUsed() { return mBodyUsed; }
+  BodyUsed() const { return mBodyUsed; }
 
   already_AddRefed<Promise>
   ArrayBuffer(ErrorResult& aRv)

dom/fetch/InternalRequest.cpp

@@ -7,6 +7,7 @@
 
 #include "nsIContentPolicy.h"
 #include "nsIDocument.h"
+#include "nsStreamUtils.h"
 
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/ScriptSettings.h"
@@ -43,6 +44,53 @@ InternalRequest::GetRequestConstructorCopy(nsIGlobalObject* aGlobal, ErrorResult
   return copy.forget();
 }
 
+already_AddRefed<InternalRequest>
+InternalRequest::Clone()
+{
+  nsRefPtr<InternalRequest> clone = new InternalRequest(*this);
+
+  if (!mBodyStream) {
+    return clone.forget();
+  }
+
+  nsCOMPtr<nsIInputStream> clonedBody;
+  nsCOMPtr<nsIInputStream> replacementBody;
+
+  nsresult rv = NS_CloneInputStream(mBodyStream, getter_AddRefs(clonedBody),
+                                    getter_AddRefs(replacementBody));
+  if (NS_WARN_IF(NS_FAILED(rv))) { return nullptr; }
+
+  clone->mBodyStream.swap(clonedBody);
+  if (replacementBody) {
+    mBodyStream.swap(replacementBody);
+  }
+
+  return clone.forget();
+}
+
+InternalRequest::InternalRequest(const InternalRequest& aOther)
+  : mMethod(aOther.mMethod)
+  , mURL(aOther.mURL)
+  , mHeaders(new InternalHeaders(*aOther.mHeaders))
+  , mContentPolicyType(aOther.mContentPolicyType)
+  , mReferrer(aOther.mReferrer)
+  , mMode(aOther.mMode)
+  , mCredentialsMode(aOther.mCredentialsMode)
+  , mResponseTainting(aOther.mResponseTainting)
+  , mCacheMode(aOther.mCacheMode)
+  , mAuthenticationFlag(aOther.mAuthenticationFlag)
+  , mForceOriginHeader(aOther.mForceOriginHeader)
+  , mPreserveContentCodings(aOther.mPreserveContentCodings)
+  , mSameOriginDataURL(aOther.mSameOriginDataURL)
+  , mSandboxedStorageAreaURLs(aOther.mSandboxedStorageAreaURLs)
+  , mSkipServiceWorker(aOther.mSkipServiceWorker)
+  , mSynchronous(aOther.mSynchronous)
+  , mUnsafeRequest(aOther.mUnsafeRequest)
+  , mUseURLCredentials(aOther.mUseURLCredentials)
+{
+  // NOTE: does not copy body stream... use the fallible Clone() for that
+}
+
 InternalRequest::~InternalRequest()
 {
 }

dom/fetch/InternalRequest.h

@@ -67,28 +67,7 @@ public:
   {
   }
 
-  explicit InternalRequest(const InternalRequest& aOther)
+  already_AddRefed<InternalRequest> Clone();
-    : mMethod(aOther.mMethod)
-    , mURL(aOther.mURL)
-    , mHeaders(aOther.mHeaders)
-    , mBodyStream(aOther.mBodyStream)
-    , mContentPolicyType(aOther.mContentPolicyType)
-    , mReferrer(aOther.mReferrer)
-    , mMode(aOther.mMode)
-    , mCredentialsMode(aOther.mCredentialsMode)
-    , mResponseTainting(aOther.mResponseTainting)
-    , mCacheMode(aOther.mCacheMode)
-    , mAuthenticationFlag(aOther.mAuthenticationFlag)
-    , mForceOriginHeader(aOther.mForceOriginHeader)
-    , mPreserveContentCodings(aOther.mPreserveContentCodings)
-    , mSameOriginDataURL(aOther.mSameOriginDataURL)
-    , mSandboxedStorageAreaURLs(aOther.mSandboxedStorageAreaURLs)
-    , mSkipServiceWorker(aOther.mSkipServiceWorker)
-    , mSynchronous(aOther.mSynchronous)
-    , mUnsafeRequest(aOther.mUnsafeRequest)
-    , mUseURLCredentials(aOther.mUseURLCredentials)
-  {
-  }
 
   void
   GetMethod(nsCString& aMethod) const
@@ -293,6 +272,9 @@ public:
   GetRequestConstructorCopy(nsIGlobalObject* aGlobal, ErrorResult& aRv) const;
 
 private:
+  // Does not copy mBodyStream.  Use fallible Clone() for complete copy.
+  explicit InternalRequest(const InternalRequest& aOther);
+
   ~InternalRequest();
 
   nsCString mMethod;

dom/fetch/InternalResponse.cpp

@@ -8,6 +8,7 @@
 #include "nsIDOMFile.h"
 
 #include "mozilla/dom/InternalHeaders.h"
+#include "nsStreamUtils.h"
 
 namespace mozilla {
 namespace dom {
@@ -22,7 +23,7 @@ InternalResponse::InternalResponse(uint16_t aStatus, const nsACString& aStatusTe
 }
 
 // Headers are not copied since BasicResponse and CORSResponse both need custom
-// header handling.
+// header handling.  Body is not copied as it cannot be shared directly.
 InternalResponse::InternalResponse(const InternalResponse& aOther)
   : mType(aOther.mType)
   , mTerminationReason(aOther.mTerminationReason)
@@ -30,11 +31,35 @@ InternalResponse::InternalResponse(const InternalResponse& aOther)
   , mFinalURL(aOther.mFinalURL)
   , mStatus(aOther.mStatus)
   , mStatusText(aOther.mStatusText)
-  , mBody(aOther.mBody)
   , mContentType(aOther.mContentType)
 {
 }
 
+already_AddRefed<InternalResponse>
+InternalResponse::Clone()
+{
+  nsRefPtr<InternalResponse> clone = new InternalResponse(*this);
+  clone->mHeaders = new InternalHeaders(*mHeaders);
+
+  if (!mBody) {
+    return clone.forget();
+  }
+
+  nsCOMPtr<nsIInputStream> clonedBody;
+  nsCOMPtr<nsIInputStream> replacementBody;
+
+  nsresult rv = NS_CloneInputStream(mBody, getter_AddRefs(clonedBody),
+                                    getter_AddRefs(replacementBody));
+  if (NS_WARN_IF(NS_FAILED(rv))) { return nullptr; }
+
+  clone->mBody.swap(clonedBody);
+  if (replacementBody) {
+    mBody.swap(replacementBody);
+  }
+
+  return clone.forget();
+}
+
 // static
 already_AddRefed<InternalResponse>
 InternalResponse::BasicResponse(InternalResponse* aInner)
@@ -43,6 +68,7 @@ InternalResponse::BasicResponse(InternalResponse* aInner)
   nsRefPtr<InternalResponse> basic = new InternalResponse(*aInner);
   basic->mType = ResponseType::Basic;
   basic->mHeaders = InternalHeaders::BasicHeaders(aInner->mHeaders);
+  basic->mBody.swap(aInner->mBody);
   return basic.forget();
 }
 
@@ -54,6 +80,7 @@ InternalResponse::CORSResponse(InternalResponse* aInner)
   nsRefPtr<InternalResponse> cors = new InternalResponse(*aInner);
   cors->mType = ResponseType::Cors;
   cors->mHeaders = InternalHeaders::CORSHeaders(aInner->mHeaders);
+  cors->mBody.swap(aInner->mBody);
   return cors.forget();
 }
 

dom/fetch/InternalResponse.h

@@ -25,6 +25,8 @@ public:
 
   InternalResponse(uint16_t aStatus, const nsACString& aStatusText);
 
+  already_AddRefed<InternalResponse> Clone();
+
   static already_AddRefed<InternalResponse>
   NetworkError()
   {
@@ -125,8 +127,8 @@ private:
   ~InternalResponse()
   { }
 
-  // Used to create filtered responses.
+  // Used to create filtered and cloned responses.
-  // Does not copy headers.
+  // Does not copy headers or body stream.
   explicit InternalResponse(const InternalResponse& aOther);
 
   ResponseType mType;

dom/fetch/Request.cpp

@@ -63,7 +63,7 @@ Request::Constructor(const GlobalObject& aGlobal,
     inputReq->GetBody(getter_AddRefs(body));
     if (body) {
       if (inputReq->BodyUsed()) {
-        aRv.ThrowTypeError(MSG_REQUEST_BODY_CONSUMED_ERROR);
+        aRv.ThrowTypeError(MSG_FETCH_BODY_CONSUMED_ERROR);
         return nullptr;
       } else {
         inputReq->SetBodyUsed();
@@ -255,12 +255,20 @@ Request::Constructor(const GlobalObject& aGlobal,
 }
 
 already_AddRefed<Request>
-Request::Clone() const
+Request::Clone(ErrorResult& aRv) const
 {
-  // FIXME(nsm): Bug 1073231. This is incorrect, but the clone method isn't
+  if (BodyUsed()) {
-  // well defined yet.
+    aRv.ThrowTypeError(MSG_FETCH_BODY_CONSUMED_ERROR);
-  nsRefPtr<Request> request = new Request(mOwner,
+    return nullptr;
-                                          new InternalRequest(*mRequest));
+  }
+
+  nsRefPtr<InternalRequest> ir = mRequest->Clone();
+  if (!ir) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return nullptr;
+  }
+
+  nsRefPtr<Request> request = new Request(mOwner, ir);
   return request.forget();
 }
 

dom/fetch/Request.h

@@ -115,7 +115,7 @@ public:
   }
 
   already_AddRefed<Request>
-  Clone() const;
+  Clone(ErrorResult& aRv) const;
 
   already_AddRefed<InternalRequest>
   GetInternalRequest();

dom/fetch/Response.cpp

@@ -190,19 +190,23 @@ Response::Constructor(const GlobalObject& aGlobal,
   return r.forget();
 }
 
-// FIXME(nsm): Bug 1073231: This is currently unspecced!
 already_AddRefed<Response>
-Response::Clone()
+Response::Clone(ErrorResult& aRv) const
 {
-  nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(mOwner);
+  if (BodyUsed()) {
-  nsRefPtr<Response> response = new Response(global, mInternalResponse);
+    aRv.ThrowTypeError(MSG_FETCH_BODY_CONSUMED_ERROR);
+    return nullptr;
+  }
+
+  nsRefPtr<InternalResponse> ir = mInternalResponse->Clone();
+  nsRefPtr<Response> response = new Response(mOwner, ir);
   return response.forget();
 }
 
 void
 Response::SetBody(nsIInputStream* aBody)
 {
-  // FIXME(nsm): Do we flip bodyUsed here?
+  MOZ_ASSERT(!BodyUsed());
   mInternalResponse->SetBody(aBody);
 }
 

dom/fetch/Response.h

@@ -112,7 +112,7 @@ public:
   }
 
   already_AddRefed<Response>
-  Clone();
+  Clone(ErrorResult& aRv) const;
 
   void
   SetBody(nsIInputStream* aBody);

dom/webidl/Request.webidl

@@ -23,7 +23,8 @@ interface Request {
   readonly attribute RequestCredentials credentials;
   readonly attribute RequestCache cache;
 
-  [NewObject] Request clone();
+  [Throws,
+   NewObject] Request clone();
 
   // Bug 1124638 - Allow chrome callers to set the context.
   [ChromeOnly]

dom/webidl/Response.webidl

@@ -25,7 +25,8 @@ interface Response {
   readonly attribute ByteString statusText;
   [SameObject] readonly attribute Headers headers;
 
-  [NewObject] Response clone();
+  [Throws,
+   NewObject] Response clone();
 };
 Response implements Body;
 

dom/workers/test/fetch/worker_test_request.js

@@ -27,21 +27,70 @@ function testDefaultCtor() {
 }
 
 function testClone() {
-  var req = (new Request("./cloned_request.txt", {
+  var orig = new Request("./cloned_request.txt", {
               method: 'POST',
               headers: { "Content-Length": 5 },
               body: "Sample body",
               mode: "same-origin",
               credentials: "same-origin",
-            })).clone();
+            });
-  ok(req.method === "POST", "Request method is POST");
+  var clone = orig.clone();
-  ok(req.headers instanceof Headers, "Request should have non-null Headers object");
+  ok(clone.method === "POST", "Request method is POST");
-  is(req.headers.get('content-length'), "5", "Request content-length should be 5.");
+  ok(clone.headers instanceof Headers, "Request should have non-null Headers object");
-  ok(req.url === (new URL("./cloned_request.txt", self.location.href)).href,
+
+  is(clone.headers.get('content-length'), "5", "Response content-length should be 5.");
+  orig.headers.set('content-length', 6);
+  is(clone.headers.get('content-length'), "5", "Request content-length should be 5.");
+
+  ok(clone.url === (new URL("./cloned_request.txt", self.location.href)).href,
        "URL should be resolved with entry settings object's API base URL");
-  ok(req.referrer === "about:client", "Default referrer is `client` which serializes to about:client.");
+  ok(clone.referrer === "about:client", "Default referrer is `client` which serializes to about:client.");
-  ok(req.mode === "same-origin", "Request mode is same-origin");
+  ok(clone.mode === "same-origin", "Request mode is same-origin");
-  ok(req.credentials === "same-origin", "Default credentials is same-origin");
+  ok(clone.credentials === "same-origin", "Default credentials is same-origin");
+
+  ok(!orig.bodyUsed, "Original body is not consumed.");
+  ok(!clone.bodyUsed, "Clone body is not consumed.");
+
+  var origBody = null;
+  var clone2 = null;
+  return orig.text().then(function (body) {
+    origBody = body;
+    is(origBody, "Sample body", "Original body string matches");
+    ok(orig.bodyUsed, "Original body is consumed.");
+    ok(!clone.bodyUsed, "Clone body is not consumed.");
+
+    try {
+      orig.clone()
+      ok(false, "Cannot clone Request whose body is already consumed");
+    } catch (e) {
+      is(e.name, "TypeError", "clone() of consumed body should throw TypeError");
+    }
+
+    clone2 = clone.clone();
+    return clone.text();
+  }).then(function (body) {
+    is(body, origBody, "Clone body matches original body.");
+    ok(clone.bodyUsed, "Clone body is consumed.");
+
+    try {
+      clone.clone()
+      ok(false, "Cannot clone Request whose body is already consumed");
+    } catch (e) {
+      is(e.name, "TypeError", "clone() of consumed body should throw TypeError");
+    }
+
+    return clone2.text();
+  }).then(function (body) {
+    is(body, origBody, "Clone body matches original body.");
+    ok(clone2.bodyUsed, "Clone body is consumed.");
+
+    try {
+      clone2.clone()
+      ok(false, "Cannot clone Request whose body is already consumed");
+    } catch (e) {
+      is(e.name, "TypeError", "clone() of consumed body should throw TypeError");
+    }
+  });
 }
 
 function testUsedRequest() {
@@ -266,7 +315,6 @@ onmessage = function() {
   var done = function() { postMessage({ type: 'finish' }) }
 
   testDefaultCtor();
-  testClone();
   testSimpleUrlParse();
   testUrlFragment();
   testMethod();
@@ -278,6 +326,7 @@ onmessage = function() {
     .then(testBodyUsed)
     .then(testBodyExtraction)
     .then(testUsedRequest)
+    .then(testClone())
     // Put more promise based tests here.
     .then(done)
     .catch(function(e) {

dom/workers/test/fetch/worker_test_response.js

@@ -18,15 +18,63 @@ function testDefaultCtor() {
 }
 
 function testClone() {
-  var res = (new Response("This is a body", {
+  var orig = new Response("This is a body", {
               status: 404,
               statusText: "Not Found",
               headers: { "Content-Length": 5 },
-            })).clone();
+            });
-  is(res.status, 404, "Response status is 404");
+  var clone = orig.clone();
-  is(res.statusText, "Not Found", "Response statusText is POST");
+  is(clone.status, 404, "Response status is 404");
-  ok(res.headers instanceof Headers, "Response should have non-null Headers object");
+  is(clone.statusText, "Not Found", "Response statusText is POST");
-  is(res.headers.get('content-length'), "5", "Response content-length should be 5.");
+  ok(clone.headers instanceof Headers, "Response should have non-null Headers object");
+
+  is(clone.headers.get('content-length'), "5", "Response content-length should be 5.");
+  orig.headers.set('content-length', 6);
+  is(clone.headers.get('content-length'), "5", "Response content-length should be 5.");
+
+  ok(!orig.bodyUsed, "Original body is not consumed.");
+  ok(!clone.bodyUsed, "Clone body is not consumed.");
+
+  var origBody = null;
+  var clone2 = null;
+  return orig.text().then(function (body) {
+    origBody = body;
+    is(origBody, "This is a body", "Original body string matches");
+    ok(orig.bodyUsed, "Original body is consumed.");
+    ok(!clone.bodyUsed, "Clone body is not consumed.");
+
+    try {
+      orig.clone()
+      ok(false, "Cannot clone Response whose body is already consumed");
+    } catch (e) {
+      is(e.name, "TypeError", "clone() of consumed body should throw TypeError");
+    }
+
+    clone2 = clone.clone();
+    return clone.text();
+  }).then(function (body) {
+    is(body, origBody, "Clone body matches original body.");
+    ok(clone.bodyUsed, "Clone body is consumed.");
+
+    try {
+      clone.clone()
+      ok(false, "Cannot clone Response whose body is already consumed");
+    } catch (e) {
+      is(e.name, "TypeError", "clone() of consumed body should throw TypeError");
+    }
+
+    return clone2.text();
+  }).then(function (body) {
+    is(body, origBody, "Clone body matches original body.");
+    ok(clone2.bodyUsed, "Clone body is consumed.");
+
+    try {
+      clone2.clone()
+      ok(false, "Cannot clone Response whose body is already consumed");
+    } catch (e) {
+      is(e.name, "TypeError", "clone() of consumed body should throw TypeError");
+    }
+  });
 }
 
 function testRedirect() {