Bug 1491816 - deal with unresponsive content processes in ChromeUtils.requestPerformanceMetrics() - r=baku
authorTarek Ziadé <tarek@mozilla.com>
Thu, 11 Oct 2018 09:40:23 +0000
changeset 499135 8958a60dd82423a4689837029be94e6d032f6b8c
parent 499134 57d97b765e4913a0abecea6444eb4e9bd43025cd
child 499136 8da12a6048fba4e59a860386b075f4d9070f79bf
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1491816
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1491816 - deal with unresponsive content processes in ChromeUtils.requestPerformanceMetrics() - r=baku Adds a timout that will resolve the promise to return even if we did not get an answer from all children. MozReview-Commit-ID: FFLwAUkkYos Differential Revision: https://phabricator.services.mozilla.com/D7265
dom/tests/browser/perfmetrics/browser.ini
dom/tests/browser/perfmetrics/browser_test_unresponsive.js
dom/tests/browser/perfmetrics/unresponsive.html
modules/libpref/init/StaticPrefList.h
toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp
toolkit/components/perfmonitoring/PerformanceMetricsCollector.h
--- a/dom/tests/browser/perfmetrics/browser.ini
+++ b/dom/tests/browser/perfmetrics/browser.ini
@@ -1,12 +1,19 @@
 [DEFAULT]
-prefs = dom.performance.enable_scheduler_timing=true
+prefs =
+  dom.performance.enable_scheduler_timing=true
+  dom.performance.children_results_ipc_timeout=500
+
 support-files =
   dummy.html
   ping_worker.html
   ping_worker2.html
   ping_worker.js
   setinterval.html
   settimeout.html
+  unresponsive.html
 
 [browser_test_performance_metrics.js]
 skip-if = verify
+
+[browser_test_unresponsive.js]
+skip-if = verify
new file mode 100644
--- /dev/null
+++ b/dom/tests/browser/perfmetrics/browser_test_unresponsive.js
@@ -0,0 +1,31 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* 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/perfmetrics";
+const PAGE_URL = ROOT_URL + "/unresponsive.html";
+
+add_task(async function test() {
+  // dom.performance.enable_scheduler_timing is set to true in browser.ini
+  waitForExplicitFinish();
+
+  await BrowserTestUtils.withNewTab({ gBrowser, url: PAGE_URL },
+    async function(browser) {
+    let dataBack = 0;
+    let tabId = gBrowser.selectedBrowser.outerWindowID;
+
+    function exploreResults(data, filterByWindowId) {
+      for (let entry of data) {
+        if (entry.windowId == tabId && entry.host != "about:blank") {
+          dataBack += 1;
+        }
+      }
+    }
+    let results = await ChromeUtils.requestPerformanceMetrics();
+    exploreResults(results);
+    Assert.ok(dataBack == 0);
+  });
+
+});
new file mode 100644
--- /dev/null
+++ b/dom/tests/browser/perfmetrics/unresponsive.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+<head>
+  <meta charset="utf-8">
+  <script type="text/javascript">
+
+  function fn() {
+    let start = Date.now();
+    while (Date.now() - start < 5000)
+      ; // do nothing
+    setTimeout(fn, 0);
+  }
+
+  setTimeout(fn, 10);
+
+  </script>
+</head>
+<body>
+  <h1>An unresponsive page</h1>
+</body>
+</html>
--- a/modules/libpref/init/StaticPrefList.h
+++ b/modules/libpref/init/StaticPrefList.h
@@ -206,16 +206,22 @@ VARCACHE_PREF(
 )
 
 VARCACHE_PREF(
   "dom.performance.enable_scheduler_timing",
   dom_performance_enable_scheduler_timing,
   RelaxedAtomicBool, true
 )
 
+VARCACHE_PREF(
+  "dom.performance.children_results_ipc_timeout",
+  dom_performance_children_results_ipc_timeout,
+  uint32_t, 1000
+)
+
 // If true. then the service worker interception and the ServiceWorkerManager
 // will live in the parent process.  This only takes effect on browser start.
 // Note, this is not currently safe to use for normal browsing yet.
 PREF("dom.serviceWorkers.parent_intercept", bool, false)
 
 // Enable PaymentRequest API
 #ifdef NIGHTLY_BUILD
 # define PREF_VALUE  true
--- a/toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp
+++ b/toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp
@@ -3,16 +3,17 @@
 /* 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/. */
 
 #include "nsThreadUtils.h"
 #include "mozilla/Logging.h"
 #include "mozilla/PerformanceUtils.h"
 #include "mozilla/PerformanceMetricsCollector.h"
+#include "mozilla/StaticPrefs.h"
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/WorkerDebugger.h"
 #include "mozilla/dom/WorkerDebuggerManager.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
@@ -20,41 +21,112 @@ static mozilla::LazyLogModule sPerfLog("
 #ifdef LOG
 #undef LOG
 #endif
 #define LOG(args) MOZ_LOG(sPerfLog, mozilla::LogLevel::Debug, args)
 
 namespace mozilla {
 
 //
+// class IPCTimeout
+//
+NS_IMPL_ISUPPORTS(IPCTimeout, nsIObserver)
+
+
+// static
+IPCTimeout*
+IPCTimeout::CreateInstance(AggregatedResults* aResults)
+{
+  MOZ_ASSERT(aResults);
+  uint32_t delay = StaticPrefs::dom_performance_children_results_ipc_timeout();
+  if (delay == 0) {
+    return nullptr;
+  }
+  return new IPCTimeout(aResults, delay);
+}
+
+
+IPCTimeout::IPCTimeout(AggregatedResults* aResults, uint32_t aDelay):
+    mResults(aResults)
+{
+  MOZ_ASSERT(aResults);
+  MOZ_ASSERT(aDelay > 0);
+  mozilla::DebugOnly<nsresult> rv = NS_NewTimerWithObserver(getter_AddRefs(mTimer),
+                                                            this,
+                                                            aDelay,
+                                                            nsITimer::TYPE_ONE_SHOT);
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  LOG(("IPCTimeout timer created"));
+}
+
+IPCTimeout::~IPCTimeout()
+{
+  Cancel();
+}
+
+void
+IPCTimeout::Cancel()
+{
+  if (mTimer) {
+    LOG(("IPCTimeout timer canceled"));
+    mTimer->Cancel();
+    mTimer = nullptr;
+  }
+}
+
+NS_IMETHODIMP
+IPCTimeout::Observe(nsISupports* aSubject, const char* aTopic, const char16_t* aData)
+{
+  MOZ_ASSERT(strcmp(aTopic, NS_TIMER_CALLBACK_TOPIC) == 0);
+  LOG(("IPCTimeout timer triggered"));
+  mResults->ResolveNow();
+  return NS_OK;
+}
+
+//
 // class AggregatedResults
 //
 AggregatedResults::AggregatedResults(nsID aUUID,
                                      PerformanceMetricsCollector* aCollector,
                                      dom::Promise* aPromise)
   : mPromise(aPromise)
   , mPendingResults(0)
   , mCollector(aCollector)
   , mUUID(aUUID)
 {
   MOZ_ASSERT(aCollector);
   MOZ_ASSERT(aPromise);
+  mIPCTimeout = IPCTimeout::CreateInstance(this);
 }
 
 void
 AggregatedResults::Abort(nsresult aReason)
 {
   MOZ_ASSERT(mPromise);
   MOZ_ASSERT(NS_FAILED(aReason));
+  if (mIPCTimeout) {
+    mIPCTimeout->Cancel();
+    mIPCTimeout = nullptr;
+  }
   mPromise->MaybeReject(aReason);
   mPromise = nullptr;
   mPendingResults = 0;
 }
 
 void
+AggregatedResults::ResolveNow()
+{
+  MOZ_ASSERT(mPromise);
+  LOG(("[%s] Early resolve", nsIDToCString(mUUID).get()));
+  mPromise->MaybeResolve(mData);
+  mIPCTimeout = nullptr;
+  mCollector->ForgetAggregatedResults(mUUID);
+}
+
+void
 AggregatedResults::AppendResult(const nsTArray<dom::PerformanceInfo>& aMetrics)
 {
   if (!mPromise) {
     // A previous call failed and the promise was already rejected
     return;
   }
   MOZ_ASSERT(mPendingResults > 0);
 
@@ -92,16 +164,20 @@ AggregatedResults::AppendResult(const ns
   }
 
   mPendingResults--;
   if (mPendingResults) {
     return;
   }
 
   LOG(("[%s] All data collected, resolving promise", nsIDToCString(mUUID).get()));
+  if (mIPCTimeout) {
+    mIPCTimeout->Cancel();
+    mIPCTimeout = nullptr;
+  }
   mPromise->MaybeResolve(mData);
   mCollector->ForgetAggregatedResults(mUUID);
 }
 
 void
 AggregatedResults::SetNumResultsRequired(uint32_t aNumResultsRequired)
 {
   MOZ_ASSERT(!mPendingResults && aNumResultsRequired);
@@ -192,28 +268,35 @@ PerformanceMetricsCollector::RequestMetr
 }
 
 
 // static
 nsresult
 PerformanceMetricsCollector::DataReceived(const nsID& aUUID,
                                           const nsTArray<PerformanceInfo>& aMetrics)
 {
-  MOZ_ASSERT(gInstance);
+  // If some content process were unresponsive on shutdown, we may get called
+  // here with late data received from children - so instead of asserting
+  // that gInstance is available, we just return.
+  if (!gInstance) {
+    LOG(("[%s] gInstance is gone", nsIDToCString(aUUID).get()));
+    return NS_OK;
+  }
   MOZ_ASSERT(XRE_IsParentProcess());
   return gInstance->DataReceivedInternal(aUUID, aMetrics);
 }
 
 nsresult
 PerformanceMetricsCollector::DataReceivedInternal(const nsID& aUUID,
                                                   const nsTArray<PerformanceInfo>& aMetrics)
 {
   MOZ_ASSERT(gInstance == this);
   UniquePtr<AggregatedResults>* results = mAggregatedResults.GetValue(aUUID);
   if (!results) {
+    LOG(("[%s] UUID is gone from mAggregatedResults", nsIDToCString(aUUID).get()));
     return NS_ERROR_FAILURE;
   }
 
   LOG(("[%s] Received one PerformanceInfo array", nsIDToCString(aUUID).get()));
   AggregatedResults* aggregatedResults = results->get();
   MOZ_ASSERT(aggregatedResults);
 
   // If this is the last result, APpendResult() will trigger the deletion
--- a/toolkit/components/perfmonitoring/PerformanceMetricsCollector.h
+++ b/toolkit/components/perfmonitoring/PerformanceMetricsCollector.h
@@ -1,27 +1,46 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* 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/. */
 
 #ifndef PerformanceMetricsCollector_h
 #define PerformanceMetricsCollector_h
 
+#include "nsIObserver.h"
+#include "nsITimer.h"
 #include "nsID.h"
 #include "mozilla/dom/ChromeUtilsBinding.h"  // defines PerformanceInfoDictionary
 #include "mozilla/dom/DOMTypes.h"   // defines PerformanceInfo
 
 namespace mozilla {
 
 namespace dom {
   class Promise;
 }
 
 class PerformanceMetricsCollector;
+class AggregatedResults;
+
+class IPCTimeout final: public nsIObserver
+{
+public:
+  NS_DECL_NSIOBSERVER
+  NS_DECL_ISUPPORTS
+  static IPCTimeout* CreateInstance(AggregatedResults* aResults);
+  void Cancel();
+
+private:
+  IPCTimeout(AggregatedResults* aResults, uint32_t aDelay);
+  ~IPCTimeout();
+
+  nsCOMPtr<nsITimer> mTimer;
+  AggregatedResults* mResults;
+};
 
 // AggregatedResults receives PerformanceInfo results that are collected
 // via IPDL from all content processes and the main process. They
 // are converted into an array of PerformanceInfoDictionary dictionaries (webidl)
 //
 // The class is instanciated with a Promise and a number of processes
 // that are supposed to send back results.
 //
@@ -33,18 +52,20 @@ class AggregatedResults final
 {
 public:
   AggregatedResults(nsID aUUID, PerformanceMetricsCollector* aCollector,
                     dom::Promise* aPromise);
   ~AggregatedResults() = default;
   void AppendResult(const nsTArray<dom::PerformanceInfo>& aMetrics);
   void SetNumResultsRequired(uint32_t aNumResultsRequired);
   void Abort(nsresult aReason);
+  void ResolveNow();
 
 private:
+  RefPtr<IPCTimeout> mIPCTimeout;
   RefPtr<dom::Promise> mPromise;
   uint32_t mPendingResults;
   FallibleTArray<dom::PerformanceInfoDictionary> mData;
 
   // AggregatedResults keeps a reference on the collector
   // so it gets destructed when all pending AggregatedResults
   // are themselves destructed when removed from
   // PerformanceMetricsCollector::mAggregatedResults.