service worker: Refactoring in registerServiceWorker().

The function had a public helper function that didn't add much value. We
were duplicating checks like !provider_ and the flow of data was more
complicated.

It's hard to read the diff so the changes are:
* Removed RegisterServiceWorkerImpl.
* Removed duplicate if (!provider_) check.
* Removed |raw_script_url|, it's unneeded indirection.
* Unified the preparation of |script_url|, |pattern_url|, and
|update_via_cache| to just before they are used.

Change-Id: Icef9dd5b26f4009fbf69d62ad918066be2c0733c
Reviewed-on: https://chromium-review.googlesource.com/1098425
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566726}

third_party/blink/renderer/modules/serviceworkers/service_worker_container.cc

@@ -164,31 +164,40 @@ void ServiceWorkerContainer::Trace(blink::Visitor* visitor) {
   ContextLifecycleObserver::Trace(visitor);
 }
 
-void ServiceWorkerContainer::RegisterServiceWorkerImpl(
-    ExecutionContext* execution_context,
-    const KURL& raw_script_url,
-    const KURL& scope,
-    mojom::ServiceWorkerUpdateViaCache update_via_cache,
-    std::unique_ptr<RegistrationCallbacks> callbacks) {
+ScriptPromise ServiceWorkerContainer::registerServiceWorker(
+    ScriptState* script_state,
+    const String& url,
+    const RegistrationOptions& options) {
+  ScriptPromiseResolver* resolver = ScriptPromiseResolver::Create(script_state);
+  ScriptPromise promise = resolver->Promise();
+
   if (!provider_) {
-    callbacks->OnError(
-        WebServiceWorkerError(mojom::blink::ServiceWorkerErrorType::kState,
-                              "Failed to register a ServiceWorker: The "
-                              "document is in an invalid state."));
-    return;
+    resolver->Reject(DOMException::Create(DOMExceptionCode::kInvalidStateError,
+                                          "Failed to register a ServiceWorker: "
+                                          "The document is in an invalid "
+                                          "state."));
+    return promise;
   }
 
-  scoped_refptr<const SecurityOrigin> document_origin =
-      execution_context->GetSecurityOrigin();
+  ExecutionContext* execution_context = ExecutionContext::From(script_state);
+  // FIXME: May be null due to worker termination: http://crbug.com/413518.
+  if (!execution_context)
+    return ScriptPromise();
+
+  auto callbacks = std::make_unique<CallbackPromiseAdapter<
+      ServiceWorkerRegistration, ServiceWorkerErrorForUpdate>>(resolver);
+
   String error_message;
   // Restrict to secure origins:
   // https://w3c.github.io/webappsec-secure-contexts/#is-settings-object-contextually-secure
   if (!execution_context->IsSecureContext(error_message)) {
     callbacks->OnError(WebServiceWorkerError(
         mojom::blink::ServiceWorkerErrorType::kSecurity, error_message));
-    return;
+    return promise;
   }
 
+  scoped_refptr<const SecurityOrigin> document_origin =
+      execution_context->GetSecurityOrigin();
   KURL page_url = KURL(NullURL(), document_origin->ToString());
   if (!SchemeRegistry::ShouldTreatURLSchemeAsAllowingServiceWorkers(
           page_url.Protocol())) {
@@ -197,11 +206,12 @@ void ServiceWorkerContainer::RegisterServiceWorkerImpl(
         String("Failed to register a ServiceWorker: The URL protocol of the "
                "current origin ('" +
                document_origin->ToString() + "') is not supported.")));
-    return;
+    return promise;
   }
 
-  KURL script_url = raw_script_url;
+  KURL script_url = execution_context->CompleteURL(url);
   script_url.RemoveFragmentIdentifier();
+
   if (!document_origin->CanRequest(script_url)) {
     scoped_refptr<const SecurityOrigin> script_origin =
         SecurityOrigin::Create(script_url);
@@ -212,7 +222,7 @@ void ServiceWorkerContainer::RegisterServiceWorkerImpl(
                                      script_origin->ToString() +
                                      "') does not match the current origin ('" +
                                      document_origin->ToString() + "').")));
-    return;
+    return promise;
   }
   if (!SchemeRegistry::ShouldTreatURLSchemeAsAllowingServiceWorkers(
           script_url.Protocol())) {
@@ -221,10 +231,14 @@ void ServiceWorkerContainer::RegisterServiceWorkerImpl(
         String("Failed to register a ServiceWorker: The URL protocol of the "
                "script ('" +
                script_url.GetString() + "') is not supported.")));
-    return;
+    return promise;
   }
 
-  KURL pattern_url = scope;
+  KURL pattern_url;
+  if (options.scope().IsNull())
+    pattern_url = KURL(script_url, "./");
+  else
+    pattern_url = execution_context->CompleteURL(options.scope());
   pattern_url.RemoveFragmentIdentifier();
 
   if (!document_origin->CanRequest(pattern_url)) {
@@ -237,7 +251,7 @@ void ServiceWorkerContainer::RegisterServiceWorkerImpl(
                                      pattern_origin->ToString() +
                                      "') does not match the current origin ('" +
                                      document_origin->ToString() + "').")));
-    return;
+    return promise;
   }
   if (!SchemeRegistry::ShouldTreatURLSchemeAsAllowingServiceWorkers(
           pattern_url.Protocol())) {
@@ -246,7 +260,7 @@ void ServiceWorkerContainer::RegisterServiceWorkerImpl(
         String("Failed to register a ServiceWorker: The URL protocol of the "
                "scope ('" +
                pattern_url.GetString() + "') is not supported.")));
-    return;
+    return promise;
   }
 
   WebString web_error_message;
@@ -256,68 +270,31 @@ void ServiceWorkerContainer::RegisterServiceWorkerImpl(
         mojom::blink::ServiceWorkerErrorType::kType,
         WebString::FromUTF8("Failed to register a ServiceWorker: " +
                             web_error_message.Utf8())));
-    return;
+    return promise;
   }
 
   ContentSecurityPolicy* csp = execution_context->GetContentSecurityPolicy();
   if (csp) {
-    if (!(csp->AllowRequestWithoutIntegrity(
-              WebURLRequest::kRequestContextServiceWorker, script_url) &&
-          csp->AllowWorkerContextFromSource(
+    if (!csp->AllowRequestWithoutIntegrity(
+            WebURLRequest::kRequestContextServiceWorker, script_url) ||
+        !csp->AllowWorkerContextFromSource(
             script_url, ResourceRequest::RedirectStatus::kNoRedirect,
-              SecurityViolationReportingPolicy::kReport))) {
+            SecurityViolationReportingPolicy::kReport)) {
       callbacks->OnError(WebServiceWorkerError(
           mojom::blink::ServiceWorkerErrorType::kSecurity,
           String(
               "Failed to register a ServiceWorker: The provided scriptURL ('" +
               script_url.GetString() +
               "') violates the Content Security Policy.")));
-      return;
-    }
-  }
-
-  provider_->RegisterServiceWorker(pattern_url, script_url, update_via_cache,
-                                   std::move(callbacks));
-}
-
-ScriptPromise ServiceWorkerContainer::registerServiceWorker(
-    ScriptState* script_state,
-    const String& url,
-    const RegistrationOptions& options) {
-  ScriptPromiseResolver* resolver = ScriptPromiseResolver::Create(script_state);
-  ScriptPromise promise = resolver->Promise();
-
-  if (!provider_) {
-    resolver->Reject(DOMException::Create(DOMExceptionCode::kInvalidStateError,
-                                          "Failed to register a ServiceWorker: "
-                                          "The document is in an invalid "
-                                          "state."));
       return promise;
     }
-
-  ExecutionContext* execution_context = ExecutionContext::From(script_state);
-  // FIXME: May be null due to worker termination: http://crbug.com/413518.
-  if (!execution_context)
-    return ScriptPromise();
-
-  KURL script_url = execution_context->CompleteURL(url);
-  script_url.RemoveFragmentIdentifier();
-
-  KURL pattern_url;
-  if (options.scope().IsNull())
-    pattern_url = KURL(script_url, "./");
-  else
-    pattern_url = execution_context->CompleteURL(options.scope());
+  }
 
   mojom::ServiceWorkerUpdateViaCache update_via_cache =
       ParseUpdateViaCache(options.updateViaCache());
 
-  RegisterServiceWorkerImpl(
-      execution_context, script_url, pattern_url, update_via_cache,
-      std::make_unique<CallbackPromiseAdapter<ServiceWorkerRegistration,
-                                              ServiceWorkerErrorForUpdate>>(
-          resolver));
-
+  provider_->RegisterServiceWorker(pattern_url, script_url, update_via_cache,
+                                   std::move(callbacks));
   return promise;
 }
 

third_party/blink/renderer/modules/serviceworkers/service_worker_container.h

@@ -75,12 +75,6 @@ class MODULES_EXPORT ServiceWorkerContainer final
   ScriptPromise ready(ScriptState*);
   WebServiceWorkerProvider* Provider() { return provider_; }
 
-  void RegisterServiceWorkerImpl(ExecutionContext*,
-                                 const KURL& script_url,
-                                 const KURL& scope,
-                                 mojom::ServiceWorkerUpdateViaCache,
-                                 std::unique_ptr<RegistrationCallbacks>);
-
   ScriptPromise registerServiceWorker(ScriptState*,
                                       const String& pattern,
                                       const RegistrationOptions&);