Title: [95093] trunk/Source/WebCore
Revision
95093
Author
pfeld...@chromium.org
Date
2011-09-14 08:09:56 -0700 (Wed, 14 Sep 2011)

Log Message

2011-09-13  Pavel Feldman  <pfeld...@google.com>

        Web Inspector: InspectorInstrumentation::frameDestroyed is called after m_page has been reset.
        https://bugs.webkit.org/show_bug.cgi?id=67997

        We should not instrument frameDestroyed event from within Frame's destructor
        since frame's m_page pointer is likely to be 0 by that time and appropriate
        instrumenting agent won't be found. As a result, stale frame with its id
        end up in the inspector.

        This change wipes out frame binding from the inspector upon detach rather
        than destroy.

        Reviewed by Tony Gentilcore.

        * inspector/InspectorInstrumentation.cpp:
        (WebCore::InspectorInstrumentation::frameDetachedImpl):
        * inspector/InspectorInstrumentation.h:
        * inspector/InspectorPageAgent.cpp:
        (WebCore::InspectorPageAgent::frameDetached):
        * inspector/InspectorPageAgent.h:
        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::detachFromParent):
        * page/Frame.cpp:
        (WebCore::Frame::~Frame):
        (WebCore::Frame::detachFromPage):
        * page/Frame.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (95092 => 95093)


--- trunk/Source/WebCore/ChangeLog	2011-09-14 14:59:46 UTC (rev 95092)
+++ trunk/Source/WebCore/ChangeLog	2011-09-14 15:09:56 UTC (rev 95093)
@@ -1,3 +1,31 @@
+2011-09-13  Pavel Feldman  <pfeld...@google.com>
+
+        Web Inspector: InspectorInstrumentation::frameDestroyed is called after m_page has been reset.
+        https://bugs.webkit.org/show_bug.cgi?id=67997
+
+        We should not instrument frameDestroyed event from within Frame's destructor
+        since frame's m_page pointer is likely to be 0 by that time and appropriate
+        instrumenting agent won't be found. As a result, stale frame with its id
+        end up in the inspector.
+
+        This change wipes out frame binding from the inspector upon detach rather
+        than destroy.
+
+        Reviewed by Tony Gentilcore.
+
+        * inspector/InspectorInstrumentation.cpp:
+        (WebCore::InspectorInstrumentation::frameDetachedImpl):
+        * inspector/InspectorInstrumentation.h:
+        * inspector/InspectorPageAgent.cpp:
+        (WebCore::InspectorPageAgent::frameDetached):
+        * inspector/InspectorPageAgent.h:
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::detachFromParent):
+        * page/Frame.cpp:
+        (WebCore::Frame::~Frame):
+        (WebCore::Frame::detachFromPage):
+        * page/Frame.h:
+
 2011-09-14  Pavel Feldman  <pfeld...@google.com>
 
         Not reviewed: rolling out r95089.

Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp (95092 => 95093)


--- trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp	2011-09-14 14:59:46 UTC (rev 95092)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp	2011-09-14 15:09:56 UTC (rev 95093)
@@ -660,12 +660,6 @@
         pageAgent->frameNavigated(loader);
 }
 
-void InspectorInstrumentation::frameDestroyedImpl(InstrumentingAgents* instrumentingAgents, Frame* frame)
-{
-    if (InspectorPageAgent* inspectorPageAgent = instrumentingAgents->inspectorPageAgent())
-        inspectorPageAgent->frameDestroyed(frame);
-}
-
 void InspectorInstrumentation::loaderDetachedFromFrameImpl(InstrumentingAgents* instrumentingAgents, DocumentLoader* loader)
 {
     if (InspectorPageAgent* inspectorPageAgent = instrumentingAgents->inspectorPageAgent())

Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.h (95092 => 95093)


--- trunk/Source/WebCore/inspector/InspectorInstrumentation.h	2011-09-14 14:59:46 UTC (rev 95092)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.h	2011-09-14 15:09:56 UTC (rev 95093)
@@ -142,7 +142,6 @@
     static void loadEventFired(Frame*, const KURL&);
     static void frameDetachedFromParent(Frame*);
     static void didCommitLoad(Frame*, DocumentLoader*);
-    static void frameDestroyed(Frame*);
     static void loaderDetachedFromFrame(Frame*, DocumentLoader*);
 
     static InspectorInstrumentationCookie willWriteHTML(Document*, unsigned int length, unsigned int startLine);
@@ -277,7 +276,6 @@
     static void loadEventFiredImpl(InstrumentingAgents*, Frame*, const KURL&);
     static void frameDetachedFromParentImpl(InstrumentingAgents*, Frame*);
     static void didCommitLoadImpl(InstrumentingAgents*, Page*, DocumentLoader*);
-    static void frameDestroyedImpl(InstrumentingAgents*, Frame*);
     static void loaderDetachedFromFrameImpl(InstrumentingAgents*, DocumentLoader*);
 
     static InspectorInstrumentationCookie willWriteHTMLImpl(InstrumentingAgents*, unsigned int length, unsigned int startLine);
@@ -909,14 +907,6 @@
 #endif
 }
 
