Convert Background Fetch' input to a WebServiceWorkerRequest vector

third_party/WebKit/Source/modules/background_fetch/BackgroundFetchManager.cpp

Index: third_party/WebKit/Source/modules/background_fetch/BackgroundFetchManager.cpp
diff --git a/third_party/WebKit/Source/modules/background_fetch/BackgroundFetchManager.cpp b/third_party/WebKit/Source/modules/background_fetch/BackgroundFetchManager.cpp
index d1018f8b240c4e73c7b5495d3bdc352066f93e24..66a39ede3d84ddd190dae948d6d6d87b4e0b27a1 100644
--- a/third_party/WebKit/Source/modules/background_fetch/BackgroundFetchManager.cpp
+++ b/third_party/WebKit/Source/modules/background_fetch/BackgroundFetchManager.cpp
@@ -7,15 +7,30 @@
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/V8ThrowException.h"
+#include "bindings/modules/v8/RequestOrUSVString.h"
+#include "bindings/modules/v8/RequestOrUSVStringOrRequestOrUSVStringSequence.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
 #include "modules/background_fetch/BackgroundFetchBridge.h"
 #include "modules/background_fetch/BackgroundFetchOptions.h"
 #include "modules/background_fetch/BackgroundFetchRegistration.h"
+#include "modules/fetch/Request.h"
 #include "modules/serviceworkers/ServiceWorkerRegistration.h"
+#include "public/platform/modules/serviceworker/WebServiceWorkerRequest.h"
 
 namespace blink {
 
+namespace {
+
+// Message for the TypeError thrown when an empty request sequence is seen.
+const char kEmptyRequestSequenceErrorMessage[] =
+    "At least one request must be given.";
+
+// Message for the TypeError thrown when a null request is seen.
+const char kNullRequestErrorMessage[] = "Requests must not be null.";
+
+}  // namespace
+
 BackgroundFetchManager::BackgroundFetchManager(
     ServiceWorkerRegistration* registration)
     : m_registration(registration) {
@@ -27,7 +42,8 @@ ScriptPromise BackgroundFetchManager::fetch(
     ScriptState* scriptState,
     const String& tag,
     const RequestOrUSVStringOrRequestOrUSVStringSequence& requests,
-    const BackgroundFetchOptions& options) {
+    const BackgroundFetchOptions& options,
+    ExceptionState& exceptionState) {
   if (!m_registration->active()) {
     return ScriptPromise::reject(
         scriptState,
@@ -36,12 +52,15 @@ ScriptPromise BackgroundFetchManager::fetch(
                                           "the ServiceWorkerRegistration."));
   }
 
+  Vector<WebServiceWorkerRequest> webRequests =
+      createWebRequestVector(scriptState, requests, exceptionState);
+  if (exceptionState.hadException())
+    return ScriptPromise();
+
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
 
-  // TODO(peter): Include the |requests| in the Mojo call.
-
-  m_bridge->fetch(tag, options,
+  m_bridge->fetch(tag, std::move(webRequests), options,
                   WTF::bind(&BackgroundFetchManager::didFetch,
                             wrapPersistent(this), wrapPersistent(resolver)));
 
@@ -91,6 +110,65 @@ ScriptPromise BackgroundFetchManager::get(ScriptState* scriptState,
   return promise;
 }
 
+// static
+Vector<WebServiceWorkerRequest> BackgroundFetchManager::createWebRequestVector(
+    ScriptState* scriptState,
+    const RequestOrUSVStringOrRequestOrUSVStringSequence& requests,
+    ExceptionState& exceptionState) {
+  Vector<WebServiceWorkerRequest> webRequests;
+
+  if (requests.isRequestOrUSVStringSequence()) {
+    HeapVector<RequestOrUSVString> requestVector =
+        requests.getAsRequestOrUSVStringSequence();
+
+    // Throw a TypeError when the developer has passed an empty sequence.
+    if (!requestVector.size()) {
+      exceptionState.throwTypeError(kEmptyRequestSequenceErrorMessage);
+      return Vector<WebServiceWorkerRequest>();
+    }
+
+    webRequests.resize(requestVector.size());
+
+    for (size_t i = 0; i < requestVector.size(); ++i) {
+      const RequestOrUSVString& requestOrUrl = requestVector[i];
+
+      Request* request = nullptr;
+      if (requestOrUrl.isRequest()) {
+        request = requestOrUrl.getAsRequest();
+      } else if (requestOrUrl.isUSVString()) {
+        request = Request::create(scriptState, requestOrUrl.getAsUSVString(),
+                                  exceptionState);
+        if (exceptionState.hadException())
+          return Vector<WebServiceWorkerRequest>();

[haraken, 2017/03/24 01:29:38]

Don't we need to throw a type error?

[Peter Beverloo, 2017/03/24 14:41:20]

No, because an exception already was thrown (through the `exceptionState`) when
creating the Request. That'll contain much more detailed information than
anything a TypeError would have.

+      } else {
+        exceptionState.throwTypeError(kNullRequestErrorMessage);
+        return Vector<WebServiceWorkerRequest>();
+      }
+
+      DCHECK(request);
+      request->populateWebServiceWorkerRequest(webRequests[i]);
+    }
+  } else if (requests.isRequest()) {
+    DCHECK(requests.getAsRequest());
+    webRequests.resize(1);
+    requests.getAsRequest()->populateWebServiceWorkerRequest(webRequests[0]);

[horo, 2017/03/24 02:01:51]

populateWebServiceWorkerRequest() doesn't support the body of the request.
This method is used for CacheStorage API which doesn't support the request body.

Don't we need to support the request body in Background Fetch?

[Peter Beverloo, 2017/03/24 14:41:20]

Right now we're focusing on `URL` to get end-to-end working, after which we'll
ramp up on the other additions necessary to fully support the Request options.

The reason for this is that our fetches will be routed through the downloads
system, which is rather different from making an immediate request in the
browser process. It's not yet clear which of the Request options are completely
supported, because much of the plumbing seems to be unused. Some options will
also have UI/UX impact that we'll have to assess.

In the ramp-up we'll fix "bugs" like this and ensure proper test coverage for
them.

+  } else if (requests.isUSVString()) {
+    Request* request =
+        Request::create(scriptState, requests.getAsUSVString(), exceptionState);
+    if (exceptionState.hadException())
+      return Vector<WebServiceWorkerRequest>();

[haraken, 2017/03/24 01:29:38]

Ditto.

[Peter Beverloo, 2017/03/24 14:41:20]

Acknowledged.

+
+    DCHECK(request);
+    webRequests.resize(1);
+    request->populateWebServiceWorkerRequest(webRequests[0]);
+  } else {
+    exceptionState.throwTypeError(kNullRequestErrorMessage);
+    return Vector<WebServiceWorkerRequest>();
+  }
+
+  return webRequests;
+}
+
 void BackgroundFetchManager::didGetRegistration(
     ScriptPromiseResolver* resolver,
     mojom::blink::BackgroundFetchError error,