ServiceWorker: Should throw TypeError instead of Unknown/SecurityError.

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

Index: third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerWindowClient.cpp
diff --git a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerWindowClient.cpp b/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerWindowClient.cpp
index fd3db7a01baae5722ec077b8b21ee299a499dc72..dbbe617f3464b26cafeb99d038a0a4047f07966f 100644
--- a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerWindowClient.cpp
+++ b/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerWindowClient.cpp
@@ -15,6 +15,7 @@
 #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 {
@@ -60,6 +61,36 @@ ScriptPromise ServiceWorkerWindowClient::focus(ScriptState* scriptState)
     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);
@@ -72,11 +103,11 @@ ScriptPromise ServiceWorkerWindowClient::navigate(ScriptState* scriptState, cons
         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;
 }