Bug 41783 - NotificationPresenter needs a cancelRequestPermission API
Summary: NotificationPresenter needs a cancelRequestPermission API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 41413
  Show dependency treegraph
 
Reported: 2010-07-07 12:00 PDT by Yael
Modified: 2010-08-03 08:38 PDT (History)
7 users (show)

See Also:


Attachments
Patch (13.02 KB, patch)
2010-07-07 15:21 PDT, Yael
no flags Details | Formatted Diff | Diff
Patch (13.03 KB, patch)
2010-07-08 06:43 PDT, Yael
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yael 2010-07-07 12:00:39 PDT
There should be a NotificationPresenter API to cancel permission request, if the ScriptExecutionContext that issued the request is gone. 
Loading the following page in Chromium, and clicking the button shows the problem. the request is still visible to the user, even though the NotificationCenter that created it is gone.

<html>
<script>
function remove() {
document.getElementById("par").innerHTML="Do not crash";
}
</script>
Click this button to remove the iframe <button onclick="remove();">click me</button><br>
<div id="par"><iframe src="http://slides.html5rocks.com/#slide12" width=800 height=800></iframe></div>
</html>
Comment 1 Yael 2010-07-07 15:21:26 PDT
Created attachment 60788 [details]
Patch

This patch is preparation work for https://bugs.webkit.org/show_bug.cgi?id=41413.
It adds a new API NotificationPresenter::cancelRequestsForPermission, but it does not implement or test the API yet. It also changes the parameters passed in NotificationPresenter::checkPermission and NotificationPresenter::requestPermission. These API now use ScriptExecutionContext instead of origin, to provide more information to the client about who issued the request.
Comment 2 WebKit Review Bot 2010-07-07 17:06:12 PDT
Attachment 60788 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3400376
Comment 3 Yael 2010-07-08 06:43:42 PDT
Created attachment 60871 [details]
Patch

Attempt to fix Chromium build
Comment 4 John Gregg 2010-07-08 09:10:59 PDT
That patch seems reasonable to me.
Comment 5 Yael 2010-07-08 09:46:55 PDT
(In reply to comment #4)
> That patch seems reasonable to me.

Thanks!
Comment 6 Laszlo Gombos 2010-07-08 10:39:05 PDT
Comment on attachment 60871 [details]
Patch

Looks good to me too as well, r+.
Comment 7 WebKit Commit Bot 2010-07-09 04:47:31 PDT
Comment on attachment 60871 [details]
Patch

Clearing flags on attachment: 60871

Committed r62939: <http://trac.webkit.org/changeset/62939>
Comment 8 WebKit Commit Bot 2010-07-09 04:47:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Simon Hausmann 2010-08-03 08:38:34 PDT
Revision r62939 cherry-picked into qtwebkit-2.1 with commit 58536183c1385be9fa93b1e7c5f875fef0736c93