Diff
Modified: trunk/Source/WebCore/ChangeLog (97642 => 97643)
--- trunk/Source/WebCore/ChangeLog 2011-10-17 20:43:43 UTC (rev 97642)
+++ trunk/Source/WebCore/ChangeLog 2011-10-17 20:46:29 UTC (rev 97643)
@@ -1,3 +1,25 @@
+2011-10-17 Dmitry Titov <dim...@chromium.org>
+
+ window.webkitNotifications uses deallocated NotificationPresenter after live Iframe transfer.
+ https://bugs.webkit.org/show_bug.cgi?id=70147
+
+ Reviewed by David Levin.
+
+ I only found a way to test this manually, since Chromium TestShell uses static instance
+ of NotificationPresenter instead of per-page one so the issue does not reproduce.
+ Adding manual test that works in full build of Chromium.
+
+ * manual-tests/iframe_notifications/iframe-reparenting-close-window-child.html: Added.
+ * manual-tests/iframe_notifications/iframe-reparenting-close-window-iframe.html: Added.
+ * manual-tests/iframe_notifications/iframe-reparenting-close-window.html: Added.
+ * notifications/NotificationCenter.cpp:
+ (WebCore::NotificationCenter::disconnectFrame):
+ * page/DOMWindow.cpp:
+ (WebCore::DOMWindow::resetNotifications):
+ * page/DOMWindow.h:
+ * page/Frame.cpp:
+ (WebCore::Frame::transferChildFrameToNewDocument): reset webkitNotifications object.
+
2011-10-17 Jeff Miller <je...@apple.com>
Widget window coordinate functions should use root view coordinate functions
Added: trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window-child.html (0 => 97643)
--- trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window-child.html (rev 0)
+++ trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window-child.html 2011-10-17 20:46:29 UTC (rev 97643)
@@ -0,0 +1,29 @@
+<html>
+<script>
+
+var iframe;
+
+window.addAndTransferIframe = function()
+{
+ iframe = document.createElement('iframe');
+ iframe.setAttribute('width', '500');
+ iframe.setAttribute('height', '350');
+ iframe.setAttribute('src', 'iframe-reparenting-close-window-iframe.html');
+ window.document.body.appendChild(iframe);
+}
+
+// Called from Iframe when it is loaded and initialized.
+window.transferIframe = function()
+{
+ var backgroundWin = window.opener;
+ backgroundWin.log("Transferring Iframe now.");
+ backgroundWin.document.adoptNode(iframe);
+ backgroundWin.document.body.appendChild(iframe);
+ iframe.contentWindow.finish();
+ window.close();
+}
+</script>
+
+<body>
+</body>
+</html>
\ No newline at end of file
Property changes on: trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window-child.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window-iframe.html (0 => 97643)
--- trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window-iframe.html (rev 0)
+++ trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window-iframe.html 2011-10-17 20:46:29 UTC (rev 97643)
@@ -0,0 +1,26 @@
+<html>
+<head>
+<script>
+var logWin = window.parent.opener;
+logWin.log('Initializing Iframe');
+
+var notificationCenter = null;
+function test() {
+ notificationCenter = window.webkitNotifications;
+ logWin.log("Before transfer: checkPermission returned (should be 1): " + notificationCenter.checkPermission());
+ setTimeout("window.parent.transferIframe();", 5000); // Wait long enough for Chrome popup blocker to release the window so it can actually close.
+}
+
+function testAfterClose()
+{
+ logWin.log("After transfer: checkPermission returned (should be 2): " + notificationCenter.checkPermission());
+}
+window.finish = function() {
+ logWin.log("After transfer, the checkPermission call is accessing a destroyed object and can return invalid value or crash, depending on circumstances.");
+ setInterval(testAfterClose, 1000); // Do it several times in a row, it'll crash after 1-3 times
+}
+</script>
+</head>
+<body _onload_=test()>
+</body>
+</html>
\ No newline at end of file
Property changes on: trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window-iframe.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Added: trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window.html (0 => 97643)
--- trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window.html (rev 0)
+++ trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window.html 2011-10-17 20:46:29 UTC (rev 97643)
@@ -0,0 +1,31 @@
+<html>
+<script>
+window.log = function(message)
+{
+ document.getElementById("log").innerText += message + "\n";
+}
+
+function childLoaded()
+{
+ log("Child window loaded.");
+ window.childWindow.addAndTransferIframe();
+}
+
+function start()
+{
+ window.childWindow = window.open("iframe-reparenting-close-window-child.html", "_blank");
+ window.childWindow.addEventListener("load", childLoaded, false);
+}
+
+</script>
+<body>
+<p>Bug: https://bugs.webkit.org/show_bug.cgi?id=70147</p>
+<p>This test recreates scenario when an iframe is reparented from one page to another using 'live iframe transfer'
+ (with adoptNode() used on the iframe right before re-parenting into the other page's DOM tree).
+ Then the original page is closed, destroying some internal objects that are associated with the top frame/page/WebVeiw.</p>
+<p>In Chromium, this destroys the underlying NotificationPresenter object which is associated with the Page, and as a result, the use of webkitNotification object from _javascript_ can crash the browser or return bogus results ('use-after-delete').
+ Open this test in Chromium and click the button to start the test. If the test doesn't crash, and prints expected results, the bug is not regressed.</p>
+<button _onclick_="start()">Start test</button>
+<pre id="log"></pre>
+</body>
+</html>
Property changes on: trunk/Source/WebCore/manual-tests/iframe_notifications/iframe-reparenting-close-window.html
___________________________________________________________________
Added: svn:executable
Added: svn:eol-style
Modified: trunk/Source/WebCore/notifications/NotificationCenter.cpp (97642 => 97643)
--- trunk/Source/WebCore/notifications/NotificationCenter.cpp 2011-10-17 20:43:43 UTC (rev 97642)
+++ trunk/Source/WebCore/notifications/NotificationCenter.cpp 2011-10-17 20:46:29 UTC (rev 97643)
@@ -61,9 +61,8 @@
void NotificationCenter::disconnectFrame()
{
- // m_notificationPresenter should never be 0. But just to be safe, we check it here.
- // Due to the mysterious bug http://code.google.com/p/chromium/issues/detail?id=49323.
- ASSERT(m_notificationPresenter);
+ // Can be 0 if iframe was transferred into another page. In this case
+ // this method is invoked more then once.
if (!m_notificationPresenter)
return;
m_notificationPresenter->cancelRequestsForPermission(scriptExecutionContext());
Modified: trunk/Source/WebCore/page/DOMWindow.cpp (97642 => 97643)
--- trunk/Source/WebCore/page/DOMWindow.cpp 2011-10-17 20:43:43 UTC (rev 97642)
+++ trunk/Source/WebCore/page/DOMWindow.cpp 2011-10-17 20:46:29 UTC (rev 97643)
@@ -748,6 +748,11 @@
return m_notifications.get();
}
+
+void DOMWindow::resetNotifications()
+{
+ m_notifications->disconnectFrame();
+}
#endif
void DOMWindow::pageDestroyed()
Modified: trunk/Source/WebCore/page/DOMWindow.h (97642 => 97643)
--- trunk/Source/WebCore/page/DOMWindow.h 2011-10-17 20:43:43 UTC (rev 97642)
+++ trunk/Source/WebCore/page/DOMWindow.h 2011-10-17 20:46:29 UTC (rev 97643)
@@ -380,6 +380,9 @@
#if ENABLE(NOTIFICATIONS)
NotificationCenter* webkitNotifications() const;
+ // Renders webkitNotifications object safely inoperable, disconnects
+ // if from embedder-provided NotificationPresenter.
+ void resetNotifications();
#endif
#if ENABLE(QUOTA)
Modified: trunk/Source/WebCore/page/Frame.cpp (97642 => 97643)
--- trunk/Source/WebCore/page/Frame.cpp 2011-10-17 20:43:43 UTC (rev 97642)
+++ trunk/Source/WebCore/page/Frame.cpp 2011-10-17 20:46:29 UTC (rev 97643)
@@ -746,8 +746,12 @@
// when the Geolocation's iframe is reparented.
// See https://bugs.webkit.org/show_bug.cgi?id=55577
// and https://bugs.webkit.org/show_bug.cgi?id=52877
- if (m_domWindow)
+ if (m_domWindow) {
m_domWindow->resetGeolocation();
+#if ENABLE(NOTIFICATIONS)
+ m_domWindow->resetNotifications();
+#endif
+ }
#if ENABLE(MEDIA_STREAM)
if (m_mediaStreamFrameController)
