[webkit-reviews] review granted: [Bug 238188] Only show notification permission prompt on transient activation : [Attachment 455337] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Ben Nham <nham at apple.com>'s
request for review:
Bug 238188: Only show notification permission prompt on transient activation
https://bugs.webkit.org/show_bug.cgi?id=238188

Attachment 455337: Patch

https://bugs.webkit.org/attachment.cgi?id=455337&action=review




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 455337
  --> https://bugs.webkit.org/attachment.cgi?id=455337
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455337&action=review

> Source/WebCore/Modules/push-api/PushManager.cpp:140
> +	       auto* window = document.frame()->window();

check frame is null?

> Source/WebCore/Modules/push-api/PushManager.cpp:141
> +	       if (!window || !window->consumeTransientActivation()) {

I would be tempted to do that check before enqueuing a task, but I guess this
is more consistent the way you did it with the way the algorithm is done.
The spec is a bit loose there, I filed
https://github.com/w3c/push-api/issues/346 to cover this.
I guess the transient activation is implicitly done as part of step 8.
This is potentially racy, for instance:
- script checks subscribe permission (it is allowed).
- before this event loop task is run, permission is changed to prompt
- subscribe fails

Is it fully consistent with Chrome and Firefox, aka is transient activation
only checked when permission is neither granted nor denied?


> LayoutTests/ChangeLog:8
> +	   Add test cases to make sure that showing a permission prompt
consumes a user gesture.

Can you add a test validating that calling subscribe out of user gesture
'works' if permission is denied or permission is granted?