Bug 1474844 - Filter out empty categories - r=baku

Filters out empty categories when ChromeUtils.requestPerformanceMetrics() is called.

This test also:

- adds more test coverage
- uses the worker windowId when it has no linked window.
- properly walk to the worker parent

MozReview-Commit-ID: 3UH9a0UtVmx

--HG--
extra : rebase_source : 337b95466c7e7a30f881e881358d3b8d290f8f5b

dom/base/DocGroup.cpp

@@ -78,7 +78,7 @@ DocGroup::ReportPerformanceInfo()
 #else
   uint32_t pid = getpid();
 #endif
-  uint64_t pwid = 0;
+  uint64_t windowID = 0;
   uint16_t count = 0;
   uint64_t duration = 0;
   bool isTopLevel = false;
@@ -88,8 +88,17 @@ DocGroup::ReportPerformanceInfo()
   for (const auto& document : *this) {
     nsCOMPtr<nsIDocument> doc = do_QueryInterface(document);
     MOZ_ASSERT(doc);
+    nsCOMPtr<nsIURI> docURI = doc->GetDocumentURI();
+    if (!docURI) {
+      continue;
+    }
+    docURI->GetHost(host);
+    // If the host is empty, using the url
+    if (host.IsEmpty()) {
+      host = docURI->GetSpecOrDefault();
+    }
     // looking for the top level document URI
-    nsPIDOMWindowInner* win = doc->GetInnerWindow();
+    nsPIDOMWindowOuter* win = doc->GetWindow();
     if (!win) {
       continue;
     }
@@ -101,20 +110,12 @@ DocGroup::ReportPerformanceInfo()
     if (!top) {
       continue;
     }
-    nsCOMPtr<nsIURI> docURI = doc->GetDocumentURI();
-    if (!docURI) {
-      continue;
-    }
-    pwid = top->WindowID();
+    windowID = top->WindowID();
     isTopLevel = outer->IsTopLevelWindow();
-    docURI->GetHost(host);
-    // If the host is empty, using the url
-    if (host.IsEmpty()) {
-      docURI->GetSpec(host);
-    }
     break;
   }
 
+  MOZ_ASSERT(!host.IsEmpty());
   duration = mPerformanceCounter->GetExecutionDuration();
   FallibleTArray<CategoryDispatch> items;
 
@@ -125,11 +126,11 @@ DocGroup::ReportPerformanceInfo()
     CategoryDispatch item = CategoryDispatch(index, count);
     if (!items.AppendElement(item, fallible)) {
       NS_ERROR("Could not complete the operation");
-      return PerformanceInfo(host, pid, pwid, duration, false, isTopLevel, items);
+      return PerformanceInfo(host, pid, windowID, duration, false, isTopLevel, items);
     }
   }
 
-  return PerformanceInfo(host, pid, pwid, duration, false, isTopLevel, items);
+  return PerformanceInfo(host, pid, windowID, duration, false, isTopLevel, items);
 }
 
 nsresult

dom/tests/browser/browser.ini

@@ -11,6 +11,7 @@ support-files =
   geo_leak_test.html
   dummy.html
   ping_worker.html
+  ping_worker2.html
   test_largeAllocation.html
   test_largeAllocation.html^headers^
   test_largeAllocation2.html

dom/tests/browser/browser_test_performance_metrics.js

@@ -7,6 +7,7 @@
 const ROOT_URL = "http://example.com/browser/dom/tests/browser";
 const DUMMY_URL = ROOT_URL + "/dummy.html";
 const WORKER_URL = ROOT_URL + "/ping_worker.html";
+const WORKER_URL2 = ROOT_URL + "/ping_worker2.html";
 
 
 let nextId = 0;
@@ -52,8 +53,11 @@ add_task(async function test() {
     gBrowser, opening: "about:memory", forceNewProcess: false
   });
 
+  let page3 = await BrowserTestUtils.openNewForegroundTab({
+    gBrowser, opening: WORKER_URL
+  });
   // load a 4th tab with a worker
-  await BrowserTestUtils.withNewTab({ gBrowser, url: WORKER_URL },
+  await BrowserTestUtils.withNewTab({ gBrowser, url: WORKER_URL2 },
     async function(browser) {
     // grab events..
     let workerDuration = 0;
@@ -64,9 +68,22 @@ add_task(async function test() {
     let aboutMemoryFound = false;
     let parentProcessEvent = false;
     let workerEvent = false;
+    let subFrameIds = [];
+    let topLevelIds = [];
+    let sharedWorker = false;
 
     function exploreResults(data) {
       for (let entry of data) {
+        sharedWorker = entry.host.endsWith("shared_worker.js") || sharedWorker;
+
+        Assert.ok(entry.host != "" || entry.windowId !=0,
+                  "An entry should have a host or a windowId");
+        if (entry.windowId != 0 && !entry.isToplevel && !entry.isWorker && !subFrameIds.includes(entry.windowId)) {
+          subFrameIds.push(entry.windowId);
+        }
+        if (entry.isTopLevel && !topLevelIds.includes(entry.windowId)) {
+          topLevelIds.push(entry.windowId);
+        }
         if (entry.host == "example.com" && entry.isTopLevel) {
           isTopLevel = true;
         }
@@ -84,6 +101,7 @@ add_task(async function test() {
         }
         // let's look at the data we got back
         for (let item of entry.items) {
+          Assert.ok(item.count > 0, "Categories with an empty count are dropped");
           if (entry.isWorker) {
             workerTotal += item.count;
           } else {
@@ -104,6 +122,12 @@ add_task(async function test() {
     Assert.ok(parentProcessEvent, "parent process sent back some events");
     Assert.ok(isTopLevel, "example.com as a top level window");
     Assert.ok(aboutMemoryFound, "about:memory");
+    Assert.ok(sharedWorker, "We got some info from a shared worker");
+
+    // checking that subframes are not orphans
+    for (let frameId of subFrameIds) {
+      Assert.ok(topLevelIds.includes(frameId), "subframe is not orphan ");
+    }
 
     // Doing a second call, we shoud get bigger values
     let previousWorkerDuration = workerDuration;
@@ -122,5 +146,6 @@ add_task(async function test() {
 
   BrowserTestUtils.removeTab(page1);
   BrowserTestUtils.removeTab(page2);
+  BrowserTestUtils.removeTab(page3);
   SpecialPowers.clearUserPref("dom.performance.enable_scheduler_timing");
 });

dom/tests/browser/ping_worker.html

@@ -5,14 +5,22 @@
   <script type="text/javascript">
 
   var myWorker;
+  var shared;
+
   function init() {
    myWorker = new Worker('ping_worker.js');
    for (let i = 0; i++; i < 10) myWorker.postMessage("ping");
+
+   shared = new SharedWorker('shared_worker.js');
+   shared.port.start();
+   shared.port.onmessage = function(e) {
+     console.log(e);
+   }
   }
 
   </script>
 </head>
 <body onload="init()">
-  <h1>A page with a worker</h1>
+  <h1>A page with a worker and a shared worker</h1>
 </body>
 </html>

dom/tests/browser/ping_worker2.html

@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+<head>
+  <meta charset="utf-8">
+  <script type="text/javascript">
+
+  var shared;
+
+  function init() {
+   shared = new SharedWorker('shared_worker.js');
+   shared.port.start();
+   for (let i = 0; i++; i < 10) shared.port.postMessage(["ok"]);
+  }
+
+  </script>
+</head>
+<body onload="init()">
+  <h1>A page with a shared worker</h1>
+</body>
+</html>

dom/tests/browser/shared_worker.js

@@ -0,0 +1,9 @@
+
+onconnect = function(e) {
+  var port = e.ports[0];
+
+  port.onmessage = function(e) {
+    var workerResult = 'Result: ' + (e.data[0] * e.data[1]);
+    port.postMessage(e.data[0]);
+  }
+}

dom/workers/WorkerDebugger.cpp

@@ -485,19 +485,28 @@ WorkerDebugger::ReportPerformanceInfo()
   uint32_t pid = getpid();
 #endif
   bool isTopLevel= false;
-  uint64_t pwid = 0;
-  nsPIDOMWindowInner* win = mWorkerPrivate->GetWindow();
+  uint64_t windowID = mWorkerPrivate->WindowID();
+
+  // Walk up to our containing page and its window
+  WorkerPrivate* wp = mWorkerPrivate;
+  while (wp->GetParent()) {
+    wp = wp->GetParent();
+  }
+  nsPIDOMWindowInner* win = wp->GetWindow();
   if (win) {
     nsPIDOMWindowOuter* outer = win->GetOuterWindow();
     if (outer) {
       nsCOMPtr<nsPIDOMWindowOuter> top = outer->GetTop();
       if (top) {
-        pwid = top->WindowID();
-        isTopLevel = pwid == mWorkerPrivate->WindowID();
+        windowID = top->WindowID();
+        isTopLevel = outer->IsTopLevelWindow();
       }
     }
   }
 
+  // getting the worker URL
+  RefPtr<nsIURI> scriptURI = mWorkerPrivate->GetResolvedScriptURI();
+  nsCString url = scriptURI->GetSpecOrDefault();
 
   // Workers only produce metrics for a single category - DispatchCategory::Worker.
   // We still return an array of CategoryDispatch so the PerformanceInfo
@@ -505,7 +514,6 @@ WorkerDebugger::ReportPerformanceInfo()
   FallibleTArray<CategoryDispatch> items;
   uint64_t duration = 0;
   uint16_t count = 0;
-  RefPtr<nsIURI> uri = mWorkerPrivate->GetResolvedScriptURI();
 
   RefPtr<PerformanceCounter> perf = mWorkerPrivate->GetPerformanceCounter();
   if (perf) {
@@ -514,13 +522,11 @@ WorkerDebugger::ReportPerformanceInfo()
     CategoryDispatch item = CategoryDispatch(DispatchCategory::Worker.GetValue(), count);
     if (!items.AppendElement(item, fallible)) {
       NS_ERROR("Could not complete the operation");
-      return PerformanceInfo(uri->GetSpecOrDefault(), pid, pwid, duration,
-                            true, isTopLevel, items);
+      return PerformanceInfo(url, pid, windowID, duration, true, isTopLevel, items);
     }
   }
 
-  return PerformanceInfo(uri->GetSpecOrDefault(), pid, pwid, duration,
-                         true, isTopLevel, items);
+  return PerformanceInfo(url, pid, windowID, duration, true, isTopLevel, items);
 }
 
 } // dom namespace

toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp

@@ -63,13 +63,17 @@ AggregatedResults::AppendResult(const nsTArray<dom::PerformanceInfo>& aMetrics)
     mozilla::dom::Sequence<mozilla::dom::CategoryDispatchDictionary> items;
 
     for (const CategoryDispatch& entry : result.items()) {
+      uint32_t count = entry.count();
+      if (count == 0) {
+        continue;
+      }
       CategoryDispatchDictionary* item = items.AppendElement(fallible);
       if (NS_WARN_IF(!item)) {
         Abort(NS_ERROR_OUT_OF_MEMORY);
         return;
       }
       item->mCategory = entry.category();
-      item->mCount = entry.count();
+      item->mCount = count;
     }
 
     PerformanceInfoDictionary* data = mData.AppendElement(fallible);