ScriptStateProtectedPtr should not keep a strong reference to the context

https://bugs.webkit.org/show_bug.cgi?id=85009

Source/WebCore:

Delete console message arguments when DOMWindow where the messages were created
is reset on its frame.

Reviewed by Pavel Feldman.

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

* inspector/ConsoleMessage.cpp:
(WebCore::ConsoleMessage::addToFrontend):
(WebCore::ConsoleMessage::windowCleared):
(WebCore::ConsoleMessage::argumentCount):
(WebCore):
* inspector/ConsoleMessage.h:
(ConsoleMessage):
* inspector/InspectorConsoleAgent.cpp:
(WebCore::InspectorConsoleAgent::consoleMessageArgumentCounts):
(WebCore):
* inspector/InspectorConsoleAgent.h:
(InspectorConsoleAgent):
* page/Frame.cpp:
(WebCore::Frame::clearDOMWindow):
(WebCore::Frame::setDOMWindow):
* testing/Internals.cpp:
(WebCore):
(WebCore::Internals::consoleMessageArgumentCounts):
* testing/Internals.h:
(Internals):
* testing/Internals.idl:

LayoutTests:

Test that after frame navigation all arguments passed to the console messages
created in that frame will be discarded and not prevent the frame context from
being GC'ed.

Reviewed by Pavel Feldman.

* http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation-expected.txt: Added.
* http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation.html: Added.
* http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt:
* http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html:
* http/tests/inspector-enabled/resources/console-clear-arguments-test.js: Added.
(print):
(dumpConsoleMessageArgumentCounts):
* http/tests/inspector/console-test.js:
(initialize_ConsoleTest.InspectorTest.checkConsoleMessagesDontHaveParameters):
(initialize_ConsoleTest):

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 1f00cee..0b53254 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,25 @@
+2012-04-27  Yury Semikhatsky  <yurys@chromium.org>
+
+        ScriptStateProtectedPtr should not keep a strong reference to the context
+        https://bugs.webkit.org/show_bug.cgi?id=85009
+
+        Test that after frame navigation all arguments passed to the console messages
+        created in that frame will be discarded and not prevent the frame context from
+        being GC'ed.
+
+        Reviewed by Pavel Feldman.
+
+        * http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation-expected.txt: Added.
+        * http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation.html: Added.
+        * http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt:
+        * http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html:
+        * http/tests/inspector-enabled/resources/console-clear-arguments-test.js: Added.
+        (print):
+        (dumpConsoleMessageArgumentCounts):
+        * http/tests/inspector/console-test.js:
+        (initialize_ConsoleTest.InspectorTest.checkConsoleMessagesDontHaveParameters):
+        (initialize_ConsoleTest):
+
 2012-04-28  Csaba Osztrogonác  <ossy@webkit.org>
 
         [Qt] Unreviewed weekend gardening, skip new failing tests.

diff --git a/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation-expected.txt b/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation-expected.txt
new file mode 100644 (file)
index 0000000..59d5bbb
--- /dev/null
+++ b/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation-expected.txt
@@ -0,0 +1,10 @@
+CONSOLE MESSAGE: line 2: A message with first argument string
+CONSOLE MESSAGE: line 3: 2011
+CONSOLE MESSAGE: line 4: [object Window]
+Tests that Web Inspector will discard console message arguments when iframe where the message was logged is navigated to a different page.
+
+
+PASSED: found argument counts for 3 messages
+PASSED: message #0 has no arguments.
+PASSED: message #1 has no arguments.
+PASSED: message #2 has no arguments.

diff --git a/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation.html b/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation.html
new file mode 100644 (file)
index 0000000..f587ba0
--- /dev/null
+++ b/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation.html
@@ -0,0 +1,36 @@
+<html>
+<head>
+<script src="resources/console-clear-arguments-test.js"></script>
+<script>
+
+if (window.layoutTestController) {
+    layoutTestController.waitUntilDone();
+    layoutTestController.dumpAsText();
+}
+
+var loadCount = 0;
+function frameLoaded(event) {
+    ++loadCount;
+    if (loadCount === 1) {
+        document.getElementById('theFrame').src = "about:blank";
+        return;
+    }
+
+    dumpConsoleMessageArgumentCounts();
+    if (window.layoutTestController)
+        layoutTestController.notifyDone();
+}
+
+</script>
+
+</head>
+<body>
+<p>
+Tests that Web Inspector will discard console message arguments when iframe where the message was
+logged is navigated to a different page.
+</p>
+<iframe id="theFrame" onload="frameLoaded(event)" src="resources/console-clear-arguments-iframe.html"></iframe>
+<div id="output"></div>
+</body>
+</html>
+

diff --git a/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt b/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt
index 71e4822..528c52a 100644 (file)
--- a/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt
+++ b/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove-expected.txt
@@ -3,7 +3,7 @@ CONSOLE MESSAGE: line 3: 2011
 CONSOLE MESSAGE: line 4: [object Window]
 Tests that console message arguments will be cleared when iframe where the messages were created is removed.
 
-A message with first argument string resources/console-clear-arguments-iframe.html:2
-2011 resources/console-clear-arguments-iframe.html:3
-[object Window] resources/console-clear-arguments-iframe.html:4
-
+PASSED: found argument counts for 3 messages
+PASSED: message #0 has no arguments.
+PASSED: message #1 has no arguments.
+PASSED: message #2 has no arguments.

diff --git a/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html b/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html
index e420fe1..0ca6397 100644 (file)
--- a/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html
+++ b/LayoutTests/http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html
@@ -1,10 +1,11 @@
 <html>
 <head>
-<script src="../inspector/inspector-test.js"></script>
-<script src="../inspector/console-test.js"></script>
+<script src="resources/console-clear-arguments-test.js"></script>
 <script>
-if (window.layoutTestController)
+if (window.layoutTestController) {
     layoutTestController.waitUntilDone();
+    layoutTestController.dumpAsText();
+}
 
 function removeIFrame()
 {
@@ -14,9 +15,9 @@ function removeIFrame()
         container.removeChild(child);
         child = container.firstChild;
     }
+    dumpConsoleMessageArgumentCounts();
     if (window.layoutTestController)
-        layoutTestController.showWebInspector();
-    runTest();
+        layoutTestController.notifyDone();
 }
 
 function createIFrame()
@@ -27,13 +28,6 @@ function createIFrame()
     document.getElementById('container').appendChild(iframe);
 }
 
-function test()
-{
-    InspectorTest.expandConsoleMessages();
-    InspectorTest.dumpConsoleMessages();
-    InspectorTest.completeTest();
-}
-
 </script>
 </head>
 <body onload="createIFrame()">
@@ -41,6 +35,7 @@ function test()
 Tests that console message arguments will be cleared when iframe where the messages were
 created is removed.
 </p>
-<div id="container"/>
+<div id="container"></div>
+<div id="output"></div>
 </body>
 </html>

diff --git a/LayoutTests/http/tests/inspector-enabled/resources/console-clear-arguments-test.js b/LayoutTests/http/tests/inspector-enabled/resources/console-clear-arguments-test.js
new file mode 100644 (file)
index 0000000..b0046dd
--- /dev/null
+++ b/LayoutTests/http/tests/inspector-enabled/resources/console-clear-arguments-test.js
@@ -0,0 +1,24 @@
+function print(text)
+{
+    var output = document.getElementById("output");
+    var div = document.createElement("div");
+    div.textContent = text;
+    output.appendChild(div);
+}
+
+function dumpConsoleMessageArgumentCounts()
+{
+    var consoleMessageArgumentCounts = window.internals.consoleMessageArgumentCounts(document);
+    if (consoleMessageArgumentCounts.length === 3)
+        print("PASSED: found argument counts for 3 messages");
+    else
+        print("FAILED: unexpected number of messages: " + consoleMessageArgumentCounts.length);
+
+    for (var i = 0; i < consoleMessageArgumentCounts.length; i++) {
+        var count = consoleMessageArgumentCounts[i];
+        if (count == 0)
+            print("PASSED: message #" + i + " has no arguments.");
+        else
+            print("FAILED: message #" + i + " has " + count + " arguments.");
+    }
+}

diff --git a/LayoutTests/http/tests/inspector/console-test.js b/LayoutTests/http/tests/inspector/console-test.js
index c5f0164..89259cb 100644 (file)
--- a/LayoutTests/http/tests/inspector/console-test.js
+++ b/LayoutTests/http/tests/inspector/console-test.js
@@ -44,4 +44,22 @@ InspectorTest.expandConsoleMessages = function()
     }
 }
 
+InspectorTest.checkConsoleMessagesDontHaveParameters = function()
+{
+    var messages = WebInspector.console.messages;
+    for (var i = 0; i < messages.length; ++i) {
+        var m = messages[i];
+        InspectorTest.addResult("Message[" + i + "]:");
+        InspectorTest.addResult("Message: " + WebInspector.displayNameForURL(m.url) + ":" + m.line + " " + m.message);
+        if ("_parameters" in m) {
+            if (m._parameters)
+                InspectorTest.addResult("FAILED: message parameters list is not empty: " + m._parameters);
+            else
+                InspectorTest.addResult("SUCCESS: message parameters list is empty. ");
+        } else {
+            InspectorTest.addResult("FAILED: didn't find _parameters field in the message.");
+        }
+    }
+}
+
 }

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index da0bec5..0ee2361 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,37 @@
+2012-04-27  Yury Semikhatsky  <yurys@chromium.org>
+
+        ScriptStateProtectedPtr should not keep a strong reference to the context
+        https://bugs.webkit.org/show_bug.cgi?id=85009
+
+        Delete console message arguments when DOMWindow where the messages were created
+        is reset on its frame.
+
+        Reviewed by Pavel Feldman.
+
+        Test: http/tests/inspector-enabled/console-clear-arguments-on-frame-navigation.html
+
+        * inspector/ConsoleMessage.cpp:
+        (WebCore::ConsoleMessage::addToFrontend):
+        (WebCore::ConsoleMessage::windowCleared):
+        (WebCore::ConsoleMessage::argumentCount):
+        (WebCore):
+        * inspector/ConsoleMessage.h:
+        (ConsoleMessage):
+        * inspector/InspectorConsoleAgent.cpp:
+        (WebCore::InspectorConsoleAgent::consoleMessageArgumentCounts):
+        (WebCore):
+        * inspector/InspectorConsoleAgent.h:
+        (InspectorConsoleAgent):
+        * page/Frame.cpp:
+        (WebCore::Frame::clearDOMWindow):
+        (WebCore::Frame::setDOMWindow):
+        * testing/Internals.cpp:
+        (WebCore):
+        (WebCore::Internals::consoleMessageArgumentCounts):
+        * testing/Internals.h:
+        (Internals):
+        * testing/Internals.idl:
+
 2012-04-27  Jochen Eisinger  <jochen@chromium.org>
 
         Ensure that there's always a provisional document loader if the frame loader is in provisional state

diff --git a/Source/WebCore/inspector/ConsoleMessage.cpp b/Source/WebCore/inspector/ConsoleMessage.cpp
index acbdde7..0aa9828 100644 (file)
--- a/Source/WebCore/inspector/ConsoleMessage.cpp
+++ b/Source/WebCore/inspector/ConsoleMessage.cpp
@@ -205,6 +205,13 @@ void ConsoleMessage::windowCleared(DOMWindow* window)
     m_arguments.clear();
 }
 
+unsigned ConsoleMessage::argumentCount()
+{
+    if (m_arguments)
+        return m_arguments->argumentCount();
+    return 0;
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(INSPECTOR)

diff --git a/Source/WebCore/inspector/ConsoleMessage.h b/Source/WebCore/inspector/ConsoleMessage.h
index 38f65c7..bd1abf5 100644 (file)
--- a/Source/WebCore/inspector/ConsoleMessage.h
+++ b/Source/WebCore/inspector/ConsoleMessage.h
@@ -67,6 +67,8 @@ public:
 
     void windowCleared(DOMWindow*);
 
+    unsigned argumentCount();
+
 private:
     MessageSource m_source;
     MessageType m_type;

diff --git a/Source/WebCore/inspector/InspectorConsoleAgent.cpp b/Source/WebCore/inspector/InspectorConsoleAgent.cpp
index 16c909e..270c4ae 100644 (file)
--- a/Source/WebCore/inspector/InspectorConsoleAgent.cpp
+++ b/Source/WebCore/inspector/InspectorConsoleAgent.cpp
@@ -148,6 +148,14 @@ void InspectorConsoleAgent::addMessageToConsole(MessageSource source, MessageTyp
     addConsoleMessage(adoptPtr(new ConsoleMessage(source, type, level, message, scriptId, lineNumber)));
 }
 
+Vector<unsigned> InspectorConsoleAgent::consoleMessageArgumentCounts()
+{
+    Vector<unsigned> result(m_consoleMessages.size());
+    for (size_t i = 0; i < m_consoleMessages.size(); i++)
+        result[i] = m_consoleMessages[i]->argumentCount();
+    return result;
+}
+
 void InspectorConsoleAgent::startTiming(const String& title)
 {
     // Follow Firebug's behavior of requiring a title that is not null or

diff --git a/Source/WebCore/inspector/InspectorConsoleAgent.h b/Source/WebCore/inspector/InspectorConsoleAgent.h
index 4ba4aaa..af7bd7b 100644 (file)
--- a/Source/WebCore/inspector/InspectorConsoleAgent.h
+++ b/Source/WebCore/inspector/InspectorConsoleAgent.h
@@ -69,6 +69,7 @@ public:
 
     void addMessageToConsole(MessageSource, MessageType, MessageLevel, const String& message, PassRefPtr<ScriptArguments>, PassRefPtr<ScriptCallStack>);
     void addMessageToConsole(MessageSource, MessageType, MessageLevel, const String& message, const String& scriptId, unsigned lineNumber);
+    Vector<unsigned> consoleMessageArgumentCounts();
 
     void startTiming(const String& title);
     void stopTiming(const String& title, PassRefPtr<ScriptCallStack>);

diff --git a/Source/WebCore/page/Frame.cpp b/Source/WebCore/page/Frame.cpp
index 5b9bfe8..a0a8211 100644 (file)
--- a/Source/WebCore/page/Frame.cpp
+++ b/Source/WebCore/page/Frame.cpp
@@ -592,8 +592,10 @@ void Frame::injectUserScriptsForWorld(DOMWrapperWorld* world, const UserScriptVe
 
 void Frame::clearDOMWindow()
 {
-    if (m_domWindow)
+    if (m_domWindow) {
+        InspectorInstrumentation::frameWindowDiscarded(this, m_domWindow.get());
         m_domWindow->clear();
+    }
     m_domWindow = 0;
 }
 
@@ -658,8 +660,10 @@ void Frame::clearTimers()
 
 void Frame::setDOMWindow(DOMWindow* domWindow)
 {
-    if (m_domWindow)
+    if (m_domWindow) {
+        InspectorInstrumentation::frameWindowDiscarded(this, m_domWindow.get());
         m_domWindow->clear();
+    }
     m_domWindow = domWindow;
 }
 

diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp
index 205b623..4c1d02c 100644 (file)
--- a/Source/WebCore/testing/Internals.cpp
+++ b/Source/WebCore/testing/Internals.cpp
@@ -42,9 +42,11 @@
 #include "HTMLInputElement.h"
 #include "HTMLNames.h"
 #include "HTMLTextAreaElement.h"
+#include "InspectorConsoleAgent.h"
 #include "InspectorController.h"
 #include "InspectorCounters.h"
 #include "InspectorInstrumentation.h"
+#include "InstrumentingAgents.h"
 #include "InternalSettings.h"
 #include "IntRect.h"
 #include "Language.h"
@@ -956,6 +958,21 @@ unsigned Internals::numberOfLiveDocuments() const
 {
     return InspectorCounters::counterValue(InspectorCounters::DocumentCounter);
 }
+
+Vector<String> Internals::consoleMessageArgumentCounts(Document* document) const
+{
+    InstrumentingAgents* instrumentingAgents = instrumentationForPage(document->page());
+    if (!instrumentingAgents)
+        return Vector<String>();
+    InspectorConsoleAgent* consoleAgent = instrumentingAgents->inspectorConsoleAgent();
+    if (!consoleAgent)
+        return Vector<String>();
+    Vector<unsigned> counts = consoleAgent->consoleMessageArgumentCounts();
+    Vector<String> result(counts.size());
+    for (size_t i = 0; i < counts.size(); i++)
+        result[i] = String::number(counts[i]);
+    return result;
+}
 #endif // ENABLE(INSPECTOR)
 
 bool Internals::hasGrammarMarker(Document* document, int from, int length, ExceptionCode&)

diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h
index 516ddeb..512f4d2 100644 (file)
--- a/Source/WebCore/testing/Internals.h
+++ b/Source/WebCore/testing/Internals.h
@@ -164,6 +164,7 @@ public:
 #if ENABLE(INSPECTOR)
     unsigned numberOfLiveNodes() const;
     unsigned numberOfLiveDocuments() const;
+    Vector<String> consoleMessageArgumentCounts(Document*) const;
 #endif
 
 #if ENABLE(FULLSCREEN_API)

diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl
index 9faf34e..7b4beb1 100644 (file)
--- a/Source/WebCore/testing/Internals.idl
+++ b/Source/WebCore/testing/Internals.idl
@@ -140,6 +140,7 @@ module window {
 
         [Conditional=INSPECTOR] unsigned long numberOfLiveNodes();
         [Conditional=INSPECTOR] unsigned long numberOfLiveDocuments();
+        [Conditional=INSPECTOR] sequence<String> consoleMessageArgumentCounts(in Document document);
 
 #if defined(ENABLE_FULLSCREEN_API) && ENABLE_FULLSCREEN_API
         void webkitWillEnterFullScreenForElement(in Document document, in Element element);