Push API: move PushManager from Navigator to ServiceWorkerRegistration [switchover 6/6]

Source/modules/push_messaging/PushManager.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/push_messaging/PushManager.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContext.h"
 #include "core/frame/LocalDOMWindow.h"

[Peter Beverloo, 2014/12/08 21:40:51]

nit: This include seems to be unused.

[Michael van Ouwerkerk, 2014/12/09 18:47:45]

Done.

 #include "modules/push_messaging/PushController.h"
 #include "modules/push_messaging/PushError.h"
 #include "modules/push_messaging/PushPermissionStatusCallback.h"
 #include "modules/push_messaging/PushRegistration.h"
 #include "modules/serviceworkers/NavigatorServiceWorker.h"

[Peter Beverloo, 2014/12/08 21:40:51]

nit: This include seems to be unused.

[Michael van Ouwerkerk, 2014/12/09 18:47:45]

Could have sworn I removed that. Maybe I mishandled some merges. Done.

 #include "modules/serviceworkers/ServiceWorkerContainer.h"

[Peter Beverloo, 2014/12/08 21:40:51]

nit: This include seems to be unused.

[Michael van Ouwerkerk, 2014/12/09 18:47:45]

Done.

+#include "modules/serviceworkers/ServiceWorkerRegistration.h"
+#include "public/platform/Platform.h"
 #include "public/platform/WebPushClient.h"
+#include "public/platform/WebPushProvider.h"
 #include "wtf/RefPtr.h"
 
 namespace blink {
 
-PushManager::PushManager()
+PushManager::PushManager(ServiceWorkerRegistration* registration)
+    : m_registration(registration)
 {

[Peter Beverloo, 2014/12/08 21:40:51]

ASSERT(registration)

[Michael van Ouwerkerk, 2014/12/09 18:47:45]

Done.

 }
 
-// FIXME: This call should be available from workers which will not have a Document object available.
-// See crbug.com/389194
 ScriptPromise PushManager::registerPushMessaging(ScriptState* scriptState)
 {
-    ASSERT(scriptState->executionContext()->isDocument());
+    if (scriptState->executionContext()->isDocument()) {
+        Document* document = toDocument(scriptState->executionContext());
+        if (!document->domWindow() || !document->frame())
+            return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Document is detached from window."));
 
-    Document* document = toDocument(scriptState->executionContext());
-    if (!document->domWindow() || !document->frame())
-        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(AbortError, "Document is detached from window."));
-
-    WebServiceWorkerProvider* serviceWorkerProvider = NavigatorServiceWorker::serviceWorker(*document->domWindow()->navigator())->provider();
-    if (!serviceWorkerProvider)
-        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(AbortError, "No Service Worker installed for this document."));
-
-    WebPushClient* client = PushController::clientFrom(document->frame());
-    ASSERT(client);
+        WebPushClient* client = PushController::clientFrom(document->frame());
+        ASSERT(client);
+        RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+        ScriptPromise promise = resolver->promise();

[Peter Beverloo, 2014/12/08 21:40:51]

There's quite a bit of shared code here. Can we perhaps rewrite this method as:

ScriptPromise PushManager::registerPushMessaging(ScriptState* scriptState)
{
    RefPtr<ScriptPromiseResolver> resolver =
ScriptPromiseResolver::create(scriptState);
    ScriptPromise promise = resolver->promise();

    // Different code paths are used for documents and workers because..
[MVANOUWERKERK TO COMPLETE] 
    if (scriptState->executionContext()->isDocument()) {
        Document* document = toDocument(scriptState->executionContext());
        if (!document->domWindow() || !document->frame())
            return promise.reject(DOMException::create(InvalidStateError,
"Document is detached from window."));

        WebPushClient* client = PushController::clientFrom(document->frame());
        ASSERT(client);

        client->registerPushMessaging(m_registration->webRegistration(), new
CallbackPromiseAdapter<PushRegistration, PushError>(resolver));
    } else {
        pushProvider()->registerPushMessaging(m_registration->webRegistration(),
new CallbackPromiseAdapter<PushRegistration, PushError>(resolver));
    }

    return promise;
}

[Michael van Ouwerkerk, 2014/12/09 18:47:45]

Done.

+        client->registerPushMessaging(m_registration->webRegistration(), new CallbackPromiseAdapter<PushRegistration, PushError>(resolver));
+        return promise;
+    }
 
     RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
-
-    client->registerPushMessaging(new CallbackPromiseAdapter<PushRegistration, PushError>(resolver), serviceWorkerProvider);
-
+    Platform::current()->pushProvider()->registerPushMessaging(m_registration->webRegistration(), new CallbackPromiseAdapter<PushRegistration, PushError>(resolver));

[Peter Beverloo, 2014/12/08 21:40:51]

Can you add a utility method to receive the pushProvider()? We'll also want to
ASSERT somewhere to avoid messed up stack traces.

[Michael van Ouwerkerk, 2014/12/09 18:47:45]

Done.

     return promise;
 }
 
-// FIXME: This call should be available from workers which will not have a Document object available.
-// See crbug.com/389194
 ScriptPromise PushManager::hasPermission(ScriptState* scriptState)
 {
-    ASSERT(scriptState->executionContext()->isDocument());
-
-    Document* document = toDocument(scriptState->executionContext());
-    if (!document->domWindow() || !document->frame())
-        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Document is detached from window."));
-    blink::WebPushClient* client = PushController::clientFrom(document->frame());
-    ASSERT(client);
-
-    // The currently implemented specification does not require a Service Worker to be present for the
-    // hasPermission() call to work, but it will become a requirement soon.
-    WebServiceWorkerProvider* serviceWorkerProvider = NavigatorServiceWorker::serviceWorker(*document->domWindow()->navigator())->provider();
-    if (!serviceWorkerProvider)
-        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "No Service Worker installed for this document."));
+    if (scriptState->executionContext()->isDocument()) {
+        Document* document = toDocument(scriptState->executionContext());
+        if (!document->domWindow() || !document->frame())
+            return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Document is detached from window."));

[Peter Beverloo, 2014/12/08 21:40:51]

I'm a bit reluctant to continue having these checks if they're not reproducible
in layout tests. Can we perhaps keep the current layout tests for this, and
update it for the new syntax?

[Michael van Ouwerkerk, 2014/12/09 18:47:45]

As discussed offline, I ran into some trouble porting the detached document
tests using the testharness.js format. I'm happy to give it another shot in
future, I've filed a bug for this: https://crbug.com/440431

I also added an entry to the task tracker and FIXMEs in the code.

+    }
 
     RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
-
     ScriptPromise promise = resolver->promise();
-    client->getPermissionStatus(new PushPermissionStatusCallback(resolver), serviceWorkerProvider);
+    Platform::current()->pushProvider()->getPermissionStatus(m_registration->webRegistration(), new PushPermissionStatusCallback(resolver));
     return promise;
 }
 
+void PushManager::trace(Visitor* visitor)
+{
+    visitor->trace(m_registration);
+}
+
 } // namespace blink