Bug 30409 – Need to turn off notifications properly at runtime

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Andrew Wilson]

This if statement should evaluate to false if notifications are disabled at runtime:

if ('webkitNotifications' in window)

Additionally, we need to disable notifications from worker context as well. This means that we need to move runtime enabling out of Settings and into some static variable/struct that can be called from any context.

You can look at https://bugs.webkit.org/show_bug.cgi?id=30240 for an example of how this was done for window.Audio()

[John Gregg]

Created attachment 41529 [details]
patch

Patch adds support for the [EnabledAtRuntime] flag on DOMWindow.webkitNotifications and WorkerContext.webkitNotifications.  

After I switch chromium over to using this instead of Settings, I'll remove notifications from Settings in a second patch.

[Eric Seidel (no email)]

I'm not sure if this is a two-sided patch needing special commit, or if this can be added to the commit-queue.

[John Gregg]

I already had atwilson commit this for me:
http://trac.webkit.org/changeset/49893

[Eric Seidel (no email)]

So the bug should have been closed. :)

[John Gregg]

And now there's an even newer way of doing this.

[John Gregg]

Created attachment 41750 [details]
new patch for the new way

[John Gregg]

Comment on attachment 41750 [details]
new patch for the new way

d'oh forgot the r? last week.  now the patch is too out of date... will merge and resubmit.

[John Gregg]

Created attachment 42219 [details]
new patch for the new way 

resynced and recreated the patch.

[David Levin]

Comment on attachment 42219 [details]
new patch for the new way 

> Index: WebCore/bindings/v8/RuntimeEnabledFeatures.h
> +    static bool isNotificationsEnabled;

Hmmm, this should have been m_ since these are class members but I see you're following a pattern.

> Index: WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
> +    return WebCore::RuntimeEnabledFeatures::notificationsEnabled();

Since the code is in namespace WebCore, WebCore:: isn't needed.


> Index: WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp
> +    return WebCore::RuntimeEnabledFeatures::notificationsEnabled();

Since the code is in namespace WebCore, WebCore:: isn't needed.


Just remove the WebCore:: from the two places mentioned on landing.

[John Gregg]

Created attachment 42221 [details]
patch w/levin's feedback

removing unnecessary WebCore::'s

[WebKit Commit Bot]

Comment on attachment 42221 [details]
patch w/levin's feedback

Clearing flags on attachment: 42221

Committed r50348: <http://trac.webkit.org/changeset/50348>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.