Bug 1474844 - Filter out empty categories - r?baku draft
authorTarek Ziadé <tarek@mozilla.com>
Fri, 13 Jul 2018 11:57:59 +0200
changeset 817778 3b64203285bb6be2ebec37aaf64f78b98da25f07
parent 817583 96c61b1dd0a1ecbc37fd4e257ff0bdff6f56df8b
push id116159
push usertziade@mozilla.com
push dateFri, 13 Jul 2018 12:38:54 +0000
reviewersbaku
bugs1474844
milestone63.0a1
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
dom/base/DocGroup.cpp
dom/tests/browser/browser.ini
dom/tests/browser/browser_test_performance_metrics.js
dom/tests/browser/ping_worker.html
dom/tests/browser/ping_worker2.html
dom/tests/browser/shared_worker.js
dom/workers/WorkerDebugger.cpp
toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp
--- a/dom/base/DocGroup.cpp
+++ b/dom/base/DocGroup.cpp
@@ -73,68 +73,69 @@ DocGroup::ReportPerformanceInfo()
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(mPerformanceCounter);
 #if defined(XP_WIN)
   uint32_t pid = GetCurrentProcessId();
 #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;
   nsCString host;
 
   // iterating on documents until we find the top window
   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;
     }
     nsPIDOMWindowOuter* outer = win->GetOuterWindow();
     if (!outer) {
       continue;
     }
     nsCOMPtr<nsPIDOMWindowOuter> top = outer->GetTop();
     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;
 
   // now that we have the host and window ids, let's look at the perf counters
   for (uint32_t index = 0; index < (uint32_t)TaskCategory::Count; index++) {
     TaskCategory category = static_cast<TaskCategory>(index);
     count = mPerformanceCounter->GetDispatchCount(DispatchCategory(category));
     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
 DocGroup::Dispatch(TaskCategory aCategory,
                    already_AddRefed<nsIRunnable>&& aRunnable)
 {
   if (mPerformanceCounter) {
     mPerformanceCounter->IncrementDispatchCounter(DispatchCategory(aCategory));
--- a/dom/tests/browser/browser.ini
+++ b/dom/tests/browser/browser.ini
@@ -6,16 +6,17 @@ support-files =
   page_localstorage_e10s.html
   position.html
   test-console-api.html
   test_bug1004814.html
   worker_bug1004814.js
   geo_leak_test.html
   dummy.html
   ping_worker.html
+  ping_worker2.html
   test_largeAllocation.html
   test_largeAllocation.html^headers^
   test_largeAllocation2.html
   test_largeAllocation2.html^headers^
   test_largeAllocationFormSubmit.sjs
   helper_largeAllocation.js
   !/dom/tests/mochitest/geolocation/network_geolocation.sjs
 
--- a/dom/tests/browser/browser_test_performance_metrics.js
+++ b/dom/tests/browser/browser_test_performance_metrics.js
@@ -2,16 +2,17 @@
 /* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 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;
 
 function jsonrpc(tab, method, params) {
   let currentId = nextId++;
   let messageManager = tab.linkedBrowser.messageManager;
   messageManager.sendAsyncMessage("jsonrpc", {
@@ -47,31 +48,47 @@ add_task(async function test() {
   let page1 = await BrowserTestUtils.openNewForegroundTab({
     gBrowser, opening: "about:about", forceNewProcess: false
   });
 
   let page2 = await BrowserTestUtils.openNewForegroundTab({
     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;
     let workerTotal = 0;
     let duration = 0;
     let total = 0;
     let isTopLevel = false;
     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;
         }
         if (entry.host == "about:memory") {
           aboutMemoryFound = true;
         }
         if (entry.pid == Services.appinfo.processID) {
           parentProcessEvent = true;
@@ -79,16 +96,17 @@ add_task(async function test() {
         if (entry.isWorker) {
           workerEvent = true;
           workerDuration += entry.duration;
         } else {
           duration += entry.duration;
         }
         // 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 {
             total += item.count;
           }
         }
       }
     }
@@ -99,16 +117,22 @@ add_task(async function test() {
 
     Assert.ok(workerDuration > 0, "Worker duration should be positive");
     Assert.ok(workerTotal > 0, "Worker count should be positive");
     Assert.ok(duration > 0, "Duration should be positive");
     Assert.ok(total > 0, "Should get a positive count");
     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;
     let previousWorkerTotal = workerTotal;
     let previousDuration = duration;
     let previousTotal = total;
 
     results = await ChromeUtils.requestPerformanceMetrics();
@@ -117,10 +141,11 @@ add_task(async function test() {
     Assert.ok(workerDuration > previousWorkerDuration, "Worker duration should be positive");
     Assert.ok(workerTotal > previousWorkerTotal, "Worker count should be positive");
     Assert.ok(duration > previousDuration, "Duration should be positive");
     Assert.ok(total > previousTotal, "Should get a positive count");
   });
 
   BrowserTestUtils.removeTab(page1);
   BrowserTestUtils.removeTab(page2);
+  BrowserTestUtils.removeTab(page3);
   SpecialPowers.clearUserPref("dom.performance.enable_scheduler_timing");
 });
--- a/dom/tests/browser/ping_worker.html
+++ b/dom/tests/browser/ping_worker.html
@@ -1,18 +1,26 @@
 <!DOCTYPE html>
 <html lang="en" dir="ltr">
 <head>
   <meta charset="utf-8">
   <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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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]);
+  }
+}
--- a/dom/workers/WorkerDebugger.cpp
+++ b/dom/workers/WorkerDebugger.cpp
@@ -480,48 +480,54 @@ WorkerDebugger::ReportPerformanceInfo()
   AssertIsOnMainThread();
 
 #if defined(XP_WIN)
   uint32_t pid = GetCurrentProcessId();
 #else
   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
   // struct is common to all performance counters throughout Firefox.
   FallibleTArray<CategoryDispatch> items;
   uint64_t duration = 0;
   uint16_t count = 0;
-  RefPtr<nsIURI> uri = mWorkerPrivate->GetResolvedScriptURI();
 
   RefPtr<PerformanceCounter> perf = mWorkerPrivate->GetPerformanceCounter();
   if (perf) {
     count =  perf->GetTotalDispatchCount();
     duration = perf->GetExecutionDuration();
     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
 } // mozilla namespace
--- a/toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp
+++ b/toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp
@@ -58,23 +58,27 @@ AggregatedResults::AppendResult(const ns
   }
   MOZ_ASSERT(mPendingResults > 0);
 
   // Each PerformanceInfo is converted into a PerformanceInfoDictionary
   for (const PerformanceInfo& result : 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);
     if (NS_WARN_IF(!data)) {
       Abort(NS_ERROR_OUT_OF_MEMORY);
       return;
     }
     data->mPid = result.pid();