-inline void InspectorInstrumentation::frameDestroyed(Frame* frame)
-{
-#if ENABLE(INSPECTOR)
-    if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForFrame(frame))
-        frameDestroyedImpl(instrumentingAgents, frame);
-#endif
-}
-
 inline void InspectorInstrumentation::loaderDetachedFromFrame(Frame* frame, DocumentLoader* loader)
 {
 #if ENABLE(INSPECTOR)

Modified: trunk/Source/WebCore/inspector/InspectorPageAgent.cpp (95092 => 95093)


--- trunk/Source/WebCore/inspector/InspectorPageAgent.cpp	2011-09-14 14:59:46 UTC (rev 95092)
+++ trunk/Source/WebCore/inspector/InspectorPageAgent.cpp	2011-09-14 15:09:56 UTC (rev 95093)
@@ -558,7 +558,12 @@
 
 void InspectorPageAgent::frameDetached(Frame* frame)
 {
-    m_frontend->frameDetached(frameId(frame));
+    HashMap<Frame*, String>::iterator iterator = m_frameToIdentifier.find(frame);
+    if (iterator != m_frameToIdentifier.end()) {
+        m_frontend->frameDetached(iterator->second);
+        m_identifierToFrame.remove(iterator->second);
+        m_frameToIdentifier.remove(iterator);
+    }
 }
 
 Frame* InspectorPageAgent::mainFrame()
@@ -596,15 +601,6 @@
     return identifier;
 }
 
-void InspectorPageAgent::frameDestroyed(Frame* frame)
-{
-    HashMap<Frame*, String>::iterator iterator = m_frameToIdentifier.find(frame);
-    if (iterator != m_frameToIdentifier.end()) {
-        m_identifierToFrame.remove(iterator->second);
-        m_frameToIdentifier.remove(iterator);
-    }
-}
-
 void InspectorPageAgent::loaderDetachedFromFrame(DocumentLoader* loader)
 {
     HashMap<DocumentLoader*, String>::iterator iterator = m_loaderToIdentifier.find(loader);

Modified: trunk/Source/WebCore/inspector/InspectorPageAgent.h (95092 => 95093)


--- trunk/Source/WebCore/inspector/InspectorPageAgent.h	2011-09-14 14:59:46 UTC (rev 95092)
+++ trunk/Source/WebCore/inspector/InspectorPageAgent.h	2011-09-14 15:09:56 UTC (rev 95093)
@@ -102,7 +102,6 @@
     void loadEventFired();
     void frameNavigated(DocumentLoader*);
     void frameDetached(Frame*);
-    void frameDestroyed(Frame*);
     void loaderDetachedFromFrame(DocumentLoader*);
 
     // Inspector Controller API

Modified: trunk/Source/WebCore/page/Frame.cpp (95092 => 95093)


--- trunk/Source/WebCore/page/Frame.cpp	2011-09-14 14:59:46 UTC (rev 95092)
+++ trunk/Source/WebCore/page/Frame.cpp	2011-09-14 15:09:56 UTC (rev 95093)
@@ -238,8 +238,6 @@
     for (HashSet<FrameDestructionObserver*>::iterator it = m_destructionObservers.begin(); it != stop; ++it)
         (*it)->frameDestroyed();
 
-    InspectorInstrumentation::frameDestroyed(this);
-
     if (m_view) {
         m_view->hide();
         m_view->clearFrame();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to