Changeset 119219 in webkit

Timestamp:

Jun 1, 2012 5:30:09 AM (12 years ago)

Author:

morrita@google.com

Message:

Frame::willDetachPage() shouldn't be called more than once.
​https://bugs.webkit.org/show_bug.cgi?id=88056

Reviewed by Ryosuke Niwa.

This change moved willDetachPage() to private and calls it from
detachFromParent(). Also, it checks m_page and call
willDetachPage() only once, if m_page is available.

No new tests. Covered by existing suites.

• loader/FrameLoader.cpp:

(WebCore::FrameLoader::closeAndRemoveChild):
(WebCore::FrameLoader::detachFromParent):

• page/Frame.cpp:

(WebCore::Frame::detachFromPage):
(WebCore):
(WebCore::Frame::willDetachPage):

• page/Frame.h:

(Frame):

• page/Page.cpp:

(WebCore::Page::~Page):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/FrameLoader.cpp (modified) (2 diffs)
• page/Frame.cpp (modified) (1 diff)
• page/Frame.h (modified) (3 diffs)
• page/Page.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-06-01  MORITA Hajime  <morrita@google.com>
+
+        Frame::willDetachPage() shouldn't be called more than once.
+        https://bugs.webkit.org/show_bug.cgi?id=88056
+
+        Reviewed by Ryosuke Niwa.
+
+        This change moved willDetachPage() to private and calls it from
+        detachFromParent().  Also, it checks m_page and call
+        willDetachPage() only once, if m_page is available.
+
+        No new tests. Covered by existing suites.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::closeAndRemoveChild):
+        (WebCore::FrameLoader::detachFromParent):
+        * page/Frame.cpp:
+        (WebCore::Frame::detachFromPage):
+        (WebCore):
+        (WebCore::Frame::willDetachPage):
+        * page/Frame.h:
+        (Frame):
+        * page/Page.cpp:
+        (WebCore::Page::~Page):
+
 2012-06-01  Pierre Rossi  <pierre.rossi@gmail.com>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -2267 +2267 @@
     if (child->ownerElement() && child->page())
         child->page()->decrementFrameCount();
-    child->willDetachPage();
     child->detachFromPage();
 
@@ -2346 +2345 @@
     } else {
         m_frame->setView(0);
-        m_frame->willDetachPage();
         m_frame->detachFromPage();
     }

trunk/Source/WebCore/page/Frame.cpp

@@ -690 +690 @@
 }
 
-void Frame::willDetachPage()
+void Frame::detachFromPage()
 {
     if (Frame* parent = tree()->parent())
         parent->loader()->checkLoadComplete();
-
+    if (m_page)
+        willDetachPage();
+    m_page = 0;
+}
+
+
+void Frame::willDetachPage()
+{
     HashSet<FrameDestructionObserver*>::iterator stop = m_destructionObservers.end();
     for (HashSet<FrameDestructionObserver*>::iterator it = m_destructionObservers.begin(); it != stop; ++it)
         (*it)->willDetachPage();
 
-    // FIXME: It's unclear as to why this is called more than once, but it is,
-    // so page() could be NULL.
-    if (page() && page()->focusController()->focusedFrame() == this)
+    if (page()->focusController()->focusedFrame() == this)
         page()->focusController()->setFocusedFrame(0);
 

trunk/Source/WebCore/page/Frame.h

@@ -92 +92 @@
         void removeDestructionObserver(FrameDestructionObserver*);
 
-        void willDetachPage();
         void detachFromPage();
         void disconnectOwnerElement();
@@ -208 +207 @@
 
         void injectUserScriptsForWorld(DOMWrapperWorld*, const UserScriptVector&, UserScriptInjectionTime);
+        void willDetachPage();
 
         HashSet<FrameDestructionObserver*> m_destructionObservers;
@@ -352 +352 @@
     }
 
-    inline void Frame::detachFromPage()
-    {
-        m_page = 0;
-    }
-
     inline EventHandler* Frame::eventHandler() const
     {

trunk/Source/WebCore/page/Page.cpp

@@ -188 +188 @@
     allPages->remove(this);
     
-    for (Frame* frame = mainFrame(); frame; frame = frame->tree()->traverseNext()) {
-        frame->willDetachPage();
+    for (Frame* frame = mainFrame(); frame; frame = frame->tree()->traverseNext())
         frame->detachFromPage();
-    }
 
     m_editorClient->pageDestroyed();