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

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 (

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",

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(

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,

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);
 });

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",
               },
             ],
           },

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."
   );
 });