From 54d5f1c9ed260be01418b2f41d1fc4c7e68a5561 Mon Sep 17 00:00:00 2001 From: Dorel Luca Date: Fri, 24 Jul 2020 02:58:22 +0300 Subject: [PATCH] Backed out 4 changesets (bug 1652613) for Browser-chrome failures in browser/browser_ProcessHangNotifications.js. CLOSED TREE Backed out changeset 76b5a5d243d1 (bug 1652613) Backed out changeset 6f98c9b01920 (bug 1652613) Backed out changeset 1255237ce2e7 (bug 1652613) Backed out changeset bdf59854c900 (bug 1652613) --- browser/modules/ProcessHangMonitor.jsm | 149 ++++------------------- dom/base/nsGlobalWindowInner.cpp | 6 +- dom/base/nsGlobalWindowInner.h | 3 +- dom/ipc/PProcessHangMonitor.ipdl | 1 - dom/ipc/ProcessHangMonitor.cpp | 36 ++---- dom/ipc/ProcessHangMonitor.h | 3 +- dom/ipc/nsIHangReport.idl | 2 - js/xpconnect/src/XPCJSContext.cpp | 11 +- toolkit/components/telemetry/Events.yaml | 27 ---- 9 files changed, 42 insertions(+), 196 deletions(-) diff --git a/browser/modules/ProcessHangMonitor.jsm b/browser/modules/ProcessHangMonitor.jsm index 051370cce737..b65752c5bbc4 100644 --- a/browser/modules/ProcessHangMonitor.jsm +++ b/browser/modules/ProcessHangMonitor.jsm @@ -39,19 +39,13 @@ var ProcessHangMonitor = { /** * Collection of hang reports that haven't expired or been dismissed - * by the user. These are nsIHangReports. They are mapped to objects - * containing: - * - notificationTime: when (Cu.now()) we first showed a notification - * - waitCount: how often the user asked to wait for the script to finish - * - lastReportFromChild: when (Cu.now()) we last got hang info from the - * child. + * by the user. These are nsIHangReports. */ - _activeReports: new Map(), + _activeReports: new Set(), /** * Collection of hang reports that have been suppressed for a short - * period of time. Value is an object like in _activeReports, but also - * including a `timer` prop, which is an nsITimer for when the wait time + * period of time. Value is an nsITimer for when the wait time * expires. */ _pausedReports: new Map(), @@ -65,7 +59,6 @@ var ProcessHangMonitor = { Services.obs.addObserver(this, "quit-application-granted"); Services.obs.addObserver(this, "xpcom-shutdown"); Services.ww.registerNotification(this); - Services.telemetry.setEventRecordingEnabled("slow_script_warning", true); }, /** @@ -94,7 +87,6 @@ var ProcessHangMonitor = { report.endStartingDebugger(); } - this._recordTelemetryForReport(report, "debugging"); report.beginStartingDebugger(); let svc = Cc["@mozilla.org/dom/slow-script-debug;1"].getService( @@ -126,7 +118,6 @@ var ProcessHangMonitor = { switch (report.hangType) { case report.SLOW_SCRIPT: - this._recordTelemetryForReport(report, "user-aborted"); this.terminateScript(win); break; case report.PLUGIN_HANG: @@ -147,7 +138,6 @@ var ProcessHangMonitor = { switch (report.hangType) { case report.SLOW_SCRIPT: - this._recordTelemetryForReport(report, "user-aborted"); this.terminateGlobal(win); break; } @@ -157,10 +147,9 @@ var ProcessHangMonitor = { * Terminate whatever is causing this report, be it an add-on, page script, * or plug-in. This is done without updating any report notifications. */ - stopHang(report, endReason, backupInfo) { + stopHang(report) { switch (report.hangType) { case report.SLOW_SCRIPT: { - this._recordTelemetryForReport(report, endReason, backupInfo); if (report.addonId) { report.terminateGlobal(); } else { @@ -184,10 +173,6 @@ var ProcessHangMonitor = { if (!report) { return; } - // Update the other info we keep. - let reportInfo = this._activeReports.get(report); - reportInfo.waitCount++; - // Remove the report from the active list. this.removeActiveReport(report); @@ -200,13 +185,13 @@ var ProcessHangMonitor = { let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); timer.initWithCallback( () => { - for (let [stashedReport, pausedInfo] of this._pausedReports) { - if (pausedInfo.timer === timer) { + for (let [stashedReport, otherTimer] of this._pausedReports) { + if (otherTimer === timer) { this.removePausedReport(stashedReport); // We're still hung, so move the report back to the active // list and update the UI. - this._activeReports.set(report, pausedInfo); + this._activeReports.add(report); this.updateWindows(); break; } @@ -216,8 +201,7 @@ var ProcessHangMonitor = { timer.TYPE_ONE_SHOT ); - reportInfo.timer = timer; - this._pausedReports.set(report, reportInfo); + this._pausedReports.set(report, timer); // remove the browser notification associated with this hang this.updateWindows(); @@ -292,7 +276,7 @@ var ProcessHangMonitor = { */ onQuitApplicationGranted() { this._shuttingDown = true; - this.stopAllHangs("quit-application-granted"); + this.stopAllHangs(); this.updateWindows(); }, @@ -309,7 +293,7 @@ var ProcessHangMonitor = { // the hang. } if (!hungBrowserWindow || hungBrowserWindow == win) { - this.stopHang(report, "window-closed"); + this.stopHang(report); return true; } } else if (report.hangType == report.PLUGIN_HANG) { @@ -338,15 +322,15 @@ var ProcessHangMonitor = { this.updateWindows(); }, - stopAllHangs(endReason) { + stopAllHangs() { for (let report of this._activeReports) { - this.stopHang(report, endReason); + this.stopHang(report); } - this._activeReports = new Map(); + this._activeReports = new Set(); for (let [pausedReport] of this._pausedReports) { - this.stopHang(pausedReport, endReason); + this.stopHang(pausedReport); this.removePausedReport(pausedReport); } }, @@ -356,7 +340,7 @@ var ProcessHangMonitor = { */ findActiveReport(browser) { let frameLoader = browser.frameLoader; - for (let report of this._activeReports.keys()) { + for (let report of this._activeReports) { if (report.isReportForBrowser(frameLoader)) { return report; } @@ -377,72 +361,6 @@ var ProcessHangMonitor = { return null; }, - /** - * Tell telemetry about the report. - */ - _recordTelemetryForReport(report, endReason, backupInfo) { - let info = - this._activeReports.get(report) || - this._pausedReports.get(report) || - backupInfo; - if (!info) { - return; - } - try { - // Only report slow script hangs. - if (report.hangType != report.SLOW_SCRIPT) { - return; - } - let uri_type; - if (report.addonId) { - uri_type = "extension"; - } else if (report.scriptFileName?.startsWith("debugger")) { - uri_type = "devtools"; - } else { - try { - let url = new URL(report.scriptFileName); - if (url.protocol == "chrome:" || url.protocol == "resource:") { - uri_type = "browser"; - } else { - uri_type = "content"; - } - } catch (ex) { - Cu.reportError(ex); - uri_type = "unknown"; - } - } - let uptime = 0; - if (info.notificationTime) { - uptime = Cu.now() - info.notificationTime; - } - uptime = "" + uptime; - // We combine the duration of the hang in the content process with the - // time since we were last told about the hang in the parent. This is - // not the same as the time we showed a notification, as we only do that - // for the currently selected browser. It's as messy as it is because - // there is no cross-process monotonically increasing timestamp we can - // use. :-( - let hangDuration = - report.hangDuration + Cu.now() - info.lastReportFromChild; - Services.telemetry.recordEvent( - "slow_script_warning", - "shown", - "content", - null, - { - end_reason: endReason, - hang_duration: "" + hangDuration, - n_tab_deselect: "" + info.deselectCount, - uri_type, - uptime, - wait_count: "" + info.waitCount, - } - ); - } catch (ex) { - Cu.reportError(ex); - } - }, - /** * Remove an active hang report from the active list and cancel the timer * associated with it. @@ -457,8 +375,10 @@ var ProcessHangMonitor = { * associated with it. */ removePausedReport(report) { - let info = this._pausedReports.get(report); - info?.timer?.cancel(); + let timer = this._pausedReports.get(report); + if (timer) { + timer.cancel(); + } this._pausedReports.delete(report); }, @@ -475,7 +395,7 @@ var ProcessHangMonitor = { // we have no opportunity to ask the user whether or not they want // to stop the hang or wait, so we'll opt for stopping the hang. if (!e.hasMoreElements()) { - this.stopAllHangs("no-windows-left"); + this.stopAllHangs(); return; } @@ -498,10 +418,6 @@ var ProcessHangMonitor = { let report = this.findActiveReport(win.gBrowser.selectedBrowser); if (report) { - let info = this._activeReports.get(report); - if (info && !info.notificationTime) { - info.notificationTime = Cu.now(); - } this.showNotification(win, report); } else { this.hideNotification(win); @@ -639,16 +555,8 @@ var ProcessHangMonitor = { // If a new tab is selected or if a tab changes remoteness, then // we may need to show or hide a hang notification. + if (event.type == "TabSelect" || event.type == "TabRemotenessChange") { - if (event.type == "TabSelect" && event.detail.previousTab) { - // If we've got a notification, check the previous tab's report and - // indicate the user switched tabs while the notification was up. - let r = this.findActiveReport(event.detail.previousTab.linkedBrowser); - if (r) { - let info = this._activeReports.get(r); - info.deselectCount++; - } - } this.updateWindow(win); } }, @@ -658,17 +566,13 @@ var ProcessHangMonitor = { * before, show a notification for it in all open XUL windows. */ reportHang(report) { - let now = Cu.now(); if (this._shuttingDown) { - this.stopHang(report, "shutdown-in-progress", { - lastReportFromChild: now, - }); + this.stopHang(report); return; } // If this hang was already reported reset the timer for it. if (this._activeReports.has(report)) { - this._activeReports.get(report).lastReportFromChild = now; // if this report is in active but doesn't have a notification associated // with it, display a notification. this.updateWindows(); @@ -677,7 +581,6 @@ var ProcessHangMonitor = { // If this hang was already reported and paused by the user ignore it. if (this._pausedReports.has(report)) { - this._pausedReports.get(report).lastReportFromChild = now; return; } @@ -692,17 +595,11 @@ var ProcessHangMonitor = { Services.telemetry.getHistogramById("PLUGIN_HANG_NOTICE_COUNT").add(); } - this._activeReports.set(report, { - deselectCount: 0, - lastReportFromChild: now, - waitCount: 0, - }); + this._activeReports.add(report); this.updateWindows(); }, clearHang(report) { - this._recordTelemetryForReport(report, "cleared"); - this.removeActiveReport(report); this.removePausedReport(report); report.userCanceled(); diff --git a/dom/base/nsGlobalWindowInner.cpp b/dom/base/nsGlobalWindowInner.cpp index 409788b49648..d7bedcda7ed9 100644 --- a/dom/base/nsGlobalWindowInner.cpp +++ b/dom/base/nsGlobalWindowInner.cpp @@ -4720,8 +4720,7 @@ void nsGlobalWindowInner::FireOfflineStatusEventIfChanged() { nsGlobalWindowInner::SlowScriptResponse nsGlobalWindowInner::ShowSlowScriptDialog(JSContext* aCx, - const nsString& aAddonId, - const double aDuration) { + const nsString& aAddonId) { nsresult rv; if (Preferences::GetBool("dom.always_stop_slow_scripts")) { @@ -4769,8 +4768,7 @@ nsGlobalWindowInner::ShowSlowScriptDialog(JSContext* aCx, nsIDocShell* docShell = GetDocShell(); nsCOMPtr child = docShell ? docShell->GetBrowserChild() : nullptr; - action = - monitor->NotifySlowScript(child, filename.get(), aAddonId, aDuration); + action = monitor->NotifySlowScript(child, filename.get(), aAddonId); if (action == ProcessHangMonitor::Terminate) { return KillSlowScript; } diff --git a/dom/base/nsGlobalWindowInner.h b/dom/base/nsGlobalWindowInner.h index d2ddbd88add5..bb4edbafe55d 100644 --- a/dom/base/nsGlobalWindowInner.h +++ b/dom/base/nsGlobalWindowInner.h @@ -482,8 +482,7 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget, KillScriptGlobal }; SlowScriptResponse ShowSlowScriptDialog(JSContext* aCx, - const nsString& aAddonId, - const double aDuration); + const nsString& aAddonId); // Inner windows only. void AddGamepad(uint32_t aIndex, mozilla::dom::Gamepad* aGamepad); diff --git a/dom/ipc/PProcessHangMonitor.ipdl b/dom/ipc/PProcessHangMonitor.ipdl index 224b98adedcf..f0993417f21c 100644 --- a/dom/ipc/PProcessHangMonitor.ipdl +++ b/dom/ipc/PProcessHangMonitor.ipdl @@ -20,7 +20,6 @@ struct SlowScriptData TabId tabId; nsCString filename; nsString addonId; - double duration; }; struct PluginHangData diff --git a/dom/ipc/ProcessHangMonitor.cpp b/dom/ipc/ProcessHangMonitor.cpp index ba3619a16e8d..7dca618e0ba6 100644 --- a/dom/ipc/ProcessHangMonitor.cpp +++ b/dom/ipc/ProcessHangMonitor.cpp @@ -89,10 +89,9 @@ class HangMonitorChild : public PProcessHangMonitorChild, typedef ProcessHangMonitor::SlowScriptAction SlowScriptAction; SlowScriptAction NotifySlowScript(nsIBrowserChild* aBrowserChild, const char* aFileName, - const nsString& aAddonId, - const double aDuration); + const nsString& aAddonId); void NotifySlowScriptAsync(TabId aTabId, const nsCString& aFileName, - const nsString& aAddonId, const double aDuration); + const nsString& aAddonId); bool IsDebuggerStartupComplete(); @@ -590,17 +589,15 @@ void HangMonitorChild::Bind(Endpoint&& aEndpoint) { void HangMonitorChild::NotifySlowScriptAsync(TabId aTabId, const nsCString& aFileName, - const nsString& aAddonId, - const double aDuration) { + const nsString& aAddonId) { if (mIPCOpen) { - Unused << SendHangEvidence( - SlowScriptData(aTabId, aFileName, aAddonId, aDuration)); + Unused << SendHangEvidence(SlowScriptData(aTabId, aFileName, aAddonId)); } } HangMonitorChild::SlowScriptAction HangMonitorChild::NotifySlowScript( nsIBrowserChild* aBrowserChild, const char* aFileName, - const nsString& aAddonId, const double aDuration) { + const nsString& aAddonId) { MOZ_RELEASE_ASSERT(NS_IsMainThread()); mSentReport = true; @@ -632,10 +629,9 @@ HangMonitorChild::SlowScriptAction HangMonitorChild::NotifySlowScript( } nsAutoCString filename(aFileName); - Dispatch(NewNonOwningRunnableMethod( + Dispatch(NewNonOwningRunnableMethod( "HangMonitorChild::NotifySlowScriptAsync", this, - &HangMonitorChild::NotifySlowScriptAsync, id, filename, aAddonId, - aDuration)); + &HangMonitorChild::NotifySlowScriptAsync, id, filename, aAddonId)); return SlowScriptAction::Continue; } @@ -855,12 +851,11 @@ void HangMonitorParent::SendHangNotification(const HangData& aHangData, void HangMonitorParent::ClearHangNotification() { // chrome process, main thread MOZ_RELEASE_ASSERT(NS_IsMainThread()); + mProcess->ClearHang(); nsCOMPtr observerService = mozilla::services::GetObserverService(); observerService->NotifyObservers(mProcess, "clear-hang-report", nullptr); - - mProcess->ClearHang(); } // Take a minidump of the browser process if one wasn't already taken for the @@ -1012,17 +1007,6 @@ HangMonitoredProcess::GetHangType(uint32_t* aHangType) { return NS_OK; } -NS_IMETHODIMP -HangMonitoredProcess::GetHangDuration(double* aHangDuration) { - MOZ_RELEASE_ASSERT(NS_IsMainThread()); - if (mHangData.type() != HangData::TSlowScriptData) { - *aHangDuration = -1; - } else { - *aHangDuration = mHangData.get_SlowScriptData().duration(); - } - return NS_OK; -} - NS_IMETHODIMP HangMonitoredProcess::GetScriptBrowser(Element** aBrowser) { MOZ_RELEASE_ASSERT(NS_IsMainThread()); @@ -1283,10 +1267,10 @@ ProcessHangMonitor::Observe(nsISupports* aSubject, const char* aTopic, ProcessHangMonitor::SlowScriptAction ProcessHangMonitor::NotifySlowScript( nsIBrowserChild* aBrowserChild, const char* aFileName, - const nsString& aAddonId, const double aDuration) { + const nsString& aAddonId) { MOZ_RELEASE_ASSERT(NS_IsMainThread()); return HangMonitorChild::Get()->NotifySlowScript(aBrowserChild, aFileName, - aAddonId, aDuration); + aAddonId); } bool ProcessHangMonitor::IsDebuggerStartupComplete() { diff --git a/dom/ipc/ProcessHangMonitor.h b/dom/ipc/ProcessHangMonitor.h index d66d1efaed6c..1d1074c0b77c 100644 --- a/dom/ipc/ProcessHangMonitor.h +++ b/dom/ipc/ProcessHangMonitor.h @@ -70,8 +70,7 @@ class ProcessHangMonitor final : public nsIObserver { }; SlowScriptAction NotifySlowScript(nsIBrowserChild* aBrowserChild, const char* aFileName, - const nsString& aAddonId, - const double aDuration); + const nsString& aAddonId); void NotifyPluginHang(uint32_t aPluginId); diff --git a/dom/ipc/nsIHangReport.idl b/dom/ipc/nsIHangReport.idl index e4bb46d4cc65..a35be9ea6ada 100644 --- a/dom/ipc/nsIHangReport.idl +++ b/dom/ipc/nsIHangReport.idl @@ -32,8 +32,6 @@ interface nsIHangReport : nsISupports // Only valid for SLOW_SCRIPT reports. readonly attribute Element scriptBrowser; readonly attribute ACString scriptFileName; - // Duration of the hang so far. - readonly attribute double hangDuration; readonly attribute AString addonId; // For PLUGIN_HANGs, this field contains information about the plugin. diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index 2c1870baf1f8..d425b5022c88 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -619,8 +619,7 @@ bool XPCJSContext::InterruptCallback(JSContext* cx) { // This is at least the second interrupt callback we've received since // returning to the event loop. See how long it's been, and what the limit // is. - TimeStamp now = TimeStamp::NowLoRes(); - TimeDuration duration = now - self->mSlowScriptCheckpoint; + TimeDuration duration = TimeStamp::NowLoRes() - self->mSlowScriptCheckpoint; int32_t limit; nsString addonId; @@ -645,13 +644,13 @@ bool XPCJSContext::InterruptCallback(JSContext* cx) { return true; } - self->mSlowScriptCheckpoint = now; self->mSlowScriptActualWait += duration; // In order to guard against time changes or laptops going to sleep, we // don't trigger the slow script warning until (limit/2) seconds have // elapsed twice. if (!self->mSlowScriptSecondHalf) { + self->mSlowScriptCheckpoint = TimeStamp::NowLoRes(); self->mSlowScriptSecondHalf = true; return true; } @@ -701,8 +700,8 @@ bool XPCJSContext::InterruptCallback(JSContext* cx) { } // Show the prompt to the user, and kill if requested. - nsGlobalWindowInner::SlowScriptResponse response = win->ShowSlowScriptDialog( - cx, addonId, self->mSlowScriptActualWait.ToMilliseconds()); + nsGlobalWindowInner::SlowScriptResponse response = + win->ShowSlowScriptDialog(cx, addonId); if (response == nsGlobalWindowInner::KillSlowScript) { if (Preferences::GetBool("dom.global_stop_script", true)) { xpc::Scriptability::Get(global).Block(); @@ -736,7 +735,7 @@ bool XPCJSContext::InterruptCallback(JSContext* cx) { } // The user chose to continue the script. Reset the timer, and disable this - // machinery with a pref if the user opted out of future slow-script dialogs. + // machinery with a pref of the user opted out of future slow-script dialogs. if (response != nsGlobalWindowInner::ContinueSlowScriptAndKeepNotifying) { self->mSlowScriptCheckpoint = TimeStamp::NowLoRes(); } diff --git a/toolkit/components/telemetry/Events.yaml b/toolkit/components/telemetry/Events.yaml index 17b5a33cc3f2..e134847675ae 100644 --- a/toolkit/components/telemetry/Events.yaml +++ b/toolkit/components/telemetry/Events.yaml @@ -2271,33 +2271,6 @@ security.ui.certerror: has_sts: If the error page is for a site with HSTS headers or with a pinned key. panel_open: If the advanced panel was open at the time of the interaction. -slow_script_warning: - shown: - bug_numbers: - - 1652613 - description: > - Recorded when a slow script hang is resolved. - products: - - "firefox" - record_in_processes: ["main"] - release_channel_collection: opt-out - expiry_version: "85" - notification_emails: - - esmyth@mozilla.com - - gkruitbosch@mozilla.com - # Whether the hung script was for a content or browser process. - objects: [ - "browser", - "content", - ] - extra_keys: - end_reason: Why the warning was hidden (user action, the process becoming responsive again, the browser quitting, etc.) - wait_count: How many times the user elected to wait. - hang_duration: How long we believe the hang continued (ms). - n_tab_deselect: How many times the user switched away from a tab affected by this hang. - uri_type: The kind of script URL that hung. - uptime: How long the notification was up (ms). - webrtc.ui: share_display: objects: