- Revision
- 95089
- Author
- [email protected]
- Date
- 2011-09-14 07:14:37 -0700 (Wed, 14 Sep 2011)
Log Message
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 + adds an assertion into the inspector agents lookup with 0 page.
Reviewed by Tony Gentilcore.
* inspector/InspectorInstrumentation.cpp:
(WebCore::InspectorInstrumentation::frameDetachedImpl):
(WebCore::InspectorInstrumentation::instrumentingAgentsForPage):
* inspector/InspectorInstrumentation.h:
(WebCore::InspectorInstrumentation::frameWindowDiscarded):
(WebCore::InspectorInstrumentation::domContentLoadedEventFired):
(WebCore::InspectorInstrumentation::loadEventFired):
(WebCore::InspectorInstrumentation::frameDetached):
(WebCore::InspectorInstrumentation::didCommitLoad):
* inspector/InspectorPageAgent.cpp:
(WebCore::InspectorPageAgent::frameDetached):
* inspector/InspectorPageAgent.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::detachFromParent):
* page/Frame.cpp:
(WebCore::Frame::~Frame):
(WebCore::Frame::detachFromPage):
(WebCore::Frame::transferChildFrameToNewDocument):
* page/Frame.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (95088 => 95089)
--- trunk/Source/WebCore/ChangeLog 2011-09-14 13:52:30 UTC (rev 95088)
+++ trunk/Source/WebCore/ChangeLog 2011-09-14 14:14:37 UTC (rev 95089)
@@ -1,3 +1,38 @@
+2011-09-13 Pavel Feldman <[email protected]>
+
+ 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 + adds an assertion into the inspector agents lookup with 0 page.
+
+ Reviewed by Tony Gentilcore.
+
+ * inspector/InspectorInstrumentation.cpp:
+ (WebCore::InspectorInstrumentation::frameDetachedImpl):
+ (WebCore::InspectorInstrumentation::instrumentingAgentsForPage):
+ * inspector/InspectorInstrumentation.h:
+ (WebCore::InspectorInstrumentation::frameWindowDiscarded):
+ (WebCore::InspectorInstrumentation::domContentLoadedEventFired):
+ (WebCore::InspectorInstrumentation::loadEventFired):
+ (WebCore::InspectorInstrumentation::frameDetached):
+ (WebCore::InspectorInstrumentation::didCommitLoad):
+ * inspector/InspectorPageAgent.cpp:
+ (WebCore::InspectorPageAgent::frameDetached):
+ * inspector/InspectorPageAgent.h:
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::detachFromParent):
+ * page/Frame.cpp:
+ (WebCore::Frame::~Frame):
+ (WebCore::Frame::detachFromPage):
+ (WebCore::Frame::transferChildFrameToNewDocument):
+ * page/Frame.h:
+
2011-09-14 Sheriff Bot <[email protected]>
Unreviewed, rolling out r95080.
Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp (95088 => 95089)
--- trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp 2011-09-14 13:52:30 UTC (rev 95088)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp 2011-09-14 14:14:37 UTC (rev 95089)
@@ -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())
@@ -883,6 +877,8 @@
InstrumentingAgents* InspectorInstrumentation::instrumentingAgentsForPage(Page* page)
{
+ // We should not be getting 0 page here, but we are not quite ready to remove the test below.
+ ASSERT(page);
if (!page)
return 0;
return instrumentationForPage(page);
Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.h (95088 => 95089)
--- trunk/Source/WebCore/inspector/InspectorInstrumentation.h 2011-09-14 13:52:30 UTC (rev 95088)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.h 2011-09-14 14:14:37 UTC (rev 95089)
@@ -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);
@@ -272,7 +271,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);
@@ -408,7 +406,10 @@
inline void InspectorInstrumentation::frameWindowDiscarded(Frame* frame, DOMWindow* domWindow)
{
#if ENABLE(INSPECTOR)
- if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForFrame(frame))
+ Page* page = frame->page();
+ if (!page)
+ return; // Entire page being destroyed.
+ if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForPage(page))
frameWindowDiscardedImpl(instrumentingAgents, domWindow);
#endif
}
@@ -863,7 +864,10 @@
inline void InspectorInstrumentation::domContentLoadedEventFired(Frame* frame, const KURL& url)
{
#if ENABLE(INSPECTOR)
- if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForFrame(frame))
+ Page* page = frame->page();
+ if (!page)
+ return; // DOMContentLoaded event triggered post frame detach.
+ if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForPage(page))
domContentLoadedEventFiredImpl(instrumentingAgents, frame, url);
#endif
}
@@ -871,7 +875,10 @@
inline void InspectorInstrumentation::loadEventFired(Frame* frame, const KURL& url)
{
#if ENABLE(INSPECTOR)
- if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForFrame(frame))
+ Page* page = frame->page();
+ if (!page)
+ return; // Load event triggered post frame detach.
+ if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForPage(page))
loadEventFiredImpl(instrumentingAgents, frame, url);
#endif
}
@@ -887,24 +894,14 @@
inline void InspectorInstrumentation::didCommitLoad(Frame* frame, DocumentLoader* loader)
{
#if ENABLE(INSPECTOR)
- if (!frame)
- return;
Page* page = frame->page();
if (!page)
- return;
+ return; // Commit load triggered post frame detach.
if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForPage(page))
didCommitLoadImpl(instrumentingAgents, page, loader);
#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 (95088 => 95089)
--- trunk/Source/WebCore/inspector/InspectorPageAgent.cpp 2011-09-14 13:52:30 UTC (rev 95088)
+++ trunk/Source/WebCore/inspector/InspectorPageAgent.cpp 2011-09-14 14:14:37 UTC (rev 95089)
@@ -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 (95088 => 95089)
--- trunk/Source/WebCore/inspector/InspectorPageAgent.h 2011-09-14 13:52:30 UTC (rev 95088)
+++ trunk/Source/WebCore/inspector/InspectorPageAgent.h 2011-09-14 14:14:37 UTC (rev 95089)
@@ -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 (95088 => 95089)
--- trunk/Source/WebCore/page/Frame.cpp 2011-09-14 13:52:30 UTC (rev 95088)
+++ trunk/Source/WebCore/page/Frame.cpp 2011-09-14 14:14:37 UTC (rev 95089)
@@ -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();
