Title: [89345] trunk
Revision
89345
Author
yu...@chromium.org
Date
2011-06-21 02:58:15 -0700 (Tue, 21 Jun 2011)

Log Message

2011-06-20  Yury Semikhatsky  <yu...@chromium.org>

        Reviewed by Pavel Feldman.

        Web Inspector: console messages shouldn't prevent garbage collection of iframes
        https://bugs.webkit.org/show_bug.cgi?id=62996

        * http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt: Added.
        * http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html: Added.
        * http/tests/inspector-enabled/resources/console-clear-arguments-iframe.html: Added.
2011-06-20  Yury Semikhatsky  <yu...@chromium.org>

        Reviewed by Pavel Feldman.

        Web Inspector: console messages shouldn't prevent garbage collection of iframes
        https://bugs.webkit.org/show_bug.cgi?id=62996

        When DOMWindow is detached from its frame run through all console messages and clear
        their arguments and ScriptState references. The message text in this case will be
        the first argument serialized to string.

        Test: http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html

        * bindings/js/ScriptState.cpp:
        (WebCore::domWindowFromScriptState):
        * bindings/js/ScriptState.h:
        * bindings/v8/ScriptState.cpp:
        (WebCore::ScriptState::domWindow):
        (WebCore::domWindowFromScriptState):
        * bindings/v8/ScriptState.h:
        * inspector/ConsoleMessage.cpp:
        (WebCore::ConsoleMessage::windowCleared):
        * inspector/ConsoleMessage.h:
        * inspector/InjectedScriptManager.cpp:
        (WebCore::InjectedScriptManager::discardInjectedScriptsFor):
        * inspector/InjectedScriptManager.h:
        * inspector/InspectorConsoleAgent.cpp:
        (WebCore::InspectorConsoleAgent::domWindowCleared):
        * inspector/InspectorConsoleAgent.h:
        * inspector/InspectorInstrumentation.cpp:
        (WebCore::InspectorInstrumentation::pageDestroyedImpl):
        * inspector/InspectorInstrumentation.h:
        (WebCore::InspectorInstrumentation::pageDestroyed):
        * page/DOMWindow.cpp:
        (WebCore::DOMWindow::pageDestroyed):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (89344 => 89345)


--- trunk/LayoutTests/ChangeLog	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/LayoutTests/ChangeLog	2011-06-21 09:58:15 UTC (rev 89345)
@@ -1,3 +1,14 @@
+2011-06-20  Yury Semikhatsky  <yu...@chromium.org>
+
+        Reviewed by Pavel Feldman.
+
+        Web Inspector: console messages shouldn't prevent garbage collection of iframes
+        https://bugs.webkit.org/show_bug.cgi?id=62996
+
+        * http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt: Added.
+        * http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html: Added.
+        * http/tests/inspector-enabled/resources/console-clear-arguments-iframe.html: Added.
+
 2011-06-21  Yuta Kitamura  <yu...@chromium.org>
 
         Unreviewed, update Chromium test expectations for some flaky tests.

Added: trunk/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt (0 => 89345)


--- trunk/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt	2011-06-21 09:58:15 UTC (rev 89345)
@@ -0,0 +1,9 @@
+CONSOLE MESSAGE: line 2: A message with first argument string
+CONSOLE MESSAGE: line 3: 2011
+CONSOLE MESSAGE: line 4: [object DOMWindow]
+Tests that console message arguments will be cleared when iframe where the messages were created is removed.
+
+resources/console-clear-arguments-iframe.html:2A message with first argument string
+resources/console-clear-arguments-iframe.html:32011
+resources/console-clear-arguments-iframe.html:4[object DOMWindow]
+
Property changes on: trunk/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt
___________________________________________________________________

Added: svn:eol-style

Added: trunk/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html (0 => 89345)


--- trunk/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html	2011-06-21 09:58:15 UTC (rev 89345)
@@ -0,0 +1,46 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<script>
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+function removeIFrame()
+{
+    var container = document.getElementById('container');
+    var child = container.firstChild;
+    while (child) {
+        container.removeChild(child);
+        child = container.firstChild;
+    }
+    if (window.layoutTestController)
+        layoutTestController.showWebInspector();
+    runTest();
+}
+
+function createIFrame()
+{
+    var iframe = document.createElement('iframe');
+    iframe.setAttribute('src', 'resources/console-clear-arguments-iframe.html');
+    iframe._onload_= removeIFrame;
+    document.getElementById('container').appendChild(iframe);
+}
+
+function test()
+{
+    InspectorTest.expandConsoleMessages();
+    InspectorTest.dumpConsoleMessages();
+    InspectorTest.completeTest();
+}
+
+</script>
+</head>
+<body _onload_="createIFrame()">
+<p>
+Tests that console message arguments will be cleared when iframe where the messages were
+created is removed.
+</p>
+<div id="container"/>
+</body>
+</html>
Property changes on: trunk/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html
___________________________________________________________________

Added: svn:eol-style

Added: trunk/LayoutTests/http/tests/inspector-enabled/resources/console-clear-arguments-iframe.html (0 => 89345)


--- trunk/LayoutTests/http/tests/inspector-enabled/resources/console-clear-arguments-iframe.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/inspector-enabled/resources/console-clear-arguments-iframe.html	2011-06-21 09:58:15 UTC (rev 89345)
@@ -0,0 +1,5 @@
+<script>
+console.log("A message with first argument string", "Second argument which should be discarded on iframe removal");
+console.log(2011, "A message with first argument integer");
+console.log(window, "A message with first argument window");
+</script>
Property changes on: trunk/LayoutTests/http/tests/inspector-enabled/resources/console-clear-arguments-iframe.html
___________________________________________________________________

Added: svn:eol-style

Modified: trunk/Source/WebCore/ChangeLog (89344 => 89345)


--- trunk/Source/WebCore/ChangeLog	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/ChangeLog	2011-06-21 09:58:15 UTC (rev 89345)
@@ -1,3 +1,39 @@
+2011-06-20  Yury Semikhatsky  <yu...@chromium.org>
+
+        Reviewed by Pavel Feldman.
+
+        Web Inspector: console messages shouldn't prevent garbage collection of iframes
+        https://bugs.webkit.org/show_bug.cgi?id=62996
+
+        When DOMWindow is detached from its frame run through all console messages and clear
+        their arguments and ScriptState references. The message text in this case will be
+        the first argument serialized to string.
+
+        Test: http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html
+
+        * bindings/js/ScriptState.cpp:
+        (WebCore::domWindowFromScriptState):
+        * bindings/js/ScriptState.h:
+        * bindings/v8/ScriptState.cpp:
+        (WebCore::ScriptState::domWindow):
+        (WebCore::domWindowFromScriptState):
+        * bindings/v8/ScriptState.h:
+        * inspector/ConsoleMessage.cpp:
+        (WebCore::ConsoleMessage::windowCleared):
+        * inspector/ConsoleMessage.h:
+        * inspector/InjectedScriptManager.cpp:
+        (WebCore::InjectedScriptManager::discardInjectedScriptsFor):
+        * inspector/InjectedScriptManager.h:
+        * inspector/InspectorConsoleAgent.cpp:
+        (WebCore::InspectorConsoleAgent::domWindowCleared):
+        * inspector/InspectorConsoleAgent.h:
+        * inspector/InspectorInstrumentation.cpp:
+        (WebCore::InspectorInstrumentation::pageDestroyedImpl):
+        * inspector/InspectorInstrumentation.h:
+        (WebCore::InspectorInstrumentation::pageDestroyed):
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::pageDestroyed):
+
 2011-06-21  Mikhail Naganov  <mnaga...@chromium.org>
 
         Reviewed by Yury Semikhatsky.

Modified: trunk/Source/WebCore/bindings/js/ScriptState.cpp (89344 => 89345)


--- trunk/Source/WebCore/bindings/js/ScriptState.cpp	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/bindings/js/ScriptState.cpp	2011-06-21 09:58:15 UTC (rev 89345)
@@ -59,6 +59,13 @@
     return 0;
 }
 
