<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[177033] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/177033">177033</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2014-12-09 12:31:41 -0800 (Tue, 09 Dec 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>[WK2] Crash when answering notification permission request after navigating
https://bugs.webkit.org/show_bug.cgi?id=139429
<rdar://problem/18921122>
Reviewed by Andreas Kling.
Source/WebKit2:
When requesting a notification permission, navigating away and then
answering the permission, WebKit2 would crash. This is because upon
navigating, the request is cancelled and removed from the HashMaps
in NotificationPermissionRequestManager. When
didReceiveNotificationPermissionDecision() is later called, it would
look for the request identifier in m_idToOriginMap HashMap. As the
request was cancelled, HashMap::take() call would return null for
the SecurityOrigin*. This security origin pointer is then removed
from m_originToIDMap, but the code was failing to do a null check
first. Calling HashMap::remove(nullptr) would then crash.
This patch adds the missing null check and a layout test to cover
this case.
Test: http/tests/notifications/legacy/notification-request-permission-then-navigate.html
* WebProcess/Notifications/NotificationPermissionRequestManager.cpp:
(WebKit::NotificationPermissionRequestManager::didReceiveNotificationPermissionDecision):
LayoutTests:
Add layout test to cover the case where the notification permission is
requesting before navigating to a new URL and answered after the page
is navigated away.
* http/tests/notifications/legacy/notification-request-permission-then-navigate-expected.txt: Added.
* http/tests/notifications/legacy/notification-request-permission-then-navigate.html: Added.
* http/tests/notifications/legacy/resources/notify-opener-done.html: Added.
* http/tests/notifications/legacy/resources/request-permission-then-navigate.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2WebProcessNotificationsNotificationPermissionRequestManagercpp">trunk/Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestshttptestsnotificationslegacynotificationrequestpermissionthennavigateexpectedtxt">trunk/LayoutTests/http/tests/notifications/legacy/notification-request-permission-then-navigate-expected.txt</a></li>
<li><a href="#trunkLayoutTestshttptestsnotificationslegacynotificationrequestpermissionthennavigatehtml">trunk/LayoutTests/http/tests/notifications/legacy/notification-request-permission-then-navigate.html</a></li>
<li>trunk/LayoutTests/http/tests/notifications/legacy/resources/</li>
<li><a href="#trunkLayoutTestshttptestsnotificationslegacyresourcesnotifyopenerdonehtml">trunk/LayoutTests/http/tests/notifications/legacy/resources/notify-opener-done.html</a></li>
<li><a href="#trunkLayoutTestshttptestsnotificationslegacyresourcesrequestpermissionthennavigatehtml">trunk/LayoutTests/http/tests/notifications/legacy/resources/request-permission-then-navigate.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (177032 => 177033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2014-12-09 20:29:59 UTC (rev 177032)
+++ trunk/LayoutTests/ChangeLog 2014-12-09 20:31:41 UTC (rev 177033)
</span><span class="lines">@@ -1,3 +1,20 @@
</span><ins>+2014-12-09 Chris Dumez <cdumez@apple.com>
+
+ [WK2] Crash when answering notification permission request after navigating
+ https://bugs.webkit.org/show_bug.cgi?id=139429
+ <rdar://problem/18921122>
+
+ Reviewed by Andreas Kling.
+
+ Add layout test to cover the case where the notification permission is
+ requesting before navigating to a new URL and answered after the page
+ is navigated away.
+
+ * http/tests/notifications/legacy/notification-request-permission-then-navigate-expected.txt: Added.
+ * http/tests/notifications/legacy/notification-request-permission-then-navigate.html: Added.
+ * http/tests/notifications/legacy/resources/notify-opener-done.html: Added.
+ * http/tests/notifications/legacy/resources/request-permission-then-navigate.html: Added.
+
</ins><span class="cx"> 2014-12-09 Michael Saboff <msaboff@apple.com>
</span><span class="cx">
</span><span class="cx"> DFG Tries using an inner object's getter/setter when one hasn't been defined
</span></span></pre></div>
<a id="trunkLayoutTestshttptestsnotificationslegacynotificationrequestpermissionthennavigateexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/notifications/legacy/notification-request-permission-then-navigate-expected.txt (0 => 177033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/notifications/legacy/notification-request-permission-then-navigate-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/notifications/legacy/notification-request-permission-then-navigate-expected.txt 2014-12-09 20:31:41 UTC (rev 177033)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+main frame - has 1 onunload handler(s)
+Tests that we are not crashing when a permission request is answered after navigating
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS This test did not crash
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestshttptestsnotificationslegacynotificationrequestpermissionthennavigatehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/notifications/legacy/notification-request-permission-then-navigate.html (0 => 177033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/notifications/legacy/notification-request-permission-then-navigate.html (rev 0)
+++ trunk/LayoutTests/http/tests/notifications/legacy/notification-request-permission-then-navigate.html 2014-12-09 20:31:41 UTC (rev 177033)
</span><span class="lines">@@ -0,0 +1,27 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<head>
+<script src="/resources/js-test-pre.js"></script>
+<script src="/resources/notifications-test-pre.js"></script>
+</head>
+<body>
+<script>
+description("Tests that we are not crashing when a permission request is answered after navigating");
+jsTestIsAsync = true;
+
+if (window.testRunner)
+ testRunner.setCanOpenWindows();
+
+window.addEventListener('message', function(event)
+{
+ testPassed("This test did not crash");
+ target.close();
+ finishJSTest();
+});
+
+target = window.open('resources/request-permission-then-navigate.html');
+
+</script>
+<script src="/resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestshttptestsnotificationslegacyresourcesnotifyopenerdonehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/notifications/legacy/resources/notify-opener-done.html (0 => 177033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/notifications/legacy/resources/notify-opener-done.html (rev 0)
+++ trunk/LayoutTests/http/tests/notifications/legacy/resources/notify-opener-done.html 2014-12-09 20:31:41 UTC (rev 177033)
</span><span class="lines">@@ -0,0 +1,4 @@
</span><ins>+<!DOCTYPE html>
+<script>
+opener.postMessage("DONE", '*');
+</script>
</ins></span></pre></div>
<a id="trunkLayoutTestshttptestsnotificationslegacyresourcesrequestpermissionthennavigatehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/notifications/legacy/resources/request-permission-then-navigate.html (0 => 177033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/notifications/legacy/resources/request-permission-then-navigate.html (rev 0)
+++ trunk/LayoutTests/http/tests/notifications/legacy/resources/request-permission-then-navigate.html 2014-12-09 20:31:41 UTC (rev 177033)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+<!DOCTYPE html>
+<script>
+function requestPermission()
+{
+ window.webkitNotifications.requestPermission(function() { });
+}
+
+window.onunload = requestPermission;
+window.location = "notify-opener-done.html"
+</script>
</ins></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (177032 => 177033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2014-12-09 20:29:59 UTC (rev 177032)
+++ trunk/Source/WebKit2/ChangeLog 2014-12-09 20:31:41 UTC (rev 177033)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2014-12-09 Chris Dumez <cdumez@apple.com>
+
+ [WK2] Crash when answering notification permission request after navigating
+ https://bugs.webkit.org/show_bug.cgi?id=139429
+ <rdar://problem/18921122>
+
+ Reviewed by Andreas Kling.
+
+ When requesting a notification permission, navigating away and then
+ answering the permission, WebKit2 would crash. This is because upon
+ navigating, the request is cancelled and removed from the HashMaps
+ in NotificationPermissionRequestManager. When
+ didReceiveNotificationPermissionDecision() is later called, it would
+ look for the request identifier in m_idToOriginMap HashMap. As the
+ request was cancelled, HashMap::take() call would return null for
+ the SecurityOrigin*. This security origin pointer is then removed
+ from m_originToIDMap, but the code was failing to do a null check
+ first. Calling HashMap::remove(nullptr) would then crash.
+
+ This patch adds the missing null check and a layout test to cover
+ this case.
+
+ Test: http/tests/notifications/legacy/notification-request-permission-then-navigate.html
+
+
+ * WebProcess/Notifications/NotificationPermissionRequestManager.cpp:
+ (WebKit::NotificationPermissionRequestManager::didReceiveNotificationPermissionDecision):
+
</ins><span class="cx"> 2014-12-09 Anders Carlsson <andersca@apple.com>
</span><span class="cx">
</span><span class="cx"> Implement clearing of cookies
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessNotificationsNotificationPermissionRequestManagercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cpp (177032 => 177033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cpp 2014-12-09 20:29:59 UTC (rev 177032)
+++ trunk/Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cpp 2014-12-09 20:31:41 UTC (rev 177033)
</span><span class="lines">@@ -157,6 +157,9 @@
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> RefPtr<WebCore::SecurityOrigin> origin = m_idToOriginMap.take(requestID);
</span><ins>+ if (!origin)
+ return;
+
</ins><span class="cx"> m_originToIDMap.remove(origin);
</span><span class="cx">
</span><span class="cx"> WebProcess::shared().supplement<WebNotificationManager>()->didUpdateNotificationDecision(origin->toString(), allowed);
</span></span></pre>
</div>
</div>
</body>
</html>