ServiceWorker: Should throw TypeError instead of Unknown/SecurityError.

third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerWindowClient.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "modules/serviceworkers/ServiceWorkerWindowClient.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/page/PageVisibilityState.h"
 #include "core/workers/WorkerGlobalScope.h"
 #include "core/workers/WorkerLocation.h"
 #include "modules/serviceworkers/ServiceWorkerError.h"
 #include "modules/serviceworkers/ServiceWorkerGlobalScopeClient.h"
 #include "public/platform/WebString.h"
+#include "public/platform/modules/serviceworker/WebServiceWorkerClientsInfo.h"
 #include "wtf/RefPtr.h"
 
 namespace blink {
 
 ServiceWorkerWindowClient* ServiceWorkerWindowClient::take(ScriptPromiseResolver*, PassOwnPtr<WebServiceWorkerClientInfo> webClient)
 {
     return webClient ? ServiceWorkerWindowClient::create(*webClient) : nullptr;
 }
 
 ServiceWorkerWindowClient* ServiceWorkerWindowClient::create(const WebServiceWorkerClientInfo& info)
@@ skipping 25 matching lines @@
     if (!scriptState->executionContext()->isWindowInteractionAllowed()) {
         resolver->reject(DOMException::create(InvalidAccessError, "Not allowed to focus a window."));
         return promise;
     }
     scriptState->executionContext()->consumeWindowInteraction();
 
     ServiceWorkerGlobalScopeClient::from(scriptState->executionContext())->focus(uuid(), new CallbackPromiseAdapter<ServiceWorkerWindowClient, ServiceWorkerError>(resolver));
     return promise;
 }
 
+class NavigateClientCallback : public WebServiceWorkerClientCallbacks {
+public:
+    explicit NavigateClientCallback(ScriptPromiseResolver* resolver)
+        : m_resolver(resolver) { }
+
+    void onSuccess(WebPassOwnPtr<WebServiceWorkerClientInfo> clientInfo) override
+    {
+        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
+            return;
+        m_resolver->resolve(ServiceWorkerWindowClient::take(m_resolver.get(), clientInfo.release()));
+    }
+
+    void onError(const WebServiceWorkerError& error) override
+    {
+        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
+            return;
+
+        if (error.errorType == WebServiceWorkerError::ErrorTypeUnknown)  {

[nhiroki, 2015/11/27 03:25:13]

I'd prefer not to give specific meaning to the unknown error type. How about
adding an unique error type for navigate() like ErrorNavigationError, and then
making ServiceWorkerError::take() filter it out as following code?

DOMException* ServiceWorkerError::take(ScriptPromiseResolver*, const
WebServiceWorkerError& webError) {
     switch (webError.errorType) {
     case WebServiceWorkerError::ErrorTypeNavigation:
         // ErrorTypeNavigation should have bailed out before calling this.
         ASSERT_NOT_REACHED();
         return DOMException::create(UnknownError);
     }
}

Bonus: openWindow() could reuse ErrorTypeNavigation.

[zino, 2015/11/27 03:41:29]

Done.

+            ScriptState::Scope scope(m_resolver->scriptState());
+            m_resolver->reject(V8ThrowException::createTypeError(m_resolver->scriptState()->isolate(), error.message));
+            return;
+        }
+
+        m_resolver->reject(ServiceWorkerError::take(m_resolver.get(), error));
+    }
+private:
+    Persistent<ScriptPromiseResolver> m_resolver;
+    WTF_MAKE_NONCOPYABLE(NavigateClientCallback);
+};
+
 ScriptPromise ServiceWorkerWindowClient::navigate(ScriptState* scriptState, const String& url)
 {
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
     ExecutionContext* context = scriptState->executionContext();
 
     KURL parsedUrl = KURL(toWorkerGlobalScope(context)->location()->url(), url);
     if (!parsedUrl.isValid() || parsedUrl.protocolIsAbout()) {
         resolver->reject(V8ThrowException::createTypeError(scriptState->isolate(), "'" + url + "' is not a valid URL."));
         return promise;
     }
     if (!context->securityOrigin()->canDisplay(parsedUrl)) {
-        resolver->reject(DOMException::create(SecurityError, "'" + parsedUrl.elidedString() + "' cannot navigate."));
+        resolver->reject(V8ThrowException::createTypeError(scriptState->isolate(), "'" + parsedUrl.elidedString() + "' cannot navigate."));
         return promise;
     }
 
-    ServiceWorkerGlobalScopeClient::from(context)->navigate(uuid(), parsedUrl, new CallbackPromiseAdapter<ServiceWorkerWindowClient, ServiceWorkerError>(resolver));
+    ServiceWorkerGlobalScopeClient::from(context)->navigate(uuid(), parsedUrl, new NavigateClientCallback(resolver));
     return promise;
 }
 
 DEFINE_TRACE(ServiceWorkerWindowClient)
 {
     ServiceWorkerClient::trace(visitor);
 }
 
 } // namespace blink