+DOMWindow* domWindowFromScriptState(ScriptState* scriptState)
+{
+    JSC::JSGlobalObject* globalObject = scriptState->lexicalGlobalObject();
+    if (!globalObject->inherits(&JSDOMWindowBase::s_info))
+        return 0;
+    return static_cast<JSDOMWindowBase*>(globalObject)->impl();
+}
 
 ScriptState* mainWorldScriptState(Frame* frame)
 {

Modified: trunk/Source/WebCore/bindings/js/ScriptState.h (89344 => 89345)


--- trunk/Source/WebCore/bindings/js/ScriptState.h	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/bindings/js/ScriptState.h	2011-06-21 09:58:15 UTC (rev 89345)
@@ -41,6 +41,7 @@
 }
 
 namespace WebCore {
+class DOMWindow;
 class DOMWrapperWorld;
 class Frame;
 class Node;
@@ -63,6 +64,8 @@
     JSC::Strong<JSC::JSGlobalObject> m_globalObject;
 };
 
+DOMWindow* domWindowFromScriptState(ScriptState*);
+
 ScriptState* mainWorldScriptState(Frame*);
 
 ScriptState* scriptStateFromNode(DOMWrapperWorld*, Node*);

Modified: trunk/Source/WebCore/bindings/v8/ScriptState.cpp (89344 => 89345)


--- trunk/Source/WebCore/bindings/v8/ScriptState.cpp	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/bindings/v8/ScriptState.cpp	2011-06-21 09:58:15 UTC (rev 89345)
@@ -35,6 +35,7 @@
 #include "Node.h"
 #include "Page.h"
 #include "ScriptController.h"
+#include "V8DOMWindow.h"
 #include "V8HiddenPropertyName.h"
 
 #include "WorkerContext.h"
@@ -58,6 +59,13 @@
     m_context.Clear();
 }
 
+DOMWindow* ScriptState::domWindow() const
+{
+    v8::HandleScope handleScope;
+    v8::Handle<v8::Object> v8RealGlobal = v8::Handle<v8::Object>::Cast(m_context->Global()->GetPrototype());
+    return V8DOMWindow::toNative(v8RealGlobal);
+}
+
 ScriptState* ScriptState::forContext(v8::Local<v8::Context> context)
 {
     v8::Context::Scope contextScope(context);
@@ -94,6 +102,11 @@
     delete scriptState;
 }
 
+DOMWindow* domWindowFromScriptState(ScriptState* scriptState)
+{
+    return scriptState->domWindow();
+}
+
 ScriptState* mainWorldScriptState(Frame* frame)
 {
     v8::HandleScope handleScope;

Modified: trunk/Source/WebCore/bindings/v8/ScriptState.h (89344 => 89345)


--- trunk/Source/WebCore/bindings/v8/ScriptState.h	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/bindings/v8/ScriptState.h	2011-06-21 09:58:15 UTC (rev 89345)
@@ -37,6 +37,7 @@
 #include <wtf/RefCounted.h>
 
 namespace WebCore {
+class DOMWindow;
 class DOMWrapperWorld;
 class Frame;
 class Node;
@@ -58,6 +59,8 @@
         return v8::Local<v8::Context>::New(m_context);
     }
 
+    DOMWindow* domWindow() const;
+
     static ScriptState* forContext(v8::Local<v8::Context>);
     static ScriptState* current();
 
@@ -104,6 +107,8 @@
     v8::Persistent<v8::Context> m_context;
 };
 
+DOMWindow* domWindowFromScriptState(ScriptState*);
+
 ScriptState* mainWorldScriptState(Frame*);
 
 ScriptState* scriptStateFromNode(DOMWrapperWorld*, Node*);

Modified: trunk/Source/WebCore/inspector/ConsoleMessage.cpp (89344 => 89345)


--- trunk/Source/WebCore/inspector/ConsoleMessage.cpp	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/inspector/ConsoleMessage.cpp	2011-06-21 09:58:15 UTC (rev 89345)
@@ -187,6 +187,17 @@
         && msg->m_requestId == m_requestId;
 }
 
+void ConsoleMessage::windowCleared(DOMWindow* window)
+{
+    if (!m_arguments)
+        return;
+    if (domWindowFromScriptState(m_arguments->globalState()) != window)
+        return;
+    if (!m_message)
+        m_message = "<message collected>";
+    m_arguments.clear();
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(INSPECTOR)

Modified: trunk/Source/WebCore/inspector/ConsoleMessage.h (89344 => 89345)


--- trunk/Source/WebCore/inspector/ConsoleMessage.h	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/inspector/ConsoleMessage.h	2011-06-21 09:58:15 UTC (rev 89345)
@@ -39,6 +39,7 @@
 
 namespace WebCore {
 
+class DOMWindow;
 class InjectedScriptManager;
 class InspectorFrontend;
 class InspectorObject;
@@ -64,6 +65,8 @@
     const String& message() const { return m_message; }
     MessageType type() const { return m_type; }
 
+    void windowCleared(DOMWindow*);
+
 private:
     MessageSource m_source;
     MessageType m_type;

Modified: trunk/Source/WebCore/inspector/InjectedScriptManager.cpp (89344 => 89345)


--- trunk/Source/WebCore/inspector/InjectedScriptManager.cpp	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/inspector/InjectedScriptManager.cpp	2011-06-21 09:58:15 UTC (rev 89345)
@@ -101,6 +101,22 @@
     m_idToInjectedScript.clear();
 }
 
+void InjectedScriptManager::discardInjectedScriptsFor(DOMWindow* window)
+{
+    Vector<long> idsToRemove;
+    IdToInjectedScriptMap::iterator end = m_idToInjectedScript.end();
+    for (IdToInjectedScriptMap::iterator it = m_idToInjectedScript.begin(); it != end; ++it) {
+        ScriptState* scriptState = it->second.scriptState();
+        if (window != domWindowFromScriptState(scriptState))
+            continue;
+        discardInjectedScript(scriptState);
+        idsToRemove.append(it->first);
+    }
+
+    for (size_t i = 0; i < idsToRemove.size(); i++)
+        m_idToInjectedScript.remove(idsToRemove[i]);
+}
+
 bool InjectedScriptManager::canAccessInspectedWorkerContext(ScriptState*)
 {
     return true;

Modified: trunk/Source/WebCore/inspector/InjectedScriptManager.h (89344 => 89345)


--- trunk/Source/WebCore/inspector/InjectedScriptManager.h	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/inspector/InjectedScriptManager.h	2011-06-21 09:58:15 UTC (rev 89345)
@@ -38,6 +38,7 @@
 
 namespace WebCore {
 
+class DOMWindow;
 class InjectedScript;
 class InjectedScriptHost;
 class InspectorObject;
@@ -59,6 +60,7 @@
     InjectedScript injectedScriptForId(long);
     InjectedScript injectedScriptForObjectId(const String& objectId);
     void discardInjectedScripts();
+    void discardInjectedScriptsFor(DOMWindow*);
     void releaseObjectGroup(const String& objectGroup);
 
 

Modified: trunk/Source/WebCore/inspector/InspectorConsoleAgent.cpp (89344 => 89345)


--- trunk/Source/WebCore/inspector/InspectorConsoleAgent.cpp	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/inspector/InspectorConsoleAgent.cpp	2011-06-21 09:58:15 UTC (rev 89345)
@@ -30,6 +30,7 @@
 #include "InstrumentingAgents.h"
 #include "Console.h"
 #include "ConsoleMessage.h"
+#include "DOMWindow.h"
 #include "InjectedScriptHost.h"
 #include "InjectedScriptManager.h"
 #include "InspectorAgent.h"
@@ -192,6 +193,14 @@
     addMessageToConsole(JSMessageSource, LogMessageType, LogMessageLevel, message, lastCaller.lineNumber(), lastCaller.sourceURL());
 }
 
+void InspectorConsoleAgent::frameWindowDiscarded(DOMWindow* window)
+{
+    size_t messageCount = m_consoleMessages.size();
+    for (size_t i = 0; i < messageCount; ++i)
+        m_consoleMessages[i]->windowCleared(window);
+    m_injectedScriptManager->discardInjectedScriptsFor(window);
+}
+
 void InspectorConsoleAgent::resourceRetrievedByXMLHttpRequest(const String& url, const String& sendURL, unsigned sendLineNumber)
 {
     if (!m_inspectorAgent->enabled())

Modified: trunk/Source/WebCore/inspector/InspectorConsoleAgent.h (89344 => 89345)


--- trunk/Source/WebCore/inspector/InspectorConsoleAgent.h	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/inspector/InspectorConsoleAgent.h	2011-06-21 09:58:15 UTC (rev 89345)
@@ -38,6 +38,7 @@
 #if ENABLE(INSPECTOR)
 
 class ConsoleMessage;
+class DOMWindow;
 class InspectorAgent;
 class InspectorDOMAgent;
 class InspectorFrontend;
@@ -72,6 +73,8 @@
     void stopTiming(const String& title, PassRefPtr<ScriptCallStack>);
     void count(PassRefPtr<ScriptArguments>, PassRefPtr<ScriptCallStack>);
 
+    void frameWindowDiscarded(DOMWindow*);
+
     void resourceRetrievedByXMLHttpRequest(const String& url, const String& sendURL, unsigned sendLineNumber);
     void didReceiveResponse(unsigned long identifier, const ResourceResponse&);
     void didFailLoading(unsigned long identifier, const ResourceError&);

Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp (89344 => 89345)


--- trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp	2011-06-21 09:58:15 UTC (rev 89345)
@@ -166,6 +166,12 @@
         domDebuggerAgent->didInvalidateStyleAttr(node);
 }
 
+void InspectorInstrumentation::frameWindowDiscardedImpl(InstrumentingAgents* instrumentingAgents, DOMWindow* window)
+{
+    if (InspectorConsoleAgent* consoleAgent = instrumentingAgents->inspectorConsoleAgent())
+        consoleAgent->frameWindowDiscarded(window);
+}
+
 void InspectorInstrumentation::mouseDidMoveOverElementImpl(InstrumentingAgents* instrumentingAgents, const HitTestResult& result, unsigned modifierFlags)
 {
     if (InspectorDOMAgent* domAgent = instrumentingAgents->inspectorDOMAgent())

Modified: trunk/Source/WebCore/inspector/InspectorInstrumentation.h (89344 => 89345)


--- trunk/Source/WebCore/inspector/InspectorInstrumentation.h	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/inspector/InspectorInstrumentation.h	2011-06-21 09:58:15 UTC (rev 89345)
@@ -41,6 +41,7 @@
 namespace WebCore {
 
 class CharacterData;
+class DOMWindow;
 class DOMWrapperWorld;
 class Database;
 class Element;
@@ -82,6 +83,7 @@
     static void didModifyDOMAttr(Document*, Element*);
     static void characterDataModified(Document*, CharacterData*);
     static void didInvalidateStyleAttr(Document*, Node*);
+    static void frameWindowDiscarded(Frame*, DOMWindow*);
 
     static void mouseDidMoveOverElement(Page*, const HitTestResult&, unsigned modifierFlags);
     static bool handleMousePress(Page*);
@@ -209,6 +211,7 @@
     static void didModifyDOMAttrImpl(InstrumentingAgents*, Element*);
     static void characterDataModifiedImpl(InstrumentingAgents*, CharacterData*);
     static void didInvalidateStyleAttrImpl(InstrumentingAgents*, Node*);
+    static void frameWindowDiscardedImpl(InstrumentingAgents*, DOMWindow*);
 
     static void mouseDidMoveOverElementImpl(InstrumentingAgents*, const HitTestResult&, unsigned modifierFlags);
     static bool handleMousePressImpl(InstrumentingAgents*);
@@ -398,6 +401,14 @@
 #endif
 }
 
+inline void InspectorInstrumentation::frameWindowDiscarded(Frame* frame, DOMWindow* domWindow)
+{
+#if ENABLE(INSPECTOR)
+    if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForFrame(frame))
+        frameWindowDiscardedImpl(instrumentingAgents, domWindow);
+#endif
+}
+
 inline void InspectorInstrumentation::mouseDidMoveOverElement(Page* page, const HitTestResult& result, unsigned modifierFlags)
 {
 #if ENABLE(INSPECTOR)

Modified: trunk/Source/WebCore/page/DOMWindow.cpp (89344 => 89345)


--- trunk/Source/WebCore/page/DOMWindow.cpp	2011-06-21 09:51:17 UTC (rev 89344)
+++ trunk/Source/WebCore/page/DOMWindow.cpp	2011-06-21 09:58:15 UTC (rev 89345)
@@ -709,6 +709,7 @@
 
 void DOMWindow::pageDestroyed()
 {
+    InspectorInstrumentation::frameWindowDiscarded(m_frame, this);
 #if ENABLE(NOTIFICATIONS)
     // Clearing Notifications requests involves accessing the client so it must be done
     // before the frame is detached.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to