From 234d2247153c80fe9e372f4a431e98afdcd7b548 Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Thu, 5 Sep 2019 15:20:36 +0000 Subject: [PATCH] Bug 1578354 - Remove the obsolete condition so that JS frames are now properly filtered in in the call tree r=gregtatum Bug 1557789 added categories to all JS frames, so as a result JS frames where all filtered out because of the condition removed in this patch. This condition is basically a premature optimization, removing it shouldn't bring any behavior difference. Differential Revision: https://phabricator.services.mozilla.com/D44803 --HG-- extra : moz-landing-system : lando --- .../performance/modules/logic/frame-utils.js | 9 ++- .../test/browser_perf-tree-view-02.js | 69 +++++++------------ .../test/browser_perf-tree-view-03.js | 4 +- .../test/browser_perf-tree-view-04.js | 9 ++- .../test/browser_perf-tree-view-06.js | 8 +-- .../performance/test/helpers/synth-utils.js | 30 ++++---- .../test/unit/test_frame-utils-02.js | 6 +- 7 files changed, 60 insertions(+), 75 deletions(-) diff --git a/devtools/client/performance/modules/logic/frame-utils.js b/devtools/client/performance/modules/logic/frame-utils.js index 7d06a37ec2e1..f53fbde57488 100644 --- a/devtools/client/performance/modules/logic/frame-utils.js +++ b/devtools/client/performance/modules/logic/frame-utils.js @@ -206,11 +206,6 @@ function parseLocation(location, fallbackLine, fallbackColumn) { * @param {InflatedFrame} frame */ function computeIsContentAndCategory(frame) { - // Only C++ stack frames have associated category information. - if (frame.category !== null && frame.category !== undefined) { - return; - } - const location = frame.location; // There are 3 variants of location strings in the profiler (with optional @@ -247,6 +242,10 @@ function computeIsContentAndCategory(frame) { return; } + if (frame.category !== null && frame.category !== undefined) { + return; + } + if (schemeStartIndex !== 0) { for (let j = schemeStartIndex; j < location.length; j++) { if ( diff --git a/devtools/client/performance/test/browser_perf-tree-view-02.js b/devtools/client/performance/test/browser_perf-tree-view-02.js index 2b5e57445d9b..19dd75a7f4bc 100644 --- a/devtools/client/performance/test/browser_perf-tree-view-02.js +++ b/devtools/client/performance/test/browser_perf-tree-view-02.js @@ -168,7 +168,7 @@ add_task(function() { ); is( $fun(".call-tree-category", $$(".call-tree-item")[1]).textContent.trim(), - "Gecko", + "JIT", "The .A node's function cell displays the correct category." ); @@ -208,6 +208,27 @@ add_task(function() { "The .E node in the tree has the correct class name." ); + ok( + $fun(".call-tree-url", $$(".call-tree-item")[1]) + .getAttribute("tooltiptext") + .includes("http://foo/bar/baz"), + "The .A node's function cell displays the correct url tooltiptext." + ); + is( + $fun(".call-tree-line", $$(".call-tree-item")[1]).textContent.trim(), + ":12", + "The .A node's function cell displays the correct line." + ); + is( + $fun(".call-tree-column", $$(".call-tree-item")[1]).textContent.trim(), + ":9", + "The .A node's function cell displays the correct column." + ); + is( + $fun(".call-tree-host", $$(".call-tree-item")[1]).textContent.trim(), + "foo", + "The .A node's function cell displays the correct host." + ); is( $$dur(2).textContent.trim(), "15 ms", @@ -225,36 +246,19 @@ add_task(function() { ); is( $fun(".call-tree-name", $$(".call-tree-item")[2]).textContent.trim(), - "B", + "B InterruptibleLayout", "The .A.B node's function cell displays the correct name." ); is( - $fun(".call-tree-url", $$(".call-tree-item")[2]).textContent.trim(), - "baz", - "The .A.B node's function cell displays the correct url." - ); - ok( - $fun(".call-tree-url", $$(".call-tree-item")[2]) - .getAttribute("tooltiptext") - .includes("http://foo/bar/baz"), - "The .A.B node's function cell displays the correct url tooltiptext." - ); - is( - $fun(".call-tree-line", $$(".call-tree-item")[2]).textContent.trim(), - ":34", - "The .A.B node's function cell displays the correct line." - ); - is( - $fun(".call-tree-host", $$(".call-tree-item")[2]).textContent.trim(), - "foo", - "The .A.B node's function cell displays the correct host." + $fun(".call-tree-url", $$(".call-tree-item")[2]), + null, + "The .A.B node's function cell has no url." ); is( $fun(".call-tree-category", $$(".call-tree-item")[2]).textContent.trim(), "Layout", "The .A.B node's function cell displays the correct category." ); - is( $$dur(3).textContent.trim(), "5 ms", @@ -275,27 +279,6 @@ add_task(function() { "E", "The .A.E node's function cell displays the correct name." ); - is( - $fun(".call-tree-url", $$(".call-tree-item")[3]).textContent.trim(), - "baz", - "The .A.E node's function cell displays the correct url." - ); - ok( - $fun(".call-tree-url", $$(".call-tree-item")[3]) - .getAttribute("tooltiptext") - .includes("http://foo/bar/baz"), - "The .A.E node's function cell displays the correct url tooltiptext." - ); - is( - $fun(".call-tree-line", $$(".call-tree-item")[3]).textContent.trim(), - ":90", - "The .A.E node's function cell displays the correct line." - ); - is( - $fun(".call-tree-host", $$(".call-tree-item")[3]).textContent.trim(), - "foo", - "The .A.E node's function cell displays the correct host." - ); is( $fun(".call-tree-category", $$(".call-tree-item")[3]).textContent.trim(), "GC", diff --git a/devtools/client/performance/test/browser_perf-tree-view-03.js b/devtools/client/performance/test/browser_perf-tree-view-03.js index cf80d810c83a..b69a71d51b7c 100644 --- a/devtools/client/performance/test/browser_perf-tree-view-03.js +++ b/devtools/client/performance/test/browser_perf-tree-view-03.js @@ -101,12 +101,12 @@ add_task(function() { ); is( $$nam(2).textContent.trim(), - "B", + "B InterruptibleLayout", "The .A.B node's function cell displays the correct name." ); is( $$nam(3).textContent.trim(), - "D", + "D INTER_SLICE_GC", "The .A.B.D node's function cell displays the correct name." ); is( diff --git a/devtools/client/performance/test/browser_perf-tree-view-04.js b/devtools/client/performance/test/browser_perf-tree-view-04.js index 80c9930403bd..f3281634def1 100644 --- a/devtools/client/performance/test/browser_perf-tree-view-04.js +++ b/devtools/client/performance/test/browser_perf-tree-view-04.js @@ -56,6 +56,11 @@ add_task(function() { const B = A.getChild(); const D = B.getChild(); + is( + A.target.getAttribute("tooltiptext"), + "A (http://foo/bar/baz:12:9)", + "The .A node's 'tooltiptext' attribute is correct" + ); is( D.target.getAttribute("origin"), "chrome", @@ -68,7 +73,7 @@ add_task(function() { ); is( D.target.getAttribute("tooltiptext"), - "D (http://foo/bar/baz:78:9)", + "D INTER_SLICE_GC", "The .A.B.D node's 'tooltiptext' attribute is correct." ); @@ -108,7 +113,7 @@ add_task(function() { "The sixth column displayed for tree items is correct." ); - const functionCell = D.target.childNodes[5]; + const functionCell = A.target.childNodes[5]; is( functionCell.childNodes.length, diff --git a/devtools/client/performance/test/browser_perf-tree-view-06.js b/devtools/client/performance/test/browser_perf-tree-view-06.js index ad5ec5281a2d..859fbb676519 100644 --- a/devtools/client/performance/test/browser_perf-tree-view-06.js +++ b/devtools/client/performance/test/browser_perf-tree-view-06.js @@ -36,8 +36,6 @@ add_task(async function() { treeRoot.attachTo(container); const A = treeRoot.getChild(); - const B = A.getChild(); - const D = B.getChild(); let linkEvent = null; const handler = e => { @@ -47,18 +45,18 @@ add_task(async function() { treeRoot.on("link", handler); // Fire right click. - rightMousedown(D.target.querySelector(".call-tree-url")); + rightMousedown(A.target.querySelector(".call-tree-url")); // Ensure link was not called for right click. await idleWait(100); ok(!linkEvent, "The `link` event not fired for right click."); // Fire left click. - mousedown(D.target.querySelector(".call-tree-url")); + mousedown(A.target.querySelector(".call-tree-url")); // Ensure link was called for left click. await waitUntil(() => linkEvent); - is(linkEvent, D, "The `link` event target is correct."); + is(linkEvent, A, "The `link` event target is correct."); treeRoot.off("link", handler); }); diff --git a/devtools/client/performance/test/helpers/synth-utils.js b/devtools/client/performance/test/helpers/synth-utils.js index 15a6e6113ed2..6e8438d79d1d 100644 --- a/devtools/client/performance/test/helpers/synth-utils.js +++ b/devtools/client/performance/test/helpers/synth-utils.js @@ -21,12 +21,12 @@ exports.synthesizeProfile = () => { frames: [ { category: CATEGORY_INDEX("other"), location: "(root)" }, { - category: CATEGORY_INDEX("other"), - location: "A (http://foo/bar/baz:12)", + category: CATEGORY_INDEX("js"), + location: "A (http://foo/bar/baz:12:9)", }, { category: CATEGORY_INDEX("layout"), - location: "B (http://foo/bar/baz:34)", + location: "B InterruptibleLayout", }, { category: CATEGORY_INDEX("js"), @@ -39,16 +39,16 @@ exports.synthesizeProfile = () => { frames: [ { category: CATEGORY_INDEX("other"), location: "(root)" }, { - category: CATEGORY_INDEX("other"), - location: "A (http://foo/bar/baz:12)", + category: CATEGORY_INDEX("js"), + location: "A (http://foo/bar/baz:12:9)", }, { category: CATEGORY_INDEX("layout"), - location: "B (http://foo/bar/baz:34)", + location: "B InterruptibleLayout", }, { category: CATEGORY_INDEX("gc"), - location: "D (http://foo/bar/baz:78:9)", + location: "D INTER_SLICE_GC", }, ], }, @@ -57,16 +57,16 @@ exports.synthesizeProfile = () => { frames: [ { category: CATEGORY_INDEX("other"), location: "(root)" }, { - category: CATEGORY_INDEX("other"), - location: "A (http://foo/bar/baz:12)", + category: CATEGORY_INDEX("js"), + location: "A (http://foo/bar/baz:12:9)", }, { category: CATEGORY_INDEX("layout"), - location: "B (http://foo/bar/baz:34)", + location: "B InterruptibleLayout", }, { category: CATEGORY_INDEX("gc"), - location: "D (http://foo/bar/baz:78:9)", + location: "D INTER_SLICE_GC", }, ], }, @@ -75,16 +75,16 @@ exports.synthesizeProfile = () => { frames: [ { category: CATEGORY_INDEX("other"), location: "(root)" }, { - category: CATEGORY_INDEX("other"), - location: "A (http://foo/bar/baz:12)", + category: CATEGORY_INDEX("js"), + location: "A (http://foo/bar/baz:12:9)", }, { category: CATEGORY_INDEX("gc"), - location: "E (http://foo/bar/baz:90)", + location: "E", }, { category: CATEGORY_INDEX("network"), - location: "F (http://foo/bar/baz:99)", + location: "F", }, ], }, diff --git a/devtools/client/performance/test/unit/test_frame-utils-02.js b/devtools/client/performance/test/unit/test_frame-utils-02.js index 9a568adf15bd..9d607c40463a 100644 --- a/devtools/client/performance/test/unit/test_frame-utils-02.js +++ b/devtools/client/performance/test/unit/test_frame-utils-02.js @@ -73,15 +73,15 @@ add_task(function() { ); ok( - !isContent({ category: 1, location: "file://foo -> http://bar" }), + isContent({ category: 1, location: "file://foo -> http://bar" }), "Verifying content/chrome frames is working properly." ); ok( - !isContent({ category: 1, location: "file://foo -> https://bar" }), + isContent({ category: 1, location: "file://foo -> https://bar" }), "Verifying content/chrome frames is working properly." ); ok( - !isContent({ category: 1, location: "file://foo -> file://bar" }), + isContent({ category: 1, location: "file://foo -> file://bar" }), "Verifying content/chrome frames is working properly." ); });