Blink implementation of PushManager#hasPermission()

Source/modules/push_messaging/PushManager.cpp

Index: Source/modules/push_messaging/PushManager.cpp
diff --git a/Source/modules/push_messaging/PushManager.cpp b/Source/modules/push_messaging/PushManager.cpp
index 1468d880d342ac3587df10d56c8d8d913a1c3329..acb9198ee3a45e8534575722aa67bf65848086b2 100644
--- a/Source/modules/push_messaging/PushManager.cpp
+++ b/Source/modules/push_messaging/PushManager.cpp
@@ -16,18 +16,21 @@
 #include "core/frame/LocalDOMWindow.h"
 #include "modules/push_messaging/PushController.h"
 #include "modules/push_messaging/PushError.h"
+#include "modules/push_messaging/PushPermissionCallback.h"
 #include "modules/push_messaging/PushRegistration.h"
 #include "modules/serviceworkers/NavigatorServiceWorker.h"
 #include "modules/serviceworkers/ServiceWorkerContainer.h"
 #include "public/platform/WebPushClient.h"
 #include "wtf/RefPtr.h"
 
+

[Michael van Ouwerkerk, 2014/10/20 11:25:42]

nit: delete this extra empty line

[Miguel Garcia, 2014/10/22 12:58:43]

Done.

 namespace blink {
 
 PushManager::PushManager()
 {
 }
 
+// FIXME: This calls should be available from workers which will not have a Document object available.

[Michael van Ouwerkerk, 2014/10/20 11:25:42]

nit: s/This/These/

[Michael van Ouwerkerk, 2014/10/20 11:25:42]

crbug.com/389194

[Miguel Garcia, 2014/10/22 12:58:43]

Done.

 ScriptPromise PushManager::registerPushMessaging(ScriptState* scriptState, const String& senderId)
 {
     ASSERT(scriptState->executionContext()->isDocument());
@@ -49,4 +52,25 @@ ScriptPromise PushManager::registerPushMessaging(ScriptState* scriptState, const
     return promise;
 }
 
+// FIXME: This calls should be available from workers which will not have a Document object available.

[Michael van Ouwerkerk, 2014/10/20 11:25:42]

nit: same as above

[Miguel Garcia, 2014/10/22 12:58:43]

Done.

+ScriptPromise PushManager::hasPermission(ScriptState* scriptState)
+{
+    ASSERT(scriptState->executionContext()->isDocument());
+
+    Document* document = toDocument(scriptState->executionContext());
+    if (!document->domWindow() || !document->page())
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Document is detached from window."));
+    blink::WebPushClient* client = PushController::clientFrom(document->page());
+    ASSERT(client);
+
+    WebServiceWorkerProvider* serviceWorkerProvider = NavigatorServiceWorker::serviceWorker(document->domWindow()->navigator())->provider();
+    if (!serviceWorkerProvider)
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "No Service Worker installed for this document."));

[Michael van Ouwerkerk, 2014/10/20 11:25:42]

The spec says to not provide any arguments when rejecting. If you feel
InvalidStateError is better (I agree) then please open a spec issue to propose
the API change, and a bug for us to track the divergence from the specced API.

[Peter Beverloo, 2014/10/20 12:23:49]

We don't care at all about whether there's a Service Worker or not, so this
entire check should be removed. Permissions for push are granted per origin, not
per service worker.

[Miguel Garcia, 2014/10/22 12:58:43]

mm I don't know, we use the service worker stuff later on to take the origin.
You cannot register if you don't have a SW anyway so we might just as well bail
out early.

I will file the bugs once we clarify this.





On 2014/10/20 12:23:49, Peter Beverloo wrote:

[Michael van Ouwerkerk, 2014/10/22 13:20:01]

You could take the origin from the RenderFrameHost using GetLastCommittedURL()
so no need for a service worker strictly speaking. I agree with Peter here, this
method does not require a service worker. If there isn't one now, there may
still be one later.

[Michael van Ouwerkerk, 2014/10/22 15:52:45]

Ok on further discussion... in future, when called from a service worker
context, there will not be a RenderFrameHost. It would also be an alarming error
state to not have a service worker, as the PushRegistrationManager will be owned
by a ServiceWorkerRegistration. So there is an interesting error to throw here.

+
+    RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+
+    client->permissionStatus(new PushPermissionCallback(resolver), serviceWorkerProvider);

[Michael van Ouwerkerk, 2014/10/20 11:25:42]

It's not a simple getter is it? Then please name the method as a verb e.g.
getPermissionStatus

[Miguel Garcia, 2014/10/22 12:58:43]

Done.

+    return resolver->promise();
+}
+
 } // namespace blink