Changeset 97643 in webkit

Timestamp:

Oct 17, 2011, 1:46:29 PM (14 years ago)

Author:

dimich@chromium.org

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• manual-tests/iframe_notifications (added)
• 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 (modified) (1 diff)
• page/DOMWindow.cpp (modified) (1 diff)
• page/DOMWindow.h (modified) (1 diff)
• page/Frame.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-10-17  Dmitry Titov  <dimich@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  <jeffm@apple.com>
 

trunk/Source/WebCore/notifications/NotificationCenter.cpp

@@ -62 +62 @@
 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;

trunk/Source/WebCore/page/DOMWindow.cpp

@@ -749 +749 @@
     return m_notifications.get();
 }
+
+void DOMWindow::resetNotifications()
+{
+    m_notifications->disconnectFrame();
+}
 #endif
 

trunk/Source/WebCore/page/DOMWindow.h

@@ -381 +381 @@
 #if ENABLE(NOTIFICATIONS)
         NotificationCenter* webkitNotifications() const;
+        // Renders webkitNotifications object safely inoperable, disconnects
+        // if from embedder-provided NotificationPresenter.
+        void resetNotifications();
 #endif
 

trunk/Source/WebCore/page/Frame.cpp

@@ -747 +747 @@
         // 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)