2011-09-13 Pavel Feldman <pfeldman@google.com>
authorpfeldman@chromium.org <pfeldman@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Sep 2011 15:09:56 +0000 (15:09 +0000)
committerpfeldman@chromium.org <pfeldman@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Sep 2011 15:09:56 +0000 (15:09 +0000)
        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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@95093 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorInstrumentation.cpp
Source/WebCore/inspector/InspectorInstrumentation.h
Source/WebCore/inspector/InspectorPageAgent.cpp
Source/WebCore/inspector/InspectorPageAgent.h
Source/WebCore/page/Frame.cpp

index 62516cc..3013837 100644 (file)
@@ -1,3 +1,31 @@
+2011-09-13  Pavel Feldman  <pfeldman@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  <pfeldman@google.com>
 
         Not reviewed: rolling out r95089.
index 588f37e..5d0c3fc 100644 (file)
@@ -660,12 +660,6 @@ void InspectorInstrumentation::didCommitLoadImpl(InstrumentingAgents* instrument
         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())
index 732702a..6f59ee4 100644 (file)
@@ -142,7 +142,6 @@ public:
     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 @@ private:
     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 @@ inline void InspectorInstrumentation::didCommitLoad(Frame* frame, DocumentLoader
 #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)
index 0b55f24..bddbb54 100644 (file)
@@ -558,7 +558,12 @@ void InspectorPageAgent::frameNavigated(DocumentLoader* loader)
 
 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 @@ String InspectorPageAgent::loaderId(DocumentLoader* loader)
     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);
index 89bc9bf..c8dd0de 100644 (file)
@@ -102,7 +102,6 @@ public:
     void loadEventFired();
     void frameNavigated(DocumentLoader*);
     void frameDetached(Frame*);
-    void frameDestroyed(Frame*);
     void loaderDetachedFromFrame(DocumentLoader*);
 
     // Inspector Controller API
index e891e87..756f5b7 100644 (file)
@@ -238,8 +238,6 @@ Frame::~Frame()
     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();