Diff
Modified: trunk/LayoutTests/ChangeLog (188755 => 188756)
--- trunk/LayoutTests/ChangeLog 2015-08-21 16:18:52 UTC (rev 188755)
+++ trunk/LayoutTests/ChangeLog 2015-08-21 17:05:59 UTC (rev 188756)
@@ -1,3 +1,20 @@
+2015-08-20 Brian Burg <[email protected]>
+
+ Web Inspector: add a regression test for the fix introduced in r188679
+ https://bugs.webkit.org/show_bug.cgi?id=148223
+
+ Reviewed by Joseph Pecoraro.
+
+ Add a regression test to check the number of DOMTree instances that
+ listen to DOMTreeManager.
+
+ * TestExpectations: Don't skip the inspector/dom/ directory.
+ * http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation-expected.txt: Added.
+ * http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html: Added.
+ * http/tests/inspector/dom/resources/nested-frame-1-deep.html: Added.
+ * http/tests/inspector/dom/resources/nested-frame-2-deep.html: Added.
+ * http/tests/inspector/dom/resources/nested-frame-base.html: Added.
+
2015-08-21 Chris Dumez <[email protected]>
Regression(r188698): http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html is very flaky
Modified: trunk/LayoutTests/TestExpectations (188755 => 188756)
--- trunk/LayoutTests/TestExpectations 2015-08-21 16:18:52 UTC (rev 188755)
+++ trunk/LayoutTests/TestExpectations 2015-08-21 17:05:59 UTC (rev 188756)
@@ -97,7 +97,10 @@
webkit.org/b/129057 media/controls-styling-strict.html [ Pass Failure ]
# These tests will be rewritten, just skip them until that time.
-webkit.org/b/148036 http/tests/inspector/ [ Skip ]
+webkit.org/b/148036 http/tests/inspector/console/ [ Skip ]
+webkit.org/b/148036 http/tests/inspector/css/ [ Skip ]
+webkit.org/b/148036 http/tests/inspector/page/ [ Skip ]
+webkit.org/b/148036 http/tests/inspector/replay/ [ Skip ]
webkit.org/b/129639 inspector/dom/dom-search-crash.html [ Skip ]
Added: trunk/LayoutTests/http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation-expected.txt (0 => 188756)
--- trunk/LayoutTests/http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation-expected.txt 2015-08-21 17:05:59 UTC (rev 188756)
@@ -0,0 +1,19 @@
+Testing that DOMTrees are not retained by DOMTreeManager across main frame navigations.
+
+
+
+== Running test suite: DOM.DisconnectTreeOnMainResourceChange
+-- Running test case: CheckLazyInitializationOfDOMTree
+PASS: There should not be a DOMTree listening to DOMTreeManager events initially.
+DOMTree instance count: 0
+
+-- Running test case: CheckDOMTreeCountAfterUsingGetter
+PASS: There should be a one DOMTree listening to DOMTreeManager events after creation.
+DOMTree instance count: 1
+PASS: There should be four DOMTrees listening to DOMTreeManager events after touching each Frame.
+DOMTree instance count: 4
+
+-- Running test case: CheckDOMTreeCountAfterReloading
+PASS: There should not be any DOMTrees listening to DOMTreeManager events after a main frame navigation.
+DOMTree instance count: 0
+
Added: trunk/LayoutTests/http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html (0 => 188756)
--- trunk/LayoutTests/http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html (rev 0)
+++ trunk/LayoutTests/http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html 2015-08-21 17:05:59 UTC (rev 188756)
@@ -0,0 +1,68 @@
+<!doctype html>
+<html>
+<head>
+<script src=""
+<script>
+function test()
+{
+ let suite = InspectorTest.createAsyncSuite("DOM.DisconnectTreeOnMainResourceChange");
+
+ suite.addTestCase({
+ name: "CheckLazyInitializationOfDOMTree",
+ description: "Check that DOMTree instances are created lazily.",
+ test: (resolve, reject) => {
+ let instances = WebInspector.domTreeManager.retainedObjectsWithPrototype(WebInspector.DOMTree);
+ InspectorTest.expectThat(instances.size === 0, "There should not be a DOMTree listening to DOMTreeManager events initially.");
+ InspectorTest.log("DOMTree instance count: " + instances.size);
+ resolve();
+ }
+ });
+
+ suite.addTestCase({
+ name: "CheckDOMTreeCountAfterUsingGetter",
+ description: "Check that the count of connected DOMTrees is correct after initializing them.",
+ test: (resolve, reject) => {
+ let mainFrame = WebInspector.frameResourceManager.mainFrame;
+ mainFrame.domTree; // Force creation of the root DOM tree.
+
+ let instances = WebInspector.domTreeManager.retainedObjectsWithPrototype(WebInspector.DOMTree);
+ InspectorTest.expectThat(instances.size === 1, "There should be a one DOMTree listening to DOMTreeManager events after creation.");
+ InspectorTest.log("DOMTree instance count: " + instances.size);
+
+ // Force creation of child DOM trees.
+ mainFrame.childFrames[0].domTree;
+ mainFrame.childFrames[0].childFrames[0].domTree;
+ mainFrame.childFrames[0].childFrames[0].childFrames[0].domTree;
+
+ instances = WebInspector.domTreeManager.retainedObjectsWithPrototype(WebInspector.DOMTree);
+ InspectorTest.expectThat(instances.size === 4, "There should be four DOMTrees listening to DOMTreeManager events after touching each Frame.");
+ InspectorTest.log("DOMTree instance count: " + instances.size);
+
+ resolve();
+ }
+ });
+
+ suite.addTestCase({
+ name: "CheckDOMTreeCountAfterReloading",
+ description: "Check that the count of connected DOMTrees is correct after a reload.",
+ test: (resolve, reject) => {
+ WebInspector.Frame.singleFireEventListener(WebInspector.Frame.Event.MainResourceDidChange, (event) => {
+ let instances = WebInspector.domTreeManager.retainedObjectsWithPrototype(WebInspector.DOMTree);
+ InspectorTest.expectThat(instances.size === 0, "There should not be any DOMTrees listening to DOMTreeManager events after a main frame navigation.");
+ InspectorTest.log("DOMTree instance count: " + instances.size);
+ resolve();
+ });
+
+ InspectorTest.reloadPage();
+ }
+ });
+
+ suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body _onload_="runTest()">
+ <p>Testing that DOMTrees are not retained by DOMTreeManager across main frame navigations.</p>
+ <iframe src=""
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-1-deep.html (0 => 188756)
--- trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-1-deep.html (rev 0)
+++ trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-1-deep.html 2015-08-21 17:05:59 UTC (rev 188756)
@@ -0,0 +1,7 @@
+<!doctype html>
+<html>
+<body>
+<div id="frameLabel">1-deep nested frames page.</div>
+<iframe src=""
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-2-deep.html (0 => 188756)
--- trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-2-deep.html (rev 0)
+++ trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-2-deep.html 2015-08-21 17:05:59 UTC (rev 188756)
@@ -0,0 +1,7 @@
+<!doctype html>
+<html>
+<body>
+<div id="frameLabel">2-deep nested frames page.</div>
+<iframe src=""
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-base.html (0 => 188756)
--- trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-base.html (rev 0)
+++ trunk/LayoutTests/http/tests/inspector/dom/resources/nested-frame-base.html 2015-08-21 17:05:59 UTC (rev 188756)
@@ -0,0 +1,6 @@
+<!doctype html>
+<html>
+<body>
+<div id="frameLabel">Nested frame base page.</div>
+</body>
+</html>
Modified: trunk/Source/WebInspectorUI/ChangeLog (188755 => 188756)
--- trunk/Source/WebInspectorUI/ChangeLog 2015-08-21 16:18:52 UTC (rev 188755)
+++ trunk/Source/WebInspectorUI/ChangeLog 2015-08-21 17:05:59 UTC (rev 188756)
@@ -1,3 +1,22 @@
+2015-08-20 Brian Burg <[email protected]>
+
+ Web Inspector: add a regression test for the fix introduced in r188679
+ https://bugs.webkit.org/show_bug.cgi?id=148223
+
+ Reviewed by Joseph Pecoraro.
+
+ Turns out that we were possibly leaking a single DOMTree on reload.
+ Always clear the main frame's DOMTree as well as those of sub-frames.
+
+ * UserInterface/Base/Object.js: Add back single-fire listeners.
+ (WebInspector.Object.singleFireEventListener.let.wrappedCallback):
+ (WebInspector.Object.singleFireEventListener):
+ (WebInspector.Object.retainedObjectsWithPrototype):
+ (WebInspector.Object.prototype.singleFireEventListener):
+ (WebInspector.Object.prototype.retainedObjectsWithPrototype):
+ * UserInterface/Models/Frame.js:
+ (WebInspector.Frame.prototype.removeAllChildFrames): Clear own tree too.
+
2015-08-20 Joseph Pecoraro <[email protected]>
Web Inspector: Better Remember open tab types and order if using JSContext inspector
Modified: trunk/Source/WebInspectorUI/UserInterface/Base/Object.js (188755 => 188756)
--- trunk/Source/WebInspectorUI/UserInterface/Base/Object.js 2015-08-21 16:18:52 UTC (rev 188755)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/Object.js 2015-08-21 17:05:59 UTC (rev 188756)
@@ -55,6 +55,16 @@
listeners.push({thisObject, listener});
}
+ static singleFireEventListener(eventType, listener, thisObject)
+ {
+ let wrappedCallback = function() {
+ this.removeEventListener(eventType, wrappedCallback, null);
+ listener.apply(thisObject, arguments);
+ }.bind(this);
+
+ this.addEventListener(eventType, wrappedCallback, null);
+ }
+
static removeEventListener(eventType, listener, thisObject)
{
eventType = eventType || null;
@@ -100,12 +110,28 @@
return true;
}
+ // This should only be used within regression tests to detect leaks.
+ static retainedObjectsWithPrototype(proto)
+ {
+ let results = new Set;
+ for (let eventType in this._listeners) {
+ let recordsForEvent = this._listeners[eventType];
+ for (let listener of recordsForEvent) {
+ if (listener.thisObject instanceof proto)
+ results.add(listener.thisObject);
+ }
+ }
+ return results;
+ }
+
// Public
addEventListener() { return WebInspector.Object.addEventListener.apply(this, arguments); }
+ singleFireEventListener() { return WebInspector.Object.singleFireEventListener.apply(this, arguments); }
removeEventListener() { return WebInspector.Object.removeEventListener.apply(this, arguments); }
removeAllListeners() { return WebInspector.Object.removeAllListeners.apply(this, arguments); }
hasEventListeners() { return WebInspector.Object.hasEventListeners.apply(this, arguments); }
+ retainedObjectsWithPrototype() { return WebInspector.Object.retainedObjectsWithPrototype.apply(this, arguments); }
dispatchEventToListeners(eventType, eventData)
{
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/Frame.js (188755 => 188756)
--- trunk/Source/WebInspectorUI/UserInterface/Models/Frame.js 2015-08-21 16:18:52 UTC (rev 188755)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/Frame.js 2015-08-21 17:05:59 UTC (rev 188756)
@@ -338,11 +338,10 @@
removeAllChildFrames()
{
- if (!this._childFrames.length)
- return;
+ this._detachFromParentFrame();
for (let childFrame of this._childFrames)
- childFrame._detachFromParentFrame();
+ childFrame.removeAllChildFrames();
this._childFrames = [];
this._childFrameIdentifierMap = {};
