From d53cfe736977e6834210bdc3608e6fd1c61c9c1c Mon Sep 17 00:00:00 2001 From: Bogdan Tara Date: Thu, 25 Feb 2021 22:43:33 +0200 Subject: [PATCH] Backed out 5 changesets (bug 1694229) by flod's request, lint failures CLOSED TREE Backed out changeset cb3d9e8d32e6 (bug 1694229) Backed out changeset 877471a44509 (bug 1694229) Backed out changeset 286b311d32b2 (bug 1694229) Backed out changeset 42cb688eae03 (bug 1694229) Backed out changeset d082f53d882e (bug 1694229) --- .../en-US/chrome/browser/browser.properties | 12 +- browser/modules/ProcessHangMonitor.jsm | 122 ++++-------------- .../browser_ProcessHangNotifications.js | 13 +- dom/base/nsContentUtils.cpp | 2 - dom/ipc/BrowserChild.cpp | 13 -- dom/ipc/BrowserChild.h | 7 - dom/ipc/BrowserParent.cpp | 20 +-- dom/ipc/PBrowser.ipdl | 8 -- js/xpconnect/src/XPCJSContext.cpp | 10 +- .../geckoview/test/ContentDelegateTest.kt | 1 + modules/libpref/init/StaticPrefList.yaml | 21 +-- 11 files changed, 51 insertions(+), 178 deletions(-) diff --git a/browser/locales/en-US/chrome/browser/browser.properties b/browser/locales/en-US/chrome/browser/browser.properties index 5147de3bec7d..56d9bb490309 100644 --- a/browser/locales/en-US/chrome/browser/browser.properties +++ b/browser/locales/en-US/chrome/browser/browser.properties @@ -693,21 +693,17 @@ dataReportingNotification.button.label = Choose What I Share dataReportingNotification.button.accessKey = C # Process hang reporter -# LOCALIZATION NOTE (processHang.selected_tab.label): %1$S is the name of the product (e.g., Firefox) -processHang.selected_tab.label = This page is slowing down %1$S. To speed up your browser, stop that page. -# LOCALIZATION NOTE (processHang.nonspecific_tab.label): %1$S is the name of the product (e.g., Firefox) -processHang.nonspecific_tab.label = A web page is slowing down %1$S. To speed up your browser, stop that page. -# LOCALIZATION NOTE (processHang.specific_tab.label): %1$S is the title of the tab. -# %2$S is the name of the product (e.g., Firefox) -processHang.specific_tab.label = “%1$S” is slowing down %2$S. To speed up your browser, stop that page. +processHang.label = A web page is slowing down your browser. What would you like to do? # LOCALIZATION NOTE (processHang.add-on.label): %1$S is the name of the # extension. %2$S is the name of the product (e.g., Firefox) processHang.add-on.label = A script in the extension “%1$S” is causing %2$S to slow down. processHang.add-on.learn-more.text = Learn more -processHang.button_stop.label = Stop +processHang.button_stop.label = Stop It processHang.button_stop.accessKey = S processHang.button_stop_sandbox.label = Temporarily Disable Extension on Page processHang.button_stop_sandbox.accessKey = A +processHang.button_wait.label = Wait +processHang.button_wait.accessKey = W processHang.button_debug.label = Debug Script processHang.button_debug.accessKey = D diff --git a/browser/modules/ProcessHangMonitor.jsm b/browser/modules/ProcessHangMonitor.jsm index 6a3e9f891fc2..37a3c87cf627 100644 --- a/browser/modules/ProcessHangMonitor.jsm +++ b/browser/modules/ProcessHangMonitor.jsm @@ -12,40 +12,6 @@ const { AppConstants } = ChromeUtils.import( ); const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); -/** - * Elides the middle of a string by replacing it with an elipsis if it is - * longer than `threshold` characters. Does its best to not break up grapheme - * clusters. - */ -function elideMiddleOfString(str, threshold) { - const searchDistance = 5; - const stubLength = threshold / 2 - searchDistance; - if (str.length <= threshold || stubLength < searchDistance) { - return str; - } - - function searchElisionPoint(position) { - let unsplittableCharacter = c => /[\p{M}\uDC00-\uDFFF]/u.test(c); - for (let i = 0; i < searchDistance; i++) { - if (!unsplittableCharacter(str[position + i])) { - return position + i; - } - - if (!unsplittableCharacter(str[position - i])) { - return position - i; - } - } - return position; - } - - let elisionStart = searchElisionPoint(stubLength); - let elisionEnd = searchElisionPoint(str.length - stubLength); - if (elisionStart < elisionEnd) { - str = str.slice(0, elisionStart) + "\u2026" + str.slice(elisionEnd); - } - return str; -} - /** * This JSM is responsible for observing content process hang reports * and asking the user what to do about them. See nsIHangReport for @@ -546,6 +512,13 @@ var ProcessHangMonitor = { * Show the notification for a hang. */ showNotification(win, report) { + let notification = win.gHighPriorityNotificationBox.getNotificationWithValue( + "process-hang" + ); + if (notification) { + return; + } + let bundle = win.gNavigatorBundle; let buttons = [ @@ -556,25 +529,29 @@ var ProcessHangMonitor = { ProcessHangMonitor.stopIt(win); }, }, + { + label: bundle.getString("processHang.button_wait.label"), + accessKey: bundle.getString("processHang.button_wait.accessKey"), + callback() { + ProcessHangMonitor.waitLonger(win); + }, + }, ]; - let message; - let doc = win.document; - let brandShortName = doc - .getElementById("bundle_brand") - .getString("brandShortName"); - let notificationTag; + let message = bundle.getString("processHang.label"); if (report.addonId) { - notificationTag = report.addonId; let aps = Cc["@mozilla.org/addons/policy-service;1"].getService( Ci.nsIAddonPolicyService ); + let doc = win.document; + let brandBundle = doc.getElementById("bundle_brand"); + let addonName = aps.getExtensionName(report.addonId); let label = bundle.getFormattedString("processHang.add-on.label", [ addonName, - brandShortName, + brandBundle.getString("brandShortName"), ]); let linkText = bundle.getString("processHang.add-on.learn-more.text"); @@ -602,46 +579,6 @@ var ProcessHangMonitor = { ProcessHangMonitor.stopGlobal(win); }, }); - } else { - let scriptBrowser = report.scriptBrowser; - if (scriptBrowser == win.gBrowser?.selectedBrowser) { - notificationTag = "selected-tab"; - message = bundle.getFormattedString("processHang.selected_tab.label", [ - brandShortName, - ]); - } else { - let tab = scriptBrowser?.ownerGlobal.gBrowser?.getTabForBrowser( - scriptBrowser - ); - if (!tab) { - notificationTag = "nonspecific_tab"; - message = bundle.getFormattedString( - "processHang.nonspecific_tab.label", - [brandShortName] - ); - } else { - notificationTag = scriptBrowser.browserId.toString(); - let title = tab.getAttribute("label"); - title = elideMiddleOfString(title, 60); - message = bundle.getFormattedString( - "processHang.specific_tab.label", - [title, brandShortName] - ); - } - } - } - - let notification = win.gHighPriorityNotificationBox.getNotificationWithValue( - "process-hang" - ); - if (notificationTag == notification?.getAttribute("notification-tag")) { - return; - } - - if (notification) { - notification.label = message; - notification.setAttribute("notification-tag", notificationTag); - return; } if (AppConstants.MOZ_DEV_EDITION && report.hangType == report.SLOW_SCRIPT) { @@ -654,20 +591,13 @@ var ProcessHangMonitor = { }); } - win.gHighPriorityNotificationBox - .appendNotification( - message, - "process-hang", - "chrome://browser/content/aboutRobots-icon.png", - win.gHighPriorityNotificationBox.PRIORITY_WARNING_HIGH, - buttons, - event => { - if (event == "dismissed") { - ProcessHangMonitor.waitLonger(win); - } - } - ) - .setAttribute("notification-tag", notificationTag); + win.gHighPriorityNotificationBox.appendNotification( + message, + "process-hang", + "chrome://browser/content/aboutRobots-icon.png", + win.gHighPriorityNotificationBox.PRIORITY_WARNING_HIGH, + buttons + ); }, /** diff --git a/browser/modules/test/browser/browser_ProcessHangNotifications.js b/browser/modules/test/browser/browser_ProcessHangNotifications.js index 2286d160141e..af0e26f0e0ca 100644 --- a/browser/modules/test/browser/browser_ProcessHangNotifications.js +++ b/browser/modules/test/browser/browser_ProcessHangNotifications.js @@ -125,7 +125,7 @@ TestHangReport.prototype = { }; // on dev edition we add a button for js debugging of hung scripts. -let buttonCount = AppConstants.MOZ_DEV_EDITION ? 2 : 1; +let buttonCount = AppConstants.MOZ_DEV_EDITION ? 3 : 2; add_task(async function setup() { // Create a fake WebExtensionPolicy that we can use for @@ -184,11 +184,6 @@ add_task(async function waitForScriptTest() { let buttons = notification.currentNotification.getElementsByTagName("button"); is(buttons.length, buttonCount, "proper number of buttons"); - let toolbarbuttons = notification.currentNotification.getElementsByTagName( - "toolbarbutton" - ); - is(toolbarbuttons.length, 1, "proper number of toolbarbuttons"); - let closeButton = toolbarbuttons[0]; await pushPrefs(["browser.hangNotification.waitPeriod", 1000]); @@ -211,7 +206,7 @@ add_task(async function waitForScriptTest() { }); // Click the "Wait" button this time, we shouldn't get a callback at all. - closeButton.click(); + buttons[1].click(); // send another hang pulse, we should not get a notification here Services.obs.notifyObservers(hangReport, "process-hang-report"); @@ -261,9 +256,9 @@ add_task(async function terminatePluginTest() { let notification = await promise; let buttons = notification.currentNotification.getElementsByTagName("button"); - // Plugin hangs only ever show 1 button in the notification - even in + // Plugin hangs only ever show 2 buttons in the notification - even in // DevEdition. - is(buttons.length, 1, "proper number of buttons"); + is(buttons.length, 2, "proper number of buttons"); // Click the "Stop It" button, we should get a terminate script callback buttons[0].click(); diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index e1968b9ddd5d..0b0893e9d363 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -10019,7 +10019,6 @@ bool nsContentUtils::IsMessageInputEvent(const IPC::Message& aMsg) { switch (aMsg.type()) { case mozilla::dom::PBrowser::Msg_RealMouseMoveEvent__ID: case mozilla::dom::PBrowser::Msg_RealMouseButtonEvent__ID: - case mozilla::dom::PBrowser::Msg_RealMouseEnterExitWidgetEvent__ID: case mozilla::dom::PBrowser::Msg_RealKeyEvent__ID: case mozilla::dom::PBrowser::Msg_MouseWheelEvent__ID: case mozilla::dom::PBrowser::Msg_RealTouchEvent__ID: @@ -10040,7 +10039,6 @@ bool nsContentUtils::IsMessageCriticalInputEvent(const IPC::Message& aMsg) { switch (aMsg.type()) { case mozilla::dom::PBrowser::Msg_RealMouseButtonEvent__ID: case mozilla::dom::PBrowser::Msg_RealKeyEvent__ID: - case mozilla::dom::PBrowser::Msg_MouseWheelEvent__ID: case mozilla::dom::PBrowser::Msg_RealTouchEvent__ID: case mozilla::dom::PBrowser::Msg_RealDragEvent__ID: return true; diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index a779c2e93843..fe515df85be6 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -1679,19 +1679,6 @@ mozilla::ipc::IPCResult BrowserChild::RecvNormalPriorityRealMouseButtonEvent( return RecvRealMouseButtonEvent(aEvent, aGuid, aInputBlockId); } -mozilla::ipc::IPCResult BrowserChild::RecvRealMouseEnterExitWidgetEvent( - const WidgetMouseEvent& aEvent, const ScrollableLayerGuid& aGuid, - const uint64_t& aInputBlockId) { - return RecvRealMouseButtonEvent(aEvent, aGuid, aInputBlockId); -} - -mozilla::ipc::IPCResult -BrowserChild::RecvNormalPriorityRealMouseEnterExitWidgetEvent( - const WidgetMouseEvent& aEvent, const ScrollableLayerGuid& aGuid, - const uint64_t& aInputBlockId) { - return RecvRealMouseButtonEvent(aEvent, aGuid, aInputBlockId); -} - // In case handling repeated mouse wheel takes much time, we skip firing current // wheel event if it may be coalesced to the next one. bool BrowserChild::MaybeCoalesceWheelEvent(const WidgetWheelEvent& aEvent, diff --git a/dom/ipc/BrowserChild.h b/dom/ipc/BrowserChild.h index 4d6132356610..d94ca031eb26 100644 --- a/dom/ipc/BrowserChild.h +++ b/dom/ipc/BrowserChild.h @@ -329,13 +329,6 @@ class BrowserChild final : public nsMessageManagerScriptExecutor, const mozilla::WidgetMouseEvent& aEvent, const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId); - mozilla::ipc::IPCResult RecvRealMouseEnterExitWidgetEvent( - const mozilla::WidgetMouseEvent& aEvent, const ScrollableLayerGuid& aGuid, - const uint64_t& aInputBlockId); - mozilla::ipc::IPCResult RecvNormalPriorityRealMouseEnterExitWidgetEvent( - const mozilla::WidgetMouseEvent& aEvent, const ScrollableLayerGuid& aGuid, - const uint64_t& aInputBlockId); - MOZ_CAN_RUN_SCRIPT_BOUNDARY mozilla::ipc::IPCResult RecvRealDragEvent(const WidgetDragEvent& aEvent, const uint32_t& aDragAction, diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index 55a621fe8d2c..6b3632a81fda 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -1462,10 +1462,10 @@ void BrowserParent::SendRealMouseEvent(WidgetMouseEvent& aEvent) { localEvent.mMessage = eMouseEnterIntoWidget; DebugOnly ret = isInputPriorityEventEnabled - ? SendRealMouseEnterExitWidgetEvent(localEvent, guid, blockId) - : SendNormalPriorityRealMouseEnterExitWidgetEvent(localEvent, guid, - blockId); - NS_WARNING_ASSERTION(ret, "SendRealMouseEnterExitWidgetEvent() failed"); + ? SendRealMouseButtonEvent(localEvent, guid, blockId) + : SendNormalPriorityRealMouseButtonEvent(localEvent, guid, blockId); + NS_WARNING_ASSERTION( + ret, "SendRealMouseButtonEvent(eMouseEnterIntoWidget) failed"); MOZ_ASSERT(!ret || localEvent.HasBeenPostedToRemoteProcess()); } @@ -1500,18 +1500,6 @@ void BrowserParent::SendRealMouseEvent(WidgetMouseEvent& aEvent) { return; } - if (eMouseEnterIntoWidget == aEvent.mMessage || - eMouseExitFromWidget == aEvent.mMessage) { - DebugOnly ret = - isInputPriorityEventEnabled - ? SendRealMouseEnterExitWidgetEvent(aEvent, guid, blockId) - : SendNormalPriorityRealMouseEnterExitWidgetEvent(aEvent, guid, - blockId); - NS_WARNING_ASSERTION(ret, "SendRealMouseEnterExitWidgetEvent() failed"); - MOZ_ASSERT(!ret || aEvent.HasBeenPostedToRemoteProcess()); - return; - } - DebugOnly ret = isInputPriorityEventEnabled ? SendRealMouseButtonEvent(aEvent, guid, blockId) diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index c807e740b6a3..f172618b7d36 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/dom/ipc/PBrowser.ipdl @@ -792,14 +792,6 @@ child: ScrollableLayerGuid aGuid, uint64_t aInputBlockId); - [Priority=input] - async RealMouseEnterExitWidgetEvent(WidgetMouseEvent event, - ScrollableLayerGuid aGuid, - uint64_t aInputBlockId); - async NormalPriorityRealMouseEnterExitWidgetEvent(WidgetMouseEvent event, - ScrollableLayerGuid aGuid, - uint64_t aInputBlockId); - [Priority=input] async RealKeyEvent(WidgetKeyboardEvent event); async NormalPriorityRealKeyEvent(WidgetKeyboardEvent event); diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index adf72138ae30..291346fb47fb 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -661,10 +661,12 @@ bool XPCJSContext::InterruptCallback(JSContext* cx) { return true; } - // For content scripts, we only want to show the slow script dialogue if the - // user is actually trying to perform an important interaction. - if (runningContentJS && XRE_IsContentProcess() && - StaticPrefs::dom_max_script_run_time_require_critical_input()) { + int32_t limitWithoutImportantUserInput = + StaticPrefs::dom_max_script_run_time_without_important_user_input(); + if (runningContentJS && XRE_IsContentProcess() && limit && + limitWithoutImportantUserInput > limit && + limitWithoutImportantUserInput > + self->mSlowScriptActualWait.ToSeconds()) { // Call possibly slow PeekMessages after the other common early returns in // this method. ContentChild* contentChild = ContentChild::GetSingleton(); diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt index ef497415b336..e776ca755619 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt @@ -350,6 +350,7 @@ class ContentDelegateTest : BaseSessionTest() { private fun setHangReportTestPrefs(timeout: Int = 20000) { sessionRule.setPrefsUntilTestEnd(mapOf( "dom.max_script_run_time" to 1, + "dom.max_script_run_time_without_important_user_input" to 1, "dom.max_chrome_script_run_time" to 1, "dom.max_ext_content_script_run_time" to 1, "dom.ipc.cpow.timeout" to 100, diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 44da212e64fb..5e0399bbba38 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -3546,28 +3546,19 @@ value: @IS_ANDROID@ mirror: always -# The following three prefs control the maximum script run time before slow +# The following four prefs control the maximum script run time before slow # script warning. - -# Controls the time that a content script can run before showing a -# notification. - name: dom.max_script_run_time type: int32_t value: 10 mirror: always -# Controls whether we want to wait for user input before surfacing notifying -# the parent process about a long-running script. -- name: dom.max_script_run_time.require_critical_input - type: bool -# On desktop, we don't want to annoy the user with a notification if they're -# not interacting with the browser. On Android however, we automatically -# terminate long-running scripts, so we want to make sure we don't get in the -# way of that by waiting for input. -#if defined(MOZ_WIDGET_ANDROID) - value: false +- name: dom.max_script_run_time_without_important_user_input + type: int32_t +#ifdef NIGHTLY_BUILD + value: 20 #else - value: true + value: 10 #endif mirror: always