[Background Fetch] Reject when requests have body.

With this CL,
1. Record the BackgroundFetch.HasRequestsWithBody UMA metric, which is
true if any requests in the background fetch contain a body, false otherwise.
2. Reject the fetch() promise, if any requests do contain a body.

Bug: 881344
Change-Id: I595800adefe1dcc2c8bb04bec2295cc2886d963d
Reviewed-on: https://chromium-review.googlesource.com/1210125
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590309}

third_party/WebKit/LayoutTests/external/wpt/background-fetch/fetch.https.window-expected.txt

@@ -5,5 +5,6 @@ PASS IDs must be unique among active Background Fetch registrations
 PASS Using Background Fetch to successfully fetch a single resource
 PASS Background Fetch that exceeds the quota throws a QuotaExceededError
 FAIL Fetches can have requests with duplicate URLs promise_test: Unhandled rejection with value: object "TypeError: Fetches with duplicate requests are not yet supported. Consider adding query params to make the requests unique. For updates check http://crbug.com/871174"
+FAIL Fetches can have requests with a body promise_test: Unhandled rejection with value: object "TypeError: Requests with a body are not yet supported. For updates check http://crbug.com/774054"
 Harness: the test ran to completion.
 

third_party/WebKit/LayoutTests/external/wpt/background-fetch/fetch.https.window.js

@@ -122,3 +122,26 @@ backgroundFetchTest(async (test, backgroundFetch) => {
   }
 
 }, 'Fetches can have requests with duplicate URLs');
+
+backgroundFetchTest(async (test, backgroundFetch) => {
+  const request =
+      new Request('resources/feature-name.txt',
+                  {method: 'POST', body: 'TestBody'});
+
+  const registration = await backgroundFetch.fetch('my-id', request);
+
+  const {type, eventRegistration, results} = await getMessageFromServiceWorker();
+  assert_equals('backgroundfetchsuccess', type);
+  assert_equals(results.length, 1);
+
+  assert_equals(eventRegistration.id, registration.id);
+  assert_equals(eventRegistration.state, 'success');
+  assert_equals(eventRegistration.failureReason, '');
+
+  for (const result of results) {
+    assert_true(result.url.includes('resources/feature-name.txt'));
+    assert_equals(result.status, 200);
+    assert_equals(result.text, 'Background Fetch');
+  }
+
+}, 'Fetches can have requests with a body');

third_party/blink/renderer/modules/background_fetch/background_fetch_manager.cc

@@ -170,11 +170,25 @@ ScriptPromise BackgroundFetchManager::fetch(
                                           "the ServiceWorkerRegistration."));
   }
 
-  Vector<WebServiceWorkerRequest> web_requests =
-      CreateWebRequestVector(script_state, requests, exception_state);
+  bool has_requests_with_body;
+  Vector<WebServiceWorkerRequest> web_requests = CreateWebRequestVector(
+      script_state, requests, exception_state, &has_requests_with_body);
   if (exception_state.HadException())
     return ScriptPromise();
 
+  // Record whether any requests had a body. If there were, reject the promise.
+  UMA_HISTOGRAM_BOOLEAN("BackgroundFetch.HasRequestsWithBody",
+                        has_requests_with_body);
+
+  // TODO(crbug.com/789854): Stop bailing here once we support uploads.
+  if (has_requests_with_body) {
+    return ScriptPromise::Reject(
+        script_state, V8ThrowException::CreateTypeError(
+                          script_state->GetIsolate(),
+                          "Requests with a body are not yet supported. "
+                          "For updates check http://crbug.com/774054"));
+  }
+
   ExecutionContext* execution_context = ExecutionContext::From(script_state);
 
   // A HashSet to find whether there are any duplicate requests within the
@@ -383,8 +397,12 @@ ScriptPromise BackgroundFetchManager::get(ScriptState* script_state,
 Vector<WebServiceWorkerRequest> BackgroundFetchManager::CreateWebRequestVector(
     ScriptState* script_state,
     const RequestOrUSVStringOrRequestOrUSVStringSequence& requests,
-    ExceptionState& exception_state) {
+    ExceptionState& exception_state,
+    bool* has_requests_with_body) {
+  DCHECK(has_requests_with_body);
+
   Vector<WebServiceWorkerRequest> web_requests;
+  *has_requests_with_body = false;
 
   if (requests.IsRequestOrUSVStringSequence()) {
     HeapVector<RequestOrUSVString> request_vector =
@@ -415,16 +433,21 @@ Vector<WebServiceWorkerRequest> BackgroundFetchManager::CreateWebRequestVector(
       }
 
       DCHECK(request);
+      *has_requests_with_body |= request->HasBody();
       // TODO(crbug.com/774054): Set blob data handle when adding support for
       // requests with body.
       request->PopulateWebServiceWorkerRequest(web_requests[i]);
     }
   } else if (requests.IsRequest()) {
-    DCHECK(requests.GetAsRequest());
+    auto* request = requests.GetAsRequest();
+    DCHECK(request);
+
     // TODO(crbug.com/774054): Set blob data handle when adding support for
     // requests with body.
+
+    *has_requests_with_body = request->HasBody();
     web_requests.resize(1);
-    requests.GetAsRequest()->PopulateWebServiceWorkerRequest(web_requests[0]);
+    request->PopulateWebServiceWorkerRequest(web_requests[0]);
   } else if (requests.IsUSVString()) {
     Request* request = Request::Create(script_state, requests.GetAsUSVString(),
                                        exception_state);
@@ -432,6 +455,7 @@ Vector<WebServiceWorkerRequest> BackgroundFetchManager::CreateWebRequestVector(
       return Vector<WebServiceWorkerRequest>();
 
     DCHECK(request);
+    *has_requests_with_body = request->HasBody();
     web_requests.resize(1);
     request->PopulateWebServiceWorkerRequest(web_requests[0]);
   } else {

third_party/blink/renderer/modules/background_fetch/background_fetch_manager.h

@@ -67,10 +67,12 @@ class MODULES_EXPORT BackgroundFetchManager final
 
   // Creates a vector of WebServiceWorkerRequest objects for the given set of
   // |requests|, which can be either Request objects or URL strings.
+  // |has_requests_with_body| will be set if any of the |requests| has a body.
   static Vector<WebServiceWorkerRequest> CreateWebRequestVector(
       ScriptState* script_state,
       const RequestOrUSVStringOrRequestOrUSVStringSequence& requests,
-      ExceptionState& exception_state);
+      ExceptionState& exception_state,
+      bool* has_requests_with_body);
 
   void DidLoadIcons(const String& id,
                     Vector<WebServiceWorkerRequest> web_requests,

third_party/blink/renderer/modules/background_fetch/background_fetch_manager_test.cc

@@ -25,8 +25,10 @@ class BackgroundFetchManagerTest : public testing::Test {
   Vector<WebServiceWorkerRequest> CreateWebRequestVector(
       V8TestingScope& scope,
       const RequestOrUSVStringOrRequestOrUSVStringSequence& requests) {
+    bool has_requests_with_body;
     return BackgroundFetchManager::CreateWebRequestVector(
-        scope.GetScriptState(), requests, scope.GetExceptionState());
+        scope.GetScriptState(), requests, scope.GetExceptionState(),
+        &has_requests_with_body);
   }
 };
 

tools/metrics/histograms/histograms.xml

@@ -7758,6 +7758,16 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="BackgroundFetch.HasRequestsWithBody" enum="Boolean">
+  <owner>nator@chromium.org</owner>
+  <owner>rayankans@chromium.org</owner>
+  <owner>peter@chromium.org</owner>
+  <summary>
+    Records whether the background fetch contains any requests with a body.
+    Called before the fetch is started.
+  </summary>
+</histogram>
+
 <histogram name="BackgroundFetch.IncompleteFetchesOnStartup">
   <owner>rayankans@chromium.org</owner>
   <summary>