Reject permissonState calls if userVisible is not present as parameter

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 90a7f642a0609fd76d63b1742bcce2aaf7792703..9d7810367dd2b220453c45762996cfa207868453 100644
--- a/Source/modules/push_messaging/PushManager.cpp
+++ b/Source/modules/push_messaging/PushManager.cpp
@@ -93,6 +93,10 @@ ScriptPromise PushManager::permissionState(ScriptState* scriptState, const PushS
         if (!document->domWindow() || !document->frame())
             return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Document is detached from window."));
     }
+    if (!options.userVisibleOnly()) {
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(NotSupportedError,

[johnme, 2015/05/08 15:48:22]

To change the subject (since I think Peter and Michael have covered layering),
why does this throw an exception when you access permissionState()? That's
incompatible with developers that base their implementation on the standardized
Push API, and the unexpected exception could break their entire website.
Wouldn't it be better to return "denied" instead (and possibly log a
once-per-pageload message to the console if we want to be helpful)?

[johnme, 2015/05/08 15:57:07]

Oh, it's rejecting the promise with an exception, rather than throwing one.
That's not quite so bad, but it might still cause developers to trip up, since
whilst the spec allows the promise to be rejected, it doesn't list any
circumstances under which this would happen, and it's hard to imagine any good
reasons for a UA to do so either.

[Miguel Garcia, 2015/05/08 21:00:32]

Yes, it feels more accurate than just denying. That's also what the permission
API does.


On 2015/05/08 15:57:07, johnme wrote:

+            "Chrome does not support push subscriptions that do not enable the userVisibleOnly option."));
+    }
 
     RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();