Changeset 63847 in webkit

Timestamp:

Jul 21, 2010 11:38:12 AM (14 years ago)

Author:

yael.aharon@nokia.com

Message:

2010-07-21 Yael Aharon <​yael.aharon@nokia.com>

Reviewed by Darin Adler.

Crash in Notification::disconnectFrame() triggered by Frame::lifeSupportTimerFired()
​https://bugs.webkit.org/show_bug.cgi?id=42534

Call NotificationsCenter::disconnectFrame() when the frame is disconnected from the page.
Calling it from the destructor of Frame is too late and sometimes causes access violation.
I was not able to reproduce this crash, so did not add new tests.
This patch is based on the error reported in
​http://code.google.com/p/chromium/issues/detail?id=49323.

• page/DOMWindow.cpp: (WebCore::DOMWindow::pageDestroyed):
• page/DOMWindow.h:
• page/Frame.cpp: (WebCore::Frame::pageDestroyed):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• 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/WebCore/ChangeLog

@@ +1 @@
+2010-07-21  Yael Aharon  <yael.aharon@nokia.com>
+
+        Reviewed by Darin Adler.
+
+        Crash in Notification::disconnectFrame() triggered by Frame::lifeSupportTimerFired()
+        https://bugs.webkit.org/show_bug.cgi?id=42534
+
+        Call NotificationsCenter::disconnectFrame() when the frame is disconnected from the page.
+        Calling it from the destructor of Frame is too late and sometimes causes access violation.
+        I was not able to reproduce this crash, so did not add new tests.
+        This patch is based on the error reported in 
+        http://code.google.com/p/chromium/issues/detail?id=49323.
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::pageDestroyed):
+        * page/DOMWindow.h:
+        * page/Frame.cpp:
+        (WebCore::Frame::pageDestroyed):
+
 2010-07-21  Anders Carlsson  <andersca@apple.com>
 

trunk/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);
+    if (!m_notificationPresenter)
+        return;
     m_notificationPresenter->cancelRequestsForPermission(m_scriptExecutionContext);
     m_notificationPresenter = 0;

trunk/WebCore/page/DOMWindow.cpp

@@ -674 +674 @@
 #endif
 
+void DOMWindow::pageDestroyed()
+{
+#if ENABLE(NOTIFICATIONS)
+    // Clearing Notifications requests involves accessing the client so it must be done
+    // before the frame is detached.
+    if (m_notifications)
+        m_notifications->disconnectFrame();
+    m_notifications = 0;
+#endif
+}
+
 #if ENABLE(INDEXED_DATABASE)
 IndexedDatabaseRequest* DOMWindow::indexedDB() const

trunk/WebCore/page/DOMWindow.h

@@ -228 +228 @@
         NotificationCenter* webkitNotifications() const;
 #endif
+
+        void pageDestroyed();
 
 #if ENABLE(INDEXED_DATABASE)

trunk/WebCore/page/Frame.cpp

@@ -1348 +1348 @@
         parent->loader()->checkLoadComplete();
 
+    if (m_domWindow)
+        m_domWindow->pageDestroyed();
+
     // FIXME: It's unclear as to why this is called more than once, but it is,
     // so page() could be NULL.