Reject permissonState calls if userVisible is not present as parameter

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"
@@ skipping 75 matching lines @@
 }
 
 ScriptPromise PushManager::permissionState(ScriptState* scriptState, const PushSubscriptionOptions& options)
 {
     if (scriptState->executionContext()->isDocument()) {
         Document* document = toDocument(scriptState->executionContext());
         // FIXME: add test coverage for this condition - https://crbug.com/440431
         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();
 
     pushProvider()->getPermissionStatus(m_registration->webRegistration(), toWebPushSubscriptionOptions(options), new PushPermissionStatusCallbacks(resolver));
     return promise;
 }
 
 DEFINE_TRACE(PushManager)
 {
     visitor->trace(m_registration);
 }
 
 } // namespace blink