From e1e5972b07ddee5d640adbbcebe044f053aa4bd1 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 7 Jun 2018 09:57:36 +0900 Subject: [PATCH 01/51] Bug 1467327 - Use bootstrapped clang if no system clang is found. r=froydnj --HG-- extra : rebase_source : 4e646c27321d19d2d44df9911e5ef6fdb060ac48 --- build/moz.configure/toolchain.configure | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/build/moz.configure/toolchain.configure b/build/moz.configure/toolchain.configure index 8d3acd9dbb8c..26144a095252 100755 --- a/build/moz.configure/toolchain.configure +++ b/build/moz.configure/toolchain.configure @@ -638,22 +638,27 @@ def vc_compiler_path(host, target, vs_major_version, env, vs_release_name): @depends(vc_compiler_path) @imports('os') +@imports(_from='os', _import='environ') def toolchain_search_path(vc_compiler_path): + result = [environ.get('PATH')] + if vc_compiler_path: - result = [os.environ.get('PATH')] result.extend(vc_compiler_path) - # Also add in the location to which `mach bootstrap` or - # `mach artifact toolchain` installs clang. - mozbuild_state_dir = os.environ.get('MOZBUILD_STATE_PATH', - os.path.expanduser(os.path.join('~', '.mozbuild'))) - bootstrap_clang_path = os.path.join(mozbuild_state_dir, 'clang', 'bin') - result.append(bootstrap_clang_path) + # Also add in the location to which `mach bootstrap` or + # `mach artifact toolchain` installs clang. + mozbuild_state_dir = environ.get('MOZBUILD_STATE_PATH', + os.path.expanduser(os.path.join('~', '.mozbuild'))) + bootstrap_clang_path = os.path.join(mozbuild_state_dir, 'clang', 'bin') + result.append(bootstrap_clang_path) + if vc_compiler_path: # We're going to alter PATH for good in windows.configure, but we also - # need to do it for the valid_compiler() check below. - os.environ['PATH'] = os.pathsep.join(result) - return result + # need to do it for the valid_compiler() check below. This is only needed + # on Windows, where MSVC needs PATH set to find dlls. + environ['PATH'] = os.pathsep.join(result) + + return result @template From bd7dee7a251272150d83350c8dc8cabe22943678 Mon Sep 17 00:00:00 2001 From: Andreas Pehrson Date: Tue, 5 Jun 2018 14:45:44 +0200 Subject: [PATCH 02/51] Bug 1282264 - Disable test on android. r=padenot MozReview-Commit-ID: BASmW65yidn --HG-- extra : rebase_source : 518be793db10ffb132124778e4b8e47b4c9b68a7 --- dom/media/tests/mochitest/mochitest.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/dom/media/tests/mochitest/mochitest.ini b/dom/media/tests/mochitest/mochitest.ini index 022779569030..48900f28a44f 100644 --- a/dom/media/tests/mochitest/mochitest.ini +++ b/dom/media/tests/mochitest/mochitest.ini @@ -87,6 +87,7 @@ skip-if = android_version == '18' # android(Bug 1189784, timeouts on 4.3 emulato skip-if = toolkit == 'android' && debug # Bug 1282262 [test_getUserMedia_mediaStreamConstructors.html] [test_getUserMedia_mediaStreamTrackClone.html] +skip-if = android_version == '18' # android(Bug 1189784, timeouts on 4.3 emulator) [test_getUserMedia_playAudioTwice.html] [test_getUserMedia_playVideoAudioTwice.html] [test_getUserMedia_playVideoTwice.html] From 89595cee88e148c52ec9dfe0dbf95b5bbd9c7b97 Mon Sep 17 00:00:00 2001 From: sreeise Date: Sun, 27 May 2018 02:44:59 -0400 Subject: [PATCH 03/51] Bug 1158979 - Removed StringHelper.get and changed StringHelper.initialize to return the StringHelper instance. r=nechen Uses of StringHelper.get are replaced with the StringHelper instance in BaseRobocopTest.java and UITestContext. MozReview-Commit-ID: FfMhdNCX1vC --HG-- extra : rebase_source : c83977f5b9d2d70953c6a89aa30cd28501b5c994 --- .../src/org/mozilla/gecko/tests/BaseRobocopTest.java | 3 +-- .../robocop/src/org/mozilla/gecko/tests/StringHelper.java | 8 +------- .../org/mozilla/gecko/tests/helpers/GeckoClickHelper.java | 7 ++++--- .../gecko/tests/testActivityStreamPocketReferrer.java | 7 +++---- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java index 405b099d7dd2..fa92b5e4abef 100644 --- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java +++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java @@ -174,8 +174,7 @@ public abstract class BaseRobocopTest extends ActivityInstrumentationTestCase2 Date: Wed, 6 Jun 2018 16:03:44 +0200 Subject: [PATCH 04/51] Bug 1467117 - remove unneeded sort() of devtools definitions;r=daisuke MozReview-Commit-ID: JxZX4lRSasv --HG-- extra : rebase_source : bf26ed09cb9def1f9c5fd6a3bc40968e4ce4dffc --- .../client/framework/toolbox-tabs-order-manager.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/devtools/client/framework/toolbox-tabs-order-manager.js b/devtools/client/framework/toolbox-tabs-order-manager.js index f25f62e8f6ce..cc35041e7b9c 100644 --- a/devtools/client/framework/toolbox-tabs-order-manager.js +++ b/devtools/client/framework/toolbox-tabs-order-manager.js @@ -159,16 +159,6 @@ class ToolboxTabsOrderManager { function sortPanelDefinitions(definitions) { const pref = Services.prefs.getCharPref(PREFERENCE_NAME, ""); - - if (!pref) { - definitions.sort(definition => { - return -1 * (definition.ordinal == undefined || definition.ordinal < 0 - ? Number.MAX_VALUE - : definition.ordinal - ); - }); - } - const toolIds = pref.split(","); return definitions.sort((a, b) => { From 781f794ead91899c0b6c6eaef8c35e3840388ef1 Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Mon, 4 Jun 2018 16:12:22 +0200 Subject: [PATCH 05/51] Bug 1464846 - Font Editor: create new property with desired value instead of initial. r=pbro. MozReview-Commit-ID: IopuLJQ3NV0 --HG-- extra : rebase_source : d4a8587c616526c24a401a637bcfa7808ebe3430 --- devtools/client/inspector/fonts/fonts.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/devtools/client/inspector/fonts/fonts.js b/devtools/client/inspector/fonts/fonts.js index 0c59451d668b..187544a4f124 100644 --- a/devtools/client/inspector/fonts/fonts.js +++ b/devtools/client/inspector/fonts/fonts.js @@ -220,18 +220,20 @@ class FontInspector { /** * Get a reference to a TextProperty instance from the current selected rule for a - * given property name. If one doesn't exist, create and return a new one. + * given property name. If one doesn't exist, create one with the given value. * * @param {String} name * CSS property name + * @param {String} value + * CSS property value * @return {TextProperty} */ - getTextProperty(name) { + getTextProperty(name, value) { if (!this.textProperties.has(name)) { let textProperty = this.selectedRule.textProps.find(prop => prop.name === name); if (!textProperty) { - textProperty = this.selectedRule.editor.addProperty(name, "initial", "", true); + textProperty = this.selectedRule.editor.addProperty(name, value, "", true); } this.textProperties.set(name, textProperty); @@ -702,8 +704,8 @@ class FontInspector { * CSS property value */ updatePropertyValue(name, value) { - const textProperty = this.getTextProperty(name); - if (!textProperty) { + const textProperty = this.getTextProperty(name, value); + if (!textProperty || textProperty.value === value) { return; } From 2c5f7077224c80581e9718c0b2480cc1cc34b2fd Mon Sep 17 00:00:00 2001 From: Chris Pearce Date: Mon, 21 May 2018 14:19:47 +1200 Subject: [PATCH 06/51] Bug 1467350 - Make HTMLMediaElement::mPaused Watchable and update Wakelock status via a watcher. r=jya We currently observe changes to HTMLMediaElement::mPaused via a hand-rolled wrapper class. We can use use mozilla::Watchable<> and avoid rolling our own equivalent here. This also paves the way for using state watching on other observable state in HTMLMediaElement. MozReview-Commit-ID: 4lBlJiV15iG --HG-- extra : rebase_source : 22f9d811371f9d609dc96a9d958645e5c634eb17 extra : intermediate-source : bdb8280da440a711c6cd51b65ead7ba9e4bb3597 extra : source : fd8c4b8656a9996eea94d2092caaf3c10fe2a835 --- dom/html/HTMLMediaElement.cpp | 34 +++++++++++++--------------------- dom/html/HTMLMediaElement.h | 29 ++++++----------------------- 2 files changed, 19 insertions(+), 44 deletions(-) diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index bf40de10eb09..3698da37855d 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -3884,11 +3884,12 @@ NS_IMPL_ISUPPORTS(HTMLMediaElement::ShutdownObserver, nsIObserver) HTMLMediaElement::HTMLMediaElement( already_AddRefed& aNodeInfo) : nsGenericHTMLElement(aNodeInfo) + , mWatchManager(this, OwnerDoc()->AbstractMainThreadFor(TaskCategory::Other)) , mMainThreadEventTarget(OwnerDoc()->EventTargetFor(TaskCategory::Other)) , mAbstractMainThread(OwnerDoc()->AbstractMainThreadFor(TaskCategory::Other)) , mShutdownObserver(new ShutdownObserver) , mPlayed(new TimeRanges(ToSupports(OwnerDoc()))) - , mPaused(true, *this) + , mPaused(true, "HTMLMediaElement::mPaused") , mErrorSink(new ErrorSink(this)) , mAudioChannelWrapper(new AudioChannelAgentCallback(this)) { @@ -3897,6 +3898,8 @@ HTMLMediaElement::HTMLMediaElement( DecoderDoctorLogger::LogConstruction(this); + mWatchManager.Watch(mPaused, &HTMLMediaElement::UpdateWakeLock); + ErrorResult rv; double defaultVolume = Preferences::GetFloat("media.default_volume", 1.0); @@ -4180,31 +4183,20 @@ HTMLMediaElement::MaybeDoLoad() } } -HTMLMediaElement::WakeLockBoolWrapper& -HTMLMediaElement::WakeLockBoolWrapper::operator=(bool val) -{ - if (mValue == val) { - return *this; - } - - mValue = val; - UpdateWakeLock(); - return *this; -} - void -HTMLMediaElement::WakeLockBoolWrapper::UpdateWakeLock() +HTMLMediaElement::UpdateWakeLock() { MOZ_ASSERT(NS_IsMainThread()); - - bool playing = !mValue; + // Ensure we have a wake lock if we're playing audibly. This ensures the + // device doesn't sleep while playing. + bool playing = !mPaused; bool isAudible = - mOuter.Volume() > 0.0 && !mOuter.mMuted && mOuter.mIsAudioTrackAudible; - // when playing audible media. + Volume() > 0.0 && !mMuted && mIsAudioTrackAudible; + // WakeLock when playing audible media. if (playing && isAudible) { - mOuter.WakeLockCreate(); + WakeLockCreate(); } else { - mOuter.WakeLockRelease(); + WakeLockRelease(); } } @@ -7549,7 +7541,7 @@ HTMLMediaElement::NotifyAudioPlaybackChanged(AudibleChangedReasons aReason) mAudioChannelWrapper->NotifyAudioPlaybackChanged(aReason); } // only request wake lock for audible media. - mPaused.UpdateWakeLock(); + UpdateWakeLock(); } bool diff --git a/dom/html/HTMLMediaElement.h b/dom/html/HTMLMediaElement.h index 8037a918e0b3..d7c9b4540770 100644 --- a/dom/html/HTMLMediaElement.h +++ b/dom/html/HTMLMediaElement.h @@ -839,26 +839,7 @@ protected: void SetDecoder(MediaDecoder* aDecoder); - class WakeLockBoolWrapper { - public: - WakeLockBoolWrapper(bool aVal, HTMLMediaElement& aOuter) - : mValue(aVal) - , mOuter(aOuter) - {} - - ~WakeLockBoolWrapper() {}; - - MOZ_IMPLICIT operator bool() const { return mValue; } - - WakeLockBoolWrapper& operator=(bool val); - - bool operator !() const { return !mValue; } - - void UpdateWakeLock(); - private: - bool mValue; - HTMLMediaElement& mOuter; - }; + void UpdateWakeLock(); // Holds references to the DOM wrappers for the MediaStreams that we're // writing to. @@ -896,8 +877,8 @@ protected: void ChangeNetworkState(nsMediaNetworkState aState); /** - * These two methods are called by the WakeLockBoolWrapper when the wakelock - * has to be created or released. + * These two methods are called when mPaused is changed to ensure we have + * a wake lock active when we're playing audibly. */ virtual void WakeLockCreate(); virtual void WakeLockRelease(); @@ -1373,6 +1354,8 @@ protected: void PauseIfShouldNotBePlaying(); + WatchManager mWatchManager; + // The current decoder. Load() has been called on this decoder. // At most one of mDecoder and mSrcStream can be non-null. RefPtr mDecoder; @@ -1600,7 +1583,7 @@ protected: // Playback of the video is paused either due to calling the // 'Pause' method, or playback not yet having started. - WakeLockBoolWrapper mPaused; + Watchable mPaused; // The following two fields are here for the private storage of the builtin // video controls, and control 'casting' of the video to external devices From 3623389a1b92381bd1121ca26ddbb2f114934b02 Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Thu, 7 Jun 2018 11:42:11 +0000 Subject: [PATCH 07/51] Bug 1466929 - Fix intermittent issues in browser_bookmarkProperties_remember_folders.js. r=mak Ensure we wait for the onItemMoved notification before proceeding, to avoid async issues. Also improve the wait for checks to provide better debug. Differential Revision: https://phabricator.services.mozilla.com/D1579 --- ...ser_bookmarkProperties_remember_folders.js | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/browser/components/places/tests/browser/browser_bookmarkProperties_remember_folders.js b/browser/components/places/tests/browser/browser_bookmarkProperties_remember_folders.js index cd42861ced47..b3392a07c3cf 100644 --- a/browser/components/places/tests/browser/browser_bookmarkProperties_remember_folders.js +++ b/browser/components/places/tests/browser/browser_bookmarkProperties_remember_folders.js @@ -24,23 +24,42 @@ async function hideBookmarksPanel() { await hiddenPromise; } -async function openPopupAndSelectFolder(guid) { +async function openPopupAndSelectFolder(guid, newBookmark = false) { await clickBookmarkStar(); + let notificationPromise; + if (!newBookmark) { + notificationPromise = PlacesTestUtils.waitForNotification("onItemMoved", + (id, oldParentId, oldIndex, newParentId, newIndex, type, + itemGuid, oldParentGuid, newParentGuid) => guid == newParentGuid); + } + // Expand the folder tree. document.getElementById("editBMPanel_foldersExpander").click(); document.getElementById("editBMPanel_folderTree").selectItems([guid]); await hideBookmarksPanel(); - // Ensure the meta data has had chance to be written to disk. - await PlacesTestUtils.promiseAsyncUpdates(); + if (!newBookmark) { + await notificationPromise; + } } async function assertRecentFolders(expectedGuids, msg) { + // Give the metadata chance to be written to the database before we attempt + // to open the dialog again. + let diskGuids = []; + await TestUtils.waitForCondition(async () => { + diskGuids = await PlacesUtils.metadata.get(PlacesUIUtils.LAST_USED_FOLDERS_META_KEY, []); + return diskGuids.length == expectedGuids.length; + }, `Should have written data to disk for: ${msg}`); + + Assert.deepEqual(diskGuids, expectedGuids, `Should match the disk GUIDS for ${msg}`); + await clickBookmarkStar(); let actualGuids = []; function getGuids() { + actualGuids = []; const folderMenuPopup = document.getElementById("editBMPanel_folderMenuList").children[0]; let separatorFound = false; @@ -54,10 +73,12 @@ async function assertRecentFolders(expectedGuids, msg) { } } + // The dialog fills in the folder list asnychronously, so we might need to wait + // for that to complete. await TestUtils.waitForCondition(() => { getGuids(); return actualGuids.length == expectedGuids.length; - }, msg); + }, `Should have opened dialog with expected recent folders for: ${msg}`); Assert.deepEqual(actualGuids, expectedGuids, msg); @@ -115,7 +136,7 @@ add_task(async function setup() { add_task(async function test_remember_last_folder() { await assertRecentFolders([], "Should have no recent folders to start with."); - await openPopupAndSelectFolder(folders[0].guid); + await openPopupAndSelectFolder(folders[0].guid, true); await assertRecentFolders([folders[0].guid], "Should have one folder in the list."); }); From aa581b77ce344f15f5a27c09e7283abeee5bb380 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Wed, 6 Jun 2018 20:42:20 +0200 Subject: [PATCH 08/51] Bug 1467138 - Fix disable/enable extensions from Fennec about:addons page. r=aswan This patch fixes a regression introduced by Bug 1461146 ("Replace Addon.userDisabled setter with async enable()/disable() methods"). MozReview-Commit-ID: 3VDhMaXxENp --HG-- extra : rebase_source : 80c5955af064d8c268ecd8a2b6f165a409fd752b --- mobile/android/chrome/content/aboutAddons.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mobile/android/chrome/content/aboutAddons.js b/mobile/android/chrome/content/aboutAddons.js index eb7afa26eee2..4fc1d579b59f 100644 --- a/mobile/android/chrome/content/aboutAddons.js +++ b/mobile/android/chrome/content/aboutAddons.js @@ -521,9 +521,9 @@ var Addons = { function setDisabled(addon, value) { if (value) { - return addon.enable(); + return addon.disable(); } - return addon.disable(); + return addon.enable(); } let opType; From d0788c7eded2f830e2a51ebb42332b9227fbe4fb Mon Sep 17 00:00:00 2001 From: Jan Odvarko Date: Thu, 7 Jun 2018 14:51:40 +0200 Subject: [PATCH 09/51] Bug 1465594 - Ensure proper order of XHRs; r=jryans MozReview-Commit-ID: BdieFClyunE --HG-- extra : rebase_source : 74b95c1c10f027a44fc80db18118f455f11b46c5 --- .../test/html_params-test-page.html | 73 ++++++++----------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/devtools/client/netmonitor/test/html_params-test-page.html b/devtools/client/netmonitor/test/html_params-test-page.html index 26a74aa29b4f..5a4f49ebb0f3 100644 --- a/devtools/client/netmonitor/test/html_params-test-page.html +++ b/devtools/client/netmonitor/test/html_params-test-page.html @@ -18,53 +18,42 @@ /* exported performRequests */ "use strict"; - function get(address, query) { - const xhr = new XMLHttpRequest(); - xhr.open("GET", address + query, true); - xhr.send(); + async function get(address, query) { + return new Promise(resolve => { + const xhr = new XMLHttpRequest(); + xhr.open("GET", address + query, true); + xhr.onreadystatechange = function() { + if (this.readyState == this.DONE) { + resolve(); + } + }; + xhr.send(); + }); } - function post(address, query, contentType, postBody) { - const xhr = new XMLHttpRequest(); - xhr.open("POST", address + query, true); - xhr.setRequestHeader("content-type", contentType); - xhr.send(postBody); + async function post(address, query, contentType, postBody) { + return new Promise(resolve => { + const xhr = new XMLHttpRequest(); + xhr.open("POST", address + query, true); + xhr.setRequestHeader("content-type", contentType); + xhr.onreadystatechange = function() { + if (this.readyState == this.DONE) { + resolve(); + } + }; + xhr.send(postBody); + }); } - function performRequests() { + async function performRequests() { const urlencoded = "application/x-www-form-urlencoded"; - - /* eslint-disable max-nested-callbacks */ - setTimeout(function() { - post("baz", "?a", urlencoded, '{ "foo": "bar" }'); - - setTimeout(function() { - post("baz", "?a=b", urlencoded, '{ "foo": "bar" }'); - - setTimeout(function() { - post("baz", "?a=b", urlencoded, "?foo=bar"); - - setTimeout(function() { - post("baz", "?a", undefined, '{ "foo": "bar" }'); - - setTimeout(function() { - post("baz", "?a=b", undefined, '{ "foo": "bar" }'); - - setTimeout(function() { - post("baz", "?a=b", undefined, "?foo=bar"); - - setTimeout(function() { - get("baz", ""); - - // Done. - }, 10); - }, 10); - }, 10); - }, 10); - }, 10); - }, 10); - }, 10); - /* eslint-enable max-nested-callbacks */ + await post("baz", "?a", urlencoded, '{ "foo": "bar" }'); + await post("baz", "?a=b", urlencoded, '{ "foo": "bar" }'); + await post("baz", "?a=b", urlencoded, "?foo=bar"); + await post("baz", "?a", undefined, '{ "foo": "bar" }'); + await post("baz", "?a=b", undefined, '{ "foo": "bar" }'); + await post("baz", "?a=b", undefined, "?foo=bar"); + await get("baz", ""); } From eac18ee71f0b2ffbcba857a390e5e1a26445fc5a Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Thu, 7 Jun 2018 13:01:09 +0200 Subject: [PATCH 10/51] Bug 1467232 - Clear the explanation text first, before adding any changed text. r=Dexter MozReview-Commit-ID: Po12SIQdZm --HG-- extra : rebase_source : 997d4001091e93e0ece6fa0aebea5abed18b4edc --- toolkit/content/aboutTelemetry.js | 1 + 1 file changed, 1 insertion(+) diff --git a/toolkit/content/aboutTelemetry.js b/toolkit/content/aboutTelemetry.js index f8fc7580d7a9..e9f55e60650f 100644 --- a/toolkit/content/aboutTelemetry.js +++ b/toolkit/content/aboutTelemetry.js @@ -340,6 +340,7 @@ var PingPicker = { } let pingExplanation = document.getElementById("ping-explanation"); + removeAllChildNodes(pingExplanation); pingExplanation.appendChild(fragment); pingExplanation.querySelector(".change-ping").addEventListener("click", (ev) => { document.getElementById("ping-picker").classList.remove("hidden"); From 2490c2851c7ab97a4ad4cc1354b4c78070a156d6 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Thu, 7 Jun 2018 13:01:51 +0200 Subject: [PATCH 11/51] Bug 1467232 - Ensure ping explanation is formatted correctly. r=Dexter The "pingDetails" explanation takes only 2 arguments: a link and the name of the ping to be displayed. MozReview-Commit-ID: CxLnHb73YGu --HG-- extra : rebase_source : f71d9c75c2fe10191d3f8c60be23707009ab8e7d --- toolkit/content/aboutTelemetry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/content/aboutTelemetry.js b/toolkit/content/aboutTelemetry.js index e9f55e60650f..544f340d8dd7 100644 --- a/toolkit/content/aboutTelemetry.js +++ b/toolkit/content/aboutTelemetry.js @@ -327,7 +327,7 @@ var PingPicker = { pingName = bundle.formatStringFromName("namedPing", [pingName, pingTypeText], 2); pingNameSpan.textContent = pingName; let explanation = bundle.GetStringFromName("pingDetails"); - fragment = BrowserUtils.getLocalizedFragment(document, explanation, pingLink, pingNameSpan, pingTypeText); + fragment = BrowserUtils.getLocalizedFragment(document, explanation, pingLink, pingNameSpan); } else { // Change sidebar heading text. controls.classList.add("hidden"); From df7df2196d2050a2a09ecfeedac0d3d3c05249c3 Mon Sep 17 00:00:00 2001 From: Andreas Tolfsen Date: Wed, 6 Jun 2018 18:31:28 +0100 Subject: [PATCH 12/51] Bug 1467215 - Abort initialisation on fatal error. r=maja_zf When the Marionette component fails to initialise we fail to bail out. This causes Marionette to report itself as enabled through an environment variable, an observer notification, and a log message. MozReview-Commit-ID: 2nTNNP0o5dv --HG-- extra : rebase_source : 3b780bf2f24e03ba796d50cf5094c28d951847ec --- testing/marionette/components/marionette.js | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/marionette/components/marionette.js b/testing/marionette/components/marionette.js index 08682e01a7b7..fdf3322f8033 100644 --- a/testing/marionette/components/marionette.js +++ b/testing/marionette/components/marionette.js @@ -449,6 +449,7 @@ class MarionetteMainProcess { log.fatal("Remote protocol server failed to start", e); this.uninit(); Services.startup.quit(Ci.nsIAppStartup.eForceQuit); + return; } env.set(ENV_ENABLED, "1"); From 62e21cf10386f6e85333880dd15f464cec2dd26e Mon Sep 17 00:00:00 2001 From: Andreas Tolfsen Date: Wed, 6 Jun 2018 18:34:32 +0100 Subject: [PATCH 13/51] Bug 1467215 - Reset prefs possibly set by failed init. r=maja_zf If initialisation fails on being unable to start the TCP listener, we may have already set some recommended preferences. This patch ensures any altered preferences get reset when uninit() is called in the try...catch block in init(). MozReview-Commit-ID: B5vNvTUZcO7 --HG-- extra : rebase_source : 1cab7dfdf9c60d0f294df7a0bd0356041319d161 --- testing/marionette/components/marionette.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/testing/marionette/components/marionette.js b/testing/marionette/components/marionette.js index fdf3322f8033..cf6e2dd3431a 100644 --- a/testing/marionette/components/marionette.js +++ b/testing/marionette/components/marionette.js @@ -459,14 +459,14 @@ class MarionetteMainProcess { } uninit() { + for (let k of this.alteredPrefs) { + log.debug(`Resetting recommended pref ${k}`); + Preferences.reset(k); + } + this.alteredPrefs.clear(); + if (this.running) { this.server.stop(); - for (let k of this.alteredPrefs) { - log.debug(`Resetting recommended pref ${k}`); - Preferences.reset(k); - } - this.alteredPrefs.clear(); - Services.obs.notifyObservers(this, NOTIFY_RUNNING); log.debug("Remote service is inactive"); } From 28f5f5d4a406578df0c7af52213508fe194ab97a Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Fri, 1 Jun 2018 15:35:32 +1000 Subject: [PATCH 14/51] Bug 1423017 - Add a telemetry for out-of-reach overflowing on root. r=botond MozReview-Commit-ID: 2CyZTVBFP59 --HG-- extra : rebase_source : 0f1d42fd65325aa6db8cbbe14172f0f7b7357110 --- dom/base/nsDocument.cpp | 68 ++++++++++++++++++++ dom/base/nsIDocument.h | 37 ++++++++++- layout/generic/nsGfxScrollFrame.cpp | 21 ++++-- toolkit/components/telemetry/Histograms.json | 9 +++ 4 files changed, 129 insertions(+), 6 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 5a97d28d9237..ca318e57cc37 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1498,6 +1498,7 @@ nsIDocument::nsIDocument() mStackRefCnt(0), mUpdateNestLevel(0), mViewportType(Unknown), + mViewportOverflowType(ViewportOverflowType::NoOverflow), mSubDocuments(nullptr), mHeaderData(nullptr), mFlashClassification(FlashClassification::Unclassified), @@ -3749,6 +3750,7 @@ nsIDocument::SetHeaderData(nsAtom* aHeaderField, const nsAString& aData) aHeaderField == nsGkAtoms::viewport_width || aHeaderField == nsGkAtoms::viewport_user_scalable) { mViewportType = Unknown; + mViewportOverflowType = ViewportOverflowType::NoOverflow; } // Referrer policy spec says to ignore any empty referrer policies. @@ -7296,6 +7298,7 @@ nsIDocument::GetViewportInfo(const ScreenIntSize& aDisplaySize) mValidMaxScale = !maxScaleStr.IsEmpty() && NS_SUCCEEDED(scaleMaxErrorCode); mViewportType = Specified; + mViewportOverflowType = ViewportOverflowType::NoOverflow; MOZ_FALLTHROUGH; } case Specified: @@ -7377,6 +7380,54 @@ nsIDocument::GetViewportInfo(const ScreenIntSize& aDisplaySize) } } +void +nsIDocument::UpdateViewportOverflowType(nscoord aScrolledWidth, + nscoord aScrollportWidth) +{ +#ifdef DEBUG + MOZ_ASSERT(mPresShell); + nsPresContext* pc = GetPresContext(); + MOZ_ASSERT(pc->GetViewportScrollbarStylesOverride().mHorizontal == + NS_STYLE_OVERFLOW_HIDDEN, + "Should only be called when viewport has overflow-x: hidden"); + MOZ_ASSERT(aScrolledWidth > aScrollportWidth, + "Should only be called when viewport is overflowed"); + MOZ_ASSERT(IsTopLevelContentDocument(), + "Should only be called for top-level content document"); +#endif // DEBUG + + if (!gfxPrefs::MetaViewportEnabled() || + (GetWindow() && GetWindow()->IsDesktopModeViewport())) { + mViewportOverflowType = ViewportOverflowType::Desktop; + return; + } + + static const LayoutDeviceToScreenScale + kBlinkDefaultMinScale = LayoutDeviceToScreenScale(0.25f); + LayoutDeviceToScreenScale minScale; + if (mViewportType == DisplayWidthHeight) { + minScale = kBlinkDefaultMinScale; + } else { + MOZ_ASSERT(mViewportType == Specified, + "Viewport information should have been initialized"); + if (mScaleMinFloat == kViewportMinScale) { + minScale = kBlinkDefaultMinScale; + } else { + minScale = mScaleMinFloat; + } + } + + // If the content has overflowed with minimum scale applied, don't + // change it, otherwise update the overflow type. + if (mViewportOverflowType != ViewportOverflowType::MinScaleSize) { + if (aScrolledWidth * minScale.scale < aScrollportWidth) { + mViewportOverflowType = ViewportOverflowType::ButNotMinScaleSize; + } else { + mViewportOverflowType = ViewportOverflowType::MinScaleSize; + } + } +} + EventListenerManager* nsDocument::GetOrCreateListenerManager() { @@ -12191,6 +12242,23 @@ nsIDocument::ReportUseCounters(UseCounterReportKind aKind) } } } + + if (IsTopLevelContentDocument() && !IsResourceDoc()) { + using mozilla::Telemetry::LABELS_HIDDEN_VIEWPORT_OVERFLOW_TYPE; + LABELS_HIDDEN_VIEWPORT_OVERFLOW_TYPE label; + switch (mViewportOverflowType) { +#define CASE_OVERFLOW_TYPE(t_) \ + case ViewportOverflowType::t_: \ + label = LABELS_HIDDEN_VIEWPORT_OVERFLOW_TYPE::t_; \ + break; + CASE_OVERFLOW_TYPE(NoOverflow) + CASE_OVERFLOW_TYPE(Desktop) + CASE_OVERFLOW_TYPE(ButNotMinScaleSize) + CASE_OVERFLOW_TYPE(MinScaleSize) +#undef CASE_OVERFLOW_TYPE + } + Telemetry::AccumulateCategorical(label); + } } void diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h index ed42f7625b0f..d5209ed21383 100644 --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -1200,6 +1200,17 @@ public: */ nsViewportInfo GetViewportInfo(const mozilla::ScreenIntSize& aDisplaySize); + /** + * It updates the viewport overflow type with the given two widths + * and the viewport setting of the document. + * This should only be called when there is out-of-reach overflow + * happens on the viewport, i.e. the viewport should be using + * `overflow: hidden`. And it should only be called on a top level + * content document. + */ + void UpdateViewportOverflowType(nscoord aScrolledWidth, + nscoord aScrollportWidth); + /** * True iff this doc will ignore manual character encoding overrides. */ @@ -4277,7 +4288,7 @@ protected: // Our update nesting level uint32_t mUpdateNestLevel; - enum ViewportType { + enum ViewportType : uint8_t { DisplayWidthHeight, Specified, Unknown @@ -4285,6 +4296,30 @@ protected: ViewportType mViewportType; + // Enum for how content in this document overflows viewport causing + // out-of-reach issue. Currently it only takes horizontal overflow + // into consideration. This enum and the corresponding field is only + // set and read on a top level content document. + enum class ViewportOverflowType : uint8_t { + // Viewport doesn't have out-of-reach overflow content, either + // because the content doesn't overflow, or the viewport doesn't + // have "overflow: hidden". + NoOverflow, + + // All following items indicates that the content overflows the + // scroll port which causing out-of-reach content. + + // Meta viewport is disabled or the document is in desktop mode. + Desktop, + // The content does not overflow the minimum-scale size. When there + // is no minimum scale specified, the default value used by Blink, + // 0.25, is used for this matter. + ButNotMinScaleSize, + // The content overflows the minimum-scale size. + MinScaleSize, + }; + ViewportOverflowType mViewportOverflowType; + PLDHashTable* mSubDocuments; nsDocHeaderData* mHeaderData; diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index f872d9762950..1e43362bfc0f 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -395,12 +395,12 @@ nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState, std::max(0, compositionSize.height - hScrollbarDesiredHeight)); } - if (!aForce) { - nsRect scrolledRect = - mHelper.GetUnsnappedScrolledRectInternal(aState->mContentsOverflowAreas.ScrollableOverflow(), - scrollPortSize); - nscoord oneDevPixel = aState->mBoxState.PresContext()->DevPixelsToAppUnits(1); + nsRect scrolledRect = + mHelper.GetUnsnappedScrolledRectInternal(aState->mContentsOverflowAreas.ScrollableOverflow(), + scrollPortSize); + nscoord oneDevPixel = aState->mBoxState.PresContext()->DevPixelsToAppUnits(1); + if (!aForce) { // If the style is HIDDEN then we already know that aAssumeHScroll is false if (aState->mStyles.mHorizontal != NS_STYLE_OVERFLOW_HIDDEN) { bool wantHScrollbar = @@ -426,6 +426,17 @@ nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState, } } + if (mHelper.mIsRoot && + aState->mStyles.mHorizontal == NS_STYLE_OVERFLOW_HIDDEN) { + nscoord scrolledWidth = scrolledRect.width + oneDevPixel; + if (scrolledWidth > scrollPortSize.width) { + nsIDocument* doc = PresContext()->Document(); + if (doc->IsTopLevelContentDocument()) { + doc->UpdateViewportOverflowType(scrolledWidth, scrollPortSize.width); + } + } + } + nscoord vScrollbarActualWidth = aState->mInsideBorderSize.width - scrollPortSize.width; aState->mShowHScrollbar = aAssumeHScroll; diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 003e4507e2fe..026ed42acdbd 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -13804,5 +13804,14 @@ "high": 50, "n_buckets": 20, "description": "Total number of doc groups per tab group, including docgroups fully in bfcache. Collected at the point when the top level document of the tab group is unloaded." + }, + "HIDDEN_VIEWPORT_OVERFLOW_TYPE": { + "record_in_processes": ["content"], + "alert_emails": ["xquan@mozilla.com", "botond@mozilla.com"], + "bug_numbers": [1423013, 1423017], + "expires_in_version": "65", + "kind": "categorical", + "labels": ["NoOverflow", "Desktop", "ButNotMinScaleSize", "MinScaleSize"], + "description": "How common are different types of out-of-reach viewport overflow?" } } From 5ce8f98cbacb5863f14b6d2f708d586854acbdba Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Mon, 4 Jun 2018 19:17:32 +1000 Subject: [PATCH 15/51] Bug 1452204 part 1 - Correctly set walkCallingThread. r=glandium GetCurrentThread() returns a pseudo handle, so comparing it against the passed in argument doesn't make sense in most cases. This patch changes it to using the thread id for comparison, which is guaranteed to be unique in the whole lifetime of a thread. MozReview-Commit-ID: 5TNAgLkcS6m --HG-- extra : rebase_source : d5bb21ac57a4c1149b8d332ea7b28a78ed994c62 --- mozglue/misc/StackWalk.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/mozglue/misc/StackWalk.cpp b/mozglue/misc/StackWalk.cpp index 8313edb455d7..6c98d75649f3 100644 --- a/mozglue/misc/StackWalk.cpp +++ b/mozglue/misc/StackWalk.cpp @@ -507,9 +507,15 @@ MozStackWalkThread(MozWalkStackCallback aCallback, uint32_t aSkipFrames, return; } - HANDLE currentThread = ::GetCurrentThread(); - HANDLE targetThread = aThread ? aThread : currentThread; - data.walkCallingThread = (targetThread == currentThread); + HANDLE targetThread = aThread; + if (!aThread) { + targetThread = ::GetCurrentThread(); + data.walkCallingThread = true; + } else { + DWORD threadId = ::GetThreadId(aThread); + DWORD currentThreadId = ::GetCurrentThreadId(); + data.walkCallingThread = (threadId == currentThreadId); + } // Have to duplicate handle to get a real handle. if (!myProcess) { From 75cc8c371bae3331eb55bc6b234b80a6c381102e Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Mon, 4 Jun 2018 19:23:27 +1000 Subject: [PATCH 16/51] Bug 1452204 part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread. r=glandium GetThreadContext() returns a context pointing to its own frame when it gets called with the current thread handle. That frame can go away after it returns. This patch instead uses RtlCaptureContext(), which captures the context of its caller, when walking the current thread. In the past, we also used a walker thread when nullptr is passed in for aThread, but the check doesn't cover all the cases, and having another thread is apparently more complicated than this approach. MozReview-Commit-ID: 3TAatDc9BLh --HG-- extra : rebase_source : 7978cce48b8939a723cd5ccafe86d3f7aca6d3ac --- mozglue/misc/StackWalk.cpp | 175 +++---------------------------------- 1 file changed, 12 insertions(+), 163 deletions(-) diff --git a/mozglue/misc/StackWalk.cpp b/mozglue/misc/StackWalk.cpp index 6c98d75649f3..58cd5fc65dd0 100644 --- a/mozglue/misc/StackWalk.cpp +++ b/mozglue/misc/StackWalk.cpp @@ -95,7 +95,6 @@ struct WalkStackData CONTEXT* context; }; -DWORD gStackWalkThread; CRITICAL_SECTION gDbgHelpCS; #ifdef _M_AMD64 @@ -189,61 +188,6 @@ InitializeDbgHelpCriticalSection() initialized = true; } -static unsigned int WINAPI WalkStackThread(void* aData); - -static bool -EnsureWalkThreadReady() -{ - static bool walkThreadReady = false; - static HANDLE stackWalkThread = nullptr; - static HANDLE readyEvent = nullptr; - - if (walkThreadReady) { - return walkThreadReady; - } - - if (!stackWalkThread) { - readyEvent = ::CreateEvent(nullptr, FALSE /* auto-reset*/, - FALSE /* initially non-signaled */, - nullptr); - if (!readyEvent) { - PrintError("CreateEvent"); - return false; - } - - unsigned int threadID; - stackWalkThread = (HANDLE)_beginthreadex(nullptr, 0, WalkStackThread, - (void*)readyEvent, 0, &threadID); - if (!stackWalkThread) { - PrintError("CreateThread"); - ::CloseHandle(readyEvent); - readyEvent = nullptr; - return false; - } - gStackWalkThread = threadID; - ::CloseHandle(stackWalkThread); - } - - MOZ_ASSERT((stackWalkThread && readyEvent) || - (!stackWalkThread && !readyEvent)); - - // The thread was created. Try to wait an arbitrary amount of time (1 second - // should be enough) for its event loop to start before posting events to it. - DWORD waitRet = ::WaitForSingleObject(readyEvent, 1000); - if (waitRet == WAIT_TIMEOUT) { - // We get a timeout if we're called during static initialization because - // the thread will only start executing after we return so it couldn't - // have signalled the event. If that is the case, give up for now and - // try again next time we're called. - return false; - } - ::CloseHandle(readyEvent); - stackWalkThread = nullptr; - readyEvent = nullptr; - - return walkThreadReady = true; -} - static void WalkStackMain64(struct WalkStackData* aData) { @@ -254,10 +198,9 @@ WalkStackMain64(struct WalkStackData* aData) context = &context_buf; memset(context, 0, sizeof(CONTEXT)); context->ContextFlags = CONTEXT_FULL; - if (!GetThreadContext(aData->thread, context)) { - if (aData->walkCallingThread) { - PrintError("GetThreadContext"); - } + if (aData->walkCallingThread) { + ::RtlCaptureContext(context); + } else if (!GetThreadContext(aData->thread, context)) { return; } } else { @@ -428,59 +371,6 @@ WalkStackMain64(struct WalkStackData* aData) } } -static unsigned int WINAPI -WalkStackThread(void* aData) -{ - BOOL msgRet; - MSG msg; - - // Call PeekMessage to force creation of a message queue so that - // other threads can safely post events to us. - ::PeekMessage(&msg, nullptr, WM_USER, WM_USER, PM_NOREMOVE); - - // and tell the thread that created us that we're ready. - HANDLE readyEvent = (HANDLE)aData; - ::SetEvent(readyEvent); - - while ((msgRet = ::GetMessage(&msg, (HWND)-1, 0, 0)) != 0) { - if (msgRet == -1) { - PrintError("GetMessage"); - } else { - DWORD ret; - - struct WalkStackData* data = (WalkStackData*)msg.lParam; - if (!data) { - continue; - } - - // Don't suspend the calling thread until it's waiting for - // us; otherwise the number of frames on the stack could vary. - ret = ::WaitForSingleObject(data->eventStart, INFINITE); - if (ret != WAIT_OBJECT_0) { - PrintError("WaitForSingleObject"); - } - - // Suspend the calling thread, dump his stack, and then resume him. - // He's currently waiting for us to finish so now should be a good time. - ret = ::SuspendThread(data->thread); - if (ret == (DWORD)-1) { - PrintError("ThreadSuspend"); - } else { - WalkStackMain64(data); - - ret = ::ResumeThread(data->thread); - if (ret == (DWORD)-1) { - PrintError("ThreadResume"); - } - } - - ::SetEvent(data->eventEnd); - } - } - - return 0; -} - /** * Walk the stack, translating PC's found into strings and recording the * chain in aBuffer. For this to work properly, the DLLs must be rebased @@ -496,17 +386,10 @@ MozStackWalkThread(MozWalkStackCallback aCallback, uint32_t aSkipFrames, { static HANDLE myProcess = nullptr; HANDLE myThread; - DWORD walkerReturn; struct WalkStackData data; InitializeDbgHelpCriticalSection(); - // EnsureWalkThreadReady's _beginthreadex takes a heap lock and must be - // avoided if we're walking another (i.e. suspended) thread. - if (!aThread && !EnsureWalkThreadReady()) { - return; - } - HANDLE targetThread = aThread; if (!aThread) { targetThread = ::GetCurrentThread(); @@ -555,50 +438,16 @@ MozStackWalkThread(MozWalkStackCallback aCallback, uint32_t aSkipFrames, data.sp_size = ArrayLength(local_sps); data.context = aContext; - if (aThread) { - // If we're walking the stack of another thread, we don't need to - // use a separate walker thread. + WalkStackMain64(&data); + + if (data.pc_count > data.pc_size) { + data.pcs = (void**)_alloca(data.pc_count * sizeof(void*)); + data.pc_size = data.pc_count; + data.pc_count = 0; + data.sps = (void**)_alloca(data.sp_count * sizeof(void*)); + data.sp_size = data.sp_count; + data.sp_count = 0; WalkStackMain64(&data); - - if (data.pc_count > data.pc_size) { - data.pcs = (void**)_alloca(data.pc_count * sizeof(void*)); - data.pc_size = data.pc_count; - data.pc_count = 0; - data.sps = (void**)_alloca(data.sp_count * sizeof(void*)); - data.sp_size = data.sp_count; - data.sp_count = 0; - WalkStackMain64(&data); - } - } else { - data.eventStart = ::CreateEvent(nullptr, FALSE /* auto-reset*/, - FALSE /* initially non-signaled */, nullptr); - data.eventEnd = ::CreateEvent(nullptr, FALSE /* auto-reset*/, - FALSE /* initially non-signaled */, nullptr); - - ::PostThreadMessage(gStackWalkThread, WM_USER, 0, (LPARAM)&data); - - walkerReturn = ::SignalObjectAndWait(data.eventStart, - data.eventEnd, INFINITE, FALSE); - if (walkerReturn != WAIT_OBJECT_0 && data.walkCallingThread) { - PrintError("SignalObjectAndWait (1)"); - } - if (data.pc_count > data.pc_size) { - data.pcs = (void**)_alloca(data.pc_count * sizeof(void*)); - data.pc_size = data.pc_count; - data.pc_count = 0; - data.sps = (void**)_alloca(data.sp_count * sizeof(void*)); - data.sp_size = data.sp_count; - data.sp_count = 0; - ::PostThreadMessage(gStackWalkThread, WM_USER, 0, (LPARAM)&data); - walkerReturn = ::SignalObjectAndWait(data.eventStart, - data.eventEnd, INFINITE, FALSE); - if (walkerReturn != WAIT_OBJECT_0 && data.walkCallingThread) { - PrintError("SignalObjectAndWait (2)"); - } - } - - ::CloseHandle(data.eventStart); - ::CloseHandle(data.eventEnd); } ::CloseHandle(myThread); From de691e06d2b69bbc8a0b0f52362b784589ec6513 Mon Sep 17 00:00:00 2001 From: Coroiu Cristina Date: Thu, 7 Jun 2018 17:36:02 +0300 Subject: [PATCH 17/51] Backed out changeset ad8f78a9b78c (bug 1423017) for causing multiple failures at build/src/dom/base/nsDocument.cpp on a CLOSED TREE --- dom/base/nsDocument.cpp | 68 -------------------- dom/base/nsIDocument.h | 37 +---------- layout/generic/nsGfxScrollFrame.cpp | 21 ++---- toolkit/components/telemetry/Histograms.json | 9 --- 4 files changed, 6 insertions(+), 129 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index ca318e57cc37..5a97d28d9237 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1498,7 +1498,6 @@ nsIDocument::nsIDocument() mStackRefCnt(0), mUpdateNestLevel(0), mViewportType(Unknown), - mViewportOverflowType(ViewportOverflowType::NoOverflow), mSubDocuments(nullptr), mHeaderData(nullptr), mFlashClassification(FlashClassification::Unclassified), @@ -3750,7 +3749,6 @@ nsIDocument::SetHeaderData(nsAtom* aHeaderField, const nsAString& aData) aHeaderField == nsGkAtoms::viewport_width || aHeaderField == nsGkAtoms::viewport_user_scalable) { mViewportType = Unknown; - mViewportOverflowType = ViewportOverflowType::NoOverflow; } // Referrer policy spec says to ignore any empty referrer policies. @@ -7298,7 +7296,6 @@ nsIDocument::GetViewportInfo(const ScreenIntSize& aDisplaySize) mValidMaxScale = !maxScaleStr.IsEmpty() && NS_SUCCEEDED(scaleMaxErrorCode); mViewportType = Specified; - mViewportOverflowType = ViewportOverflowType::NoOverflow; MOZ_FALLTHROUGH; } case Specified: @@ -7380,54 +7377,6 @@ nsIDocument::GetViewportInfo(const ScreenIntSize& aDisplaySize) } } -void -nsIDocument::UpdateViewportOverflowType(nscoord aScrolledWidth, - nscoord aScrollportWidth) -{ -#ifdef DEBUG - MOZ_ASSERT(mPresShell); - nsPresContext* pc = GetPresContext(); - MOZ_ASSERT(pc->GetViewportScrollbarStylesOverride().mHorizontal == - NS_STYLE_OVERFLOW_HIDDEN, - "Should only be called when viewport has overflow-x: hidden"); - MOZ_ASSERT(aScrolledWidth > aScrollportWidth, - "Should only be called when viewport is overflowed"); - MOZ_ASSERT(IsTopLevelContentDocument(), - "Should only be called for top-level content document"); -#endif // DEBUG - - if (!gfxPrefs::MetaViewportEnabled() || - (GetWindow() && GetWindow()->IsDesktopModeViewport())) { - mViewportOverflowType = ViewportOverflowType::Desktop; - return; - } - - static const LayoutDeviceToScreenScale - kBlinkDefaultMinScale = LayoutDeviceToScreenScale(0.25f); - LayoutDeviceToScreenScale minScale; - if (mViewportType == DisplayWidthHeight) { - minScale = kBlinkDefaultMinScale; - } else { - MOZ_ASSERT(mViewportType == Specified, - "Viewport information should have been initialized"); - if (mScaleMinFloat == kViewportMinScale) { - minScale = kBlinkDefaultMinScale; - } else { - minScale = mScaleMinFloat; - } - } - - // If the content has overflowed with minimum scale applied, don't - // change it, otherwise update the overflow type. - if (mViewportOverflowType != ViewportOverflowType::MinScaleSize) { - if (aScrolledWidth * minScale.scale < aScrollportWidth) { - mViewportOverflowType = ViewportOverflowType::ButNotMinScaleSize; - } else { - mViewportOverflowType = ViewportOverflowType::MinScaleSize; - } - } -} - EventListenerManager* nsDocument::GetOrCreateListenerManager() { @@ -12242,23 +12191,6 @@ nsIDocument::ReportUseCounters(UseCounterReportKind aKind) } } } - - if (IsTopLevelContentDocument() && !IsResourceDoc()) { - using mozilla::Telemetry::LABELS_HIDDEN_VIEWPORT_OVERFLOW_TYPE; - LABELS_HIDDEN_VIEWPORT_OVERFLOW_TYPE label; - switch (mViewportOverflowType) { -#define CASE_OVERFLOW_TYPE(t_) \ - case ViewportOverflowType::t_: \ - label = LABELS_HIDDEN_VIEWPORT_OVERFLOW_TYPE::t_; \ - break; - CASE_OVERFLOW_TYPE(NoOverflow) - CASE_OVERFLOW_TYPE(Desktop) - CASE_OVERFLOW_TYPE(ButNotMinScaleSize) - CASE_OVERFLOW_TYPE(MinScaleSize) -#undef CASE_OVERFLOW_TYPE - } - Telemetry::AccumulateCategorical(label); - } } void diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h index d5209ed21383..ed42f7625b0f 100644 --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -1200,17 +1200,6 @@ public: */ nsViewportInfo GetViewportInfo(const mozilla::ScreenIntSize& aDisplaySize); - /** - * It updates the viewport overflow type with the given two widths - * and the viewport setting of the document. - * This should only be called when there is out-of-reach overflow - * happens on the viewport, i.e. the viewport should be using - * `overflow: hidden`. And it should only be called on a top level - * content document. - */ - void UpdateViewportOverflowType(nscoord aScrolledWidth, - nscoord aScrollportWidth); - /** * True iff this doc will ignore manual character encoding overrides. */ @@ -4288,7 +4277,7 @@ protected: // Our update nesting level uint32_t mUpdateNestLevel; - enum ViewportType : uint8_t { + enum ViewportType { DisplayWidthHeight, Specified, Unknown @@ -4296,30 +4285,6 @@ protected: ViewportType mViewportType; - // Enum for how content in this document overflows viewport causing - // out-of-reach issue. Currently it only takes horizontal overflow - // into consideration. This enum and the corresponding field is only - // set and read on a top level content document. - enum class ViewportOverflowType : uint8_t { - // Viewport doesn't have out-of-reach overflow content, either - // because the content doesn't overflow, or the viewport doesn't - // have "overflow: hidden". - NoOverflow, - - // All following items indicates that the content overflows the - // scroll port which causing out-of-reach content. - - // Meta viewport is disabled or the document is in desktop mode. - Desktop, - // The content does not overflow the minimum-scale size. When there - // is no minimum scale specified, the default value used by Blink, - // 0.25, is used for this matter. - ButNotMinScaleSize, - // The content overflows the minimum-scale size. - MinScaleSize, - }; - ViewportOverflowType mViewportOverflowType; - PLDHashTable* mSubDocuments; nsDocHeaderData* mHeaderData; diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index 1e43362bfc0f..f872d9762950 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -395,12 +395,12 @@ nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState, std::max(0, compositionSize.height - hScrollbarDesiredHeight)); } - nsRect scrolledRect = - mHelper.GetUnsnappedScrolledRectInternal(aState->mContentsOverflowAreas.ScrollableOverflow(), - scrollPortSize); - nscoord oneDevPixel = aState->mBoxState.PresContext()->DevPixelsToAppUnits(1); - if (!aForce) { + nsRect scrolledRect = + mHelper.GetUnsnappedScrolledRectInternal(aState->mContentsOverflowAreas.ScrollableOverflow(), + scrollPortSize); + nscoord oneDevPixel = aState->mBoxState.PresContext()->DevPixelsToAppUnits(1); + // If the style is HIDDEN then we already know that aAssumeHScroll is false if (aState->mStyles.mHorizontal != NS_STYLE_OVERFLOW_HIDDEN) { bool wantHScrollbar = @@ -426,17 +426,6 @@ nsHTMLScrollFrame::TryLayout(ScrollReflowInput* aState, } } - if (mHelper.mIsRoot && - aState->mStyles.mHorizontal == NS_STYLE_OVERFLOW_HIDDEN) { - nscoord scrolledWidth = scrolledRect.width + oneDevPixel; - if (scrolledWidth > scrollPortSize.width) { - nsIDocument* doc = PresContext()->Document(); - if (doc->IsTopLevelContentDocument()) { - doc->UpdateViewportOverflowType(scrolledWidth, scrollPortSize.width); - } - } - } - nscoord vScrollbarActualWidth = aState->mInsideBorderSize.width - scrollPortSize.width; aState->mShowHScrollbar = aAssumeHScroll; diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 026ed42acdbd..003e4507e2fe 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -13804,14 +13804,5 @@ "high": 50, "n_buckets": 20, "description": "Total number of doc groups per tab group, including docgroups fully in bfcache. Collected at the point when the top level document of the tab group is unloaded." - }, - "HIDDEN_VIEWPORT_OVERFLOW_TYPE": { - "record_in_processes": ["content"], - "alert_emails": ["xquan@mozilla.com", "botond@mozilla.com"], - "bug_numbers": [1423013, 1423017], - "expires_in_version": "65", - "kind": "categorical", - "labels": ["NoOverflow", "Desktop", "ButNotMinScaleSize", "MinScaleSize"], - "description": "How common are different types of out-of-reach viewport overflow?" } } From ae0a7bc371eca646a72479318ec61fe3c402b682 Mon Sep 17 00:00:00 2001 From: Bianca Danforth Date: Wed, 6 Jun 2018 03:56:32 -0700 Subject: [PATCH 18/51] Bug 1465698 - Add readermode event probes for Savant Shield study; r=jaws These probes will register and record (for the duration of the study only): * When the user turns on Reader Mode. * When the user turns off Reader Mode. Note: The events are recorded in the content process. MozReview-Commit-ID: BNEYW3TP1aN --HG-- extra : rebase_source : 2474bdf3e10e960ab5264804dbf693c5bd3b6ac1 --- toolkit/components/reader/ReaderMode.jsm | 5 +++++ toolkit/components/telemetry/Events.yaml | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/toolkit/components/reader/ReaderMode.jsm b/toolkit/components/reader/ReaderMode.jsm index 3bc05cdc7e7d..b8b42366ecfc 100644 --- a/toolkit/components/reader/ReaderMode.jsm +++ b/toolkit/components/reader/ReaderMode.jsm @@ -96,6 +96,9 @@ var ReaderMode = { * if not, append the about:reader page in the history instead. */ enterReaderMode(docShell, win) { + Services.telemetry.recordEvent("savant", "readermode", "on", null, + { subcategory: "feature" }); + let url = win.document.location.href; let readerURL = "about:reader?url=" + encodeURIComponent(url); let webNav = docShell.QueryInterface(Ci.nsIWebNavigation); @@ -117,6 +120,8 @@ var ReaderMode = { * if not, append the original page in the history instead. */ leaveReaderMode(docShell, win) { + Services.telemetry.recordEvent("savant", "readermode", "off", null, + { subcategory: "feature" }); let url = win.document.location.href; let originalURL = this.getOriginalUrl(url); let webNav = docShell.QueryInterface(Ci.nsIWebNavigation); diff --git a/toolkit/components/telemetry/Events.yaml b/toolkit/components/telemetry/Events.yaml index 7ed0477400f7..1e7d47d6982f 100644 --- a/toolkit/components/telemetry/Events.yaml +++ b/toolkit/components/telemetry/Events.yaml @@ -120,6 +120,19 @@ savant: expiry_version: "65" extra_keys: subcategory: The broad event category for this probe. E.g. navigation + readermode: + objects: ["on", "off"] + release_channel_collection: opt-out + record_in_processes: ["content"] + description: > + This is recorded any time Reader Mode is turned on or off. + bug_numbers: [1457226, 1465698] + notification_emails: + - "bdanforth@mozilla.com" + - "shong@mozilla.com" + expiry_version: "65" + extra_keys: + subcategory: The broad event category for this probe. E.g. navigation # This category contains event entries used for Telemetry tests. # They will not be sent out with any pings. From abeccb7b3b265f26ec89cf539008557f2c702d90 Mon Sep 17 00:00:00 2001 From: Petru Lingurar Date: Wed, 6 Jun 2018 18:34:08 +0300 Subject: [PATCH 19/51] Bug 1466884 - Add permission needed to publish on Samsung AppStore; r=nalexander MozReview-Commit-ID: 3ISGwdy0nQQ --HG-- extra : rebase_source : f8df0004a10d62fbe4bd6c50c886b3245f5c9384 --- mobile/android/base/FennecManifest_permissions.xml.in | 2 ++ mobile/android/base/SamsungAppStoreManifest_permissions.xml.in | 3 +++ 2 files changed, 5 insertions(+) create mode 100644 mobile/android/base/SamsungAppStoreManifest_permissions.xml.in diff --git a/mobile/android/base/FennecManifest_permissions.xml.in b/mobile/android/base/FennecManifest_permissions.xml.in index 94bd1b39e0e0..70e5cac67602 100644 --- a/mobile/android/base/FennecManifest_permissions.xml.in +++ b/mobile/android/base/FennecManifest_permissions.xml.in @@ -8,6 +8,8 @@ (potentially) of the push feature. --> #include GcmAndroidManifest_permissions.xml.in +#include SamsungAppStoreManifest_permissions.xml.in + diff --git a/mobile/android/base/SamsungAppStoreManifest_permissions.xml.in b/mobile/android/base/SamsungAppStoreManifest_permissions.xml.in new file mode 100644 index 000000000000..a66f9257ba2c --- /dev/null +++ b/mobile/android/base/SamsungAppStoreManifest_permissions.xml.in @@ -0,0 +1,3 @@ + + \ No newline at end of file From e7002517a5d9e220571451213fb8807f56322269 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 30 May 2018 23:26:59 +0200 Subject: [PATCH 20/51] Bug 1397061 - Adjust clock skew according to CDN cache age r=mgoodwin MozReview-Commit-ID: 9HPiNIp8bJM --HG-- extra : rebase_source : 1e19a75e1dd5aa7752d511fcf3256e460dbd9e55 --- services/settings/remote-settings.js | 5 +++- .../test/unit/test_remote_settings_poll.js | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/services/settings/remote-settings.js b/services/settings/remote-settings.js index 2ea405cb1433..130a53d3b6a4 100644 --- a/services/settings/remote-settings.js +++ b/services/settings/remote-settings.js @@ -164,7 +164,10 @@ async function fetchLatestChanges(url, lastEtag) { // The server should always return ETag. But we've had situations where the CDN // was interfering. const currentEtag = response.headers.has("ETag") ? response.headers.get("ETag") : undefined; - const serverTimeMillis = Date.parse(response.headers.get("Date")); + let serverTimeMillis = Date.parse(response.headers.get("Date")); + // Since the response is served via a CDN, the Date header value could have been cached. + const ageSeconds = response.headers.has("Age") ? parseInt(response.headers.get("Age"), 10) : 0; + serverTimeMillis += ageSeconds * 1000; // Check if the server asked the clients to back off. let backoffSeconds; diff --git a/services/settings/test/unit/test_remote_settings_poll.js b/services/settings/test/unit/test_remote_settings_poll.js index 604d997698bc..e46fff695373 100644 --- a/services/settings/test/unit/test_remote_settings_poll.js +++ b/services/settings/test/unit/test_remote_settings_poll.js @@ -248,6 +248,29 @@ add_task(async function test_check_clockskew_is_updated() { add_task(clear_state); +add_task(async function test_check_clockskew_takes_age_into_account() { + const currentTime = Date.now(); + const skewSeconds = 5; + const ageCDNSeconds = 3600; + const serverTime = currentTime - (skewSeconds * 1000) - (ageCDNSeconds * 1000); + + function serverResponse(request, response) { + response.setHeader("Content-Type", "application/json; charset=UTF-8"); + response.setHeader("Date", (new Date(serverTime)).toUTCString()); + response.setHeader("Age", `${ageCDNSeconds}`); + response.write(JSON.stringify({data: []})); + response.setStatusLine(null, 200, "OK"); + } + server.registerPathHandler(CHANGES_PATH, serverResponse); + + await RemoteSettings.pollChanges(); + + const clockSkew = Services.prefs.getIntPref(PREF_CLOCK_SKEW_SECONDS); + Assert.ok(clockSkew >= skewSeconds, `clockSkew is ${clockSkew}`); +}); +add_task(clear_state); + + add_task(async function test_backoff() { const startHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY); From aac6fb947024b5162c5c375d835566c6a66c3923 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Thu, 7 Jun 2018 14:12:37 +0200 Subject: [PATCH 21/51] Bug 1460609 - Cookies are for parents r=mayhemer Make sure cookies aren't saved on channel headers in the content process. Adds test to verify that this works, and removes tests that expected cookie headers to be visible to the child. MozReview-Commit-ID: KOB83xpuAlF --HG-- extra : rebase_source : 6f9a5ef570fb23200acf8d75285e67d80b7c27f0 --- netwerk/protocol/http/HttpChannelChild.cpp | 7 ++ netwerk/protocol/http/HttpChannelParent.cpp | 46 +++++++++-- netwerk/test/unit_ipc/child_cookie_header.js | 77 +++++++++++++++++++ .../unit_ipc/test_bug248970_cookie_wrap.js | 7 -- .../unit_ipc/test_cookie_header_stripped.js | 68 ++++++++++++++++ .../test/unit_ipc/test_cookie_header_wrap.js | 12 --- netwerk/test/unit_ipc/xpcshell.ini | 6 +- 7 files changed, 193 insertions(+), 30 deletions(-) create mode 100644 netwerk/test/unit_ipc/child_cookie_header.js delete mode 100644 netwerk/test/unit_ipc/test_bug248970_cookie_wrap.js create mode 100644 netwerk/test/unit_ipc/test_cookie_header_stripped.js delete mode 100644 netwerk/test/unit_ipc/test_cookie_header_wrap.js diff --git a/netwerk/protocol/http/HttpChannelChild.cpp b/netwerk/protocol/http/HttpChannelChild.cpp index 684995d1f994..add425d10742 100644 --- a/netwerk/protocol/http/HttpChannelChild.cpp +++ b/netwerk/protocol/http/HttpChannelChild.cpp @@ -582,6 +582,10 @@ HttpChannelChild::OnStartRequest(const nsresult& channelStatus, mStatus = channelStatus; } + // Cookies headers should not be visible to the child process + MOZ_ASSERT(!requestHeaders.HasHeader(nsHttp::Cookie)); + MOZ_ASSERT(!nsHttpResponseHead(responseHead).HasHeader(nsHttp::Set_Cookie)); + if (useResponseHead && !mCanceled) mResponseHead = new nsHttpResponseHead(responseHead); @@ -1666,6 +1670,9 @@ HttpChannelChild::RecvRedirect1Begin(const uint32_t& registrarId, // Then it will be updated to new peer in OnStartRequest mPeerAddr = oldPeerAddr; + // Cookies headers should not be visible to the child process + MOZ_ASSERT(!nsHttpResponseHead(responseHead).HasHeader(nsHttp::Set_Cookie)); + mEventQ->RunOrEnqueue(new Redirect1Event(this, registrarId, newUri, redirectFlags, loadInfoForwarder, responseHead, securityInfoSerialization, diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index 368813e7080c..619b2bc29377 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -1383,8 +1383,6 @@ HttpChannelParent::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext) MOZ_ASSERT(NS_SUCCEEDED(rv)); } - nsHttpResponseHead *responseHead = chan->GetResponseHead(); - nsHttpRequestHead *requestHead = chan->GetRequestHead(); bool isFromCache = false; uint64_t cacheEntryId = 0; int32_t fetchCount = 0; @@ -1460,14 +1458,37 @@ HttpChannelParent::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext) ParentLoadInfoForwarderArgs loadInfoForwarderArg; mozilla::ipc::LoadInfoToParentLoadInfoForwarder(loadInfo, &loadInfoForwarderArg); + nsHttpResponseHead *responseHead = chan->GetResponseHead(); + bool useResponseHead = !!responseHead; + nsHttpResponseHead cleanedUpResponseHead; + if (responseHead && responseHead->HasHeader(nsHttp::Set_Cookie)) { + cleanedUpResponseHead = *responseHead; + cleanedUpResponseHead.ClearHeader(nsHttp::Set_Cookie); + responseHead = &cleanedUpResponseHead; + } + + if (!responseHead) { + responseHead = &cleanedUpResponseHead; + } + + nsHttpRequestHead *requestHead = chan->GetRequestHead(); // !!! We need to lock headers and please don't forget to unlock them !!! requestHead->Enter(); + + nsHttpHeaderArray cleanedUpRequestHeaders; + bool cleanedUpRequest = false; + if (requestHead->HasHeader(nsHttp::Cookie)) { + cleanedUpRequestHeaders = requestHead->Headers(); + cleanedUpRequestHeaders.ClearHeader(nsHttp::Cookie); + cleanedUpRequest = true; + } + rv = NS_OK; if (mIPCClosed || !SendOnStartRequest(channelStatus, - responseHead ? *responseHead : nsHttpResponseHead(), - !!responseHead, - requestHead->Headers(), + *responseHead, + useResponseHead, + cleanedUpRequest ? cleanedUpRequestHeaders : requestHead->Headers(), loadInfoForwarderArg, isFromCache, mCacheEntry ? true : false, @@ -1840,12 +1861,23 @@ HttpChannelParent::StartRedirect(uint32_t registrarId, mozilla::ipc::LoadInfoToParentLoadInfoForwarder(loadInfo, &loadInfoForwarderArg); nsHttpResponseHead *responseHead = mChannel->GetResponseHead(); + + nsHttpResponseHead cleanedUpResponseHead; + if (responseHead && responseHead->HasHeader(nsHttp::Set_Cookie)) { + cleanedUpResponseHead = *responseHead; + cleanedUpResponseHead.ClearHeader(nsHttp::Set_Cookie); + responseHead = &cleanedUpResponseHead; + } + + if (!responseHead) { + responseHead = &cleanedUpResponseHead; + } + bool result = false; if (!mIPCClosed) { result = SendRedirect1Begin(registrarId, uriParams, redirectFlags, loadInfoForwarderArg, - responseHead ? *responseHead - : nsHttpResponseHead(), + *responseHead, secInfoSerialization, channelId, mChannel->GetPeerAddr()); diff --git a/netwerk/test/unit_ipc/child_cookie_header.js b/netwerk/test/unit_ipc/child_cookie_header.js new file mode 100644 index 000000000000..2cbbe8d8b6e1 --- /dev/null +++ b/netwerk/test/unit_ipc/child_cookie_header.js @@ -0,0 +1,77 @@ +"use strict"; + +ChromeUtils.import("resource://testing-common/httpd.js"); +ChromeUtils.import("resource://gre/modules/Services.jsm"); +ChromeUtils.import("resource://gre/modules/NetUtil.jsm"); + + +function inChildProcess() { + return Cc["@mozilla.org/xre/app-info;1"] + .getService(Ci.nsIXULRuntime) + .processType != Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT; +} + +let URL = null; +function makeChan() { + return NetUtil.newChannel({uri: URL, loadUsingSystemPrincipal: true}) + .QueryInterface(Ci.nsIHttpChannel); +} + +function OpenChannelPromise(aChannel, aClosure) { + return new Promise(resolve => { + function processResponse(request, buffer, context) + { + aClosure(request.QueryInterface(Ci.nsIHttpChannel), buffer, context); + resolve(); + } + aChannel.asyncOpen2(new ChannelListener(processResponse, null)); + }); +} + +// This test doesn't do much, except to communicate with the parent, and get +// URL we need to connect to. +add_task(async function setup() { + ok(inChildProcess(), "Sanity check. This should run in the child process"); + // Initialize the URL. Parent runs the server + do_send_remote_message("start-test"); + URL = await do_await_remote_message("start-test-done"); +}); + +// This test performs a request, and checks that no cookie header are visible +// to the child process +add_task(async function test1() { + let chan = makeChan(); + + await OpenChannelPromise(chan, (request, buffer) => { + equal(buffer, "response"); + Assert.throws(() => request.getRequestHeader("Cookie"), /NS_ERROR_NOT_AVAILABLE/, "Cookie header should not be visible on request in the child"); + Assert.throws(() => request.getResponseHeader("Set-Cookie"), /NS_ERROR_NOT_AVAILABLE/, "Cookie header should not be visible on response in the child"); + }); + + // We also check that a cookie was saved by the Set-Cookie header + // in the parent. + do_send_remote_message("check-cookie-count"); + let count = await do_await_remote_message("check-cookie-count-done"); + equal(count, 1); +}); + +// This test communicates with the parent, to locally save a new cookie. +// Then it performs another request, makes sure no cookie headers are visible, +// after which it checks that both cookies are visible to the parent. +add_task(async function test2() { + do_send_remote_message("set-cookie"); + await do_await_remote_message('set-cookie-done'); + + let chan = makeChan(); + await OpenChannelPromise(chan, (request, buffer) => { + equal(buffer, "response"); + Assert.throws(() => request.getRequestHeader("Cookie"), /NS_ERROR_NOT_AVAILABLE/, "Cookie header should not be visible on request in the child"); + Assert.throws(() => request.getResponseHeader("Set-Cookie"), /NS_ERROR_NOT_AVAILABLE/, "Cookie header should not be visible on response in the child"); + }); + + // We should have two cookies. One set by the Set-Cookie header sent by the + // server, and one that was manually set in the parent. + do_send_remote_message("second-check-cookie-count"); + let count = await do_await_remote_message("second-check-cookie-count-done"); + equal(count, 2); +}); diff --git a/netwerk/test/unit_ipc/test_bug248970_cookie_wrap.js b/netwerk/test/unit_ipc/test_bug248970_cookie_wrap.js deleted file mode 100644 index 9e64fbb664a0..000000000000 --- a/netwerk/test/unit_ipc/test_bug248970_cookie_wrap.js +++ /dev/null @@ -1,7 +0,0 @@ -ChromeUtils.import("resource://gre/modules/Services.jsm"); - -function run_test() { - // Allow all cookies. - Services.prefs.setIntPref("network.cookie.cookieBehavior", 0); - run_test_in_child("../unit/test_bug248970_cookie.js"); -} \ No newline at end of file diff --git a/netwerk/test/unit_ipc/test_cookie_header_stripped.js b/netwerk/test/unit_ipc/test_cookie_header_stripped.js new file mode 100644 index 000000000000..885df1b4949a --- /dev/null +++ b/netwerk/test/unit_ipc/test_cookie_header_stripped.js @@ -0,0 +1,68 @@ +"use strict"; + +ChromeUtils.import("resource://testing-common/httpd.js"); +ChromeUtils.import("resource://gre/modules/Services.jsm"); + + +const TEST_DOMAIN = "www.example.com"; +XPCOMUtils.defineLazyGetter(this, "URL", function() { + return "http://" + TEST_DOMAIN +":" + httpserv.identity.primaryPort + "/path"; +}); + +const responseBody1 = "response"; +function requestHandler(metadata, response) +{ + response.setHeader("Content-Type", "text/plain"); + response.setHeader("Set-Cookie", "tom=cool; Max-Age=10", true); + response.bodyOutputStream.write(responseBody1, responseBody1.length); +} + +let httpserv = null; + +function run_test() { + httpserv = new HttpServer(); + httpserv.registerPathHandler("/path", requestHandler); + httpserv.start(-1); + httpserv.identity.add("http", TEST_DOMAIN, httpserv.identity.primaryPort); + + registerCleanupFunction(() => { + Services.cookies.removeCookiesWithOriginAttributes("{}", TEST_DOMAIN); + Services.prefs.clearUserPref("network.dns.localDomains"); + Services.prefs.clearUserPref("network.cookie.cookieBehavior"); + + httpserv.stop(); + httpserv = null; + }); + + Services.prefs.setCharPref("network.dns.localDomains", TEST_DOMAIN); + Services.prefs.setIntPref("network.cookie.cookieBehavior", 0); + Services.cookies.removeCookiesWithOriginAttributes("{}", TEST_DOMAIN); + + // Sends back the URL to the child script + do_await_remote_message("start-test").then(() => { + do_send_remote_message("start-test-done", URL); + }); + + // Sends back the cookie count for the domain + // Should only be one - from Set-Cookie + do_await_remote_message("check-cookie-count").then(() => { + do_send_remote_message("check-cookie-count-done", Services.cookies.countCookiesFromHost(TEST_DOMAIN)); + }); + + // Sends back the cookie count for the domain + // There should be 2 cookies. One from the Set-Cookie header, the other set + // manually. + do_await_remote_message("second-check-cookie-count").then(() => { + do_send_remote_message("second-check-cookie-count-done", Services.cookies.countCookiesFromHost(TEST_DOMAIN)); + }); + + // Sets a cookie for the test domain + do_await_remote_message("set-cookie").then(() => { + const expiry = Date.now() + 24 * 60 * 60; + Services.cookies.add(TEST_DOMAIN, "/", "cookieName", "cookieValue", false, false, false, expiry, {}); + do_send_remote_message("set-cookie-done"); + }); + + // Run the actual test logic + run_test_in_child("child_cookie_header.js"); +} diff --git a/netwerk/test/unit_ipc/test_cookie_header_wrap.js b/netwerk/test/unit_ipc/test_cookie_header_wrap.js deleted file mode 100644 index 565ab1878254..000000000000 --- a/netwerk/test/unit_ipc/test_cookie_header_wrap.js +++ /dev/null @@ -1,12 +0,0 @@ -// -// Run test script in content process instead of chrome (xpcshell's default) -// - -ChromeUtils.import("resource://gre/modules/Services.jsm"); - -function run_test() { - // Allow all cookies. - Services.prefs.setIntPref("network.cookie.cookieBehavior", 0); - Services.prefs.setBoolPref("network.cookie.ipc.sync", true); - run_test_in_child("../unit/test_cookie_header.js"); -} diff --git a/netwerk/test/unit_ipc/xpcshell.ini b/netwerk/test/unit_ipc/xpcshell.ini index 5b785e73a98c..9c908f69c002 100644 --- a/netwerk/test/unit_ipc/xpcshell.ini +++ b/netwerk/test/unit_ipc/xpcshell.ini @@ -4,13 +4,11 @@ skip-if = toolkit == 'android' support-files = child_channel_id.js !/netwerk/test/unit/test_XHR_redirects.js - !/netwerk/test/unit/test_bug248970_cookie.js !/netwerk/test/unit/test_bug528292.js !/netwerk/test/unit/test_cache-entry-id.js !/netwerk/test/unit/test_cache_jar.js !/netwerk/test/unit/test_cacheflags.js !/netwerk/test/unit/test_channel_close.js - !/netwerk/test/unit/test_cookie_header.js !/netwerk/test/unit/test_cookiejars.js !/netwerk/test/unit/test_dns_cancel.js !/netwerk/test/unit/test_dns_service.js @@ -59,14 +57,14 @@ support-files = !/netwerk/test/unit/test_multipart_streamconv.js !/netwerk/test/unit/test_original_sent_received_head.js !/netwerk/test/unit/test_alt-data_cross_process.js + child_cookie_header.js [test_bug528292_wrap.js] -[test_bug248970_cookie_wrap.js] +[test_cookie_header_stripped.js] [test_cacheflags_wrap.js] [test_cache-entry-id_wrap.js] [test_cache_jar_wrap.js] [test_channel_close_wrap.js] -[test_cookie_header_wrap.js] [test_cookiejars_wrap.js] [test_dns_cancel_wrap.js] [test_dns_service_wrap.js] From 87bf13e093ed29de1665df35331c6c792b80b47d Mon Sep 17 00:00:00 2001 From: Doug Thayer Date: Sun, 29 Apr 2018 18:21:20 -0700 Subject: [PATCH 22/51] Bug 1448040 - Remove HangMonitor/ChromeHangs r=Nika Fairly straightforward, just a blanket removal. Haven't heard anything on dev-platform or fx-data-dev regarding this removal, so I think it's likely safe to remove on Nightly, and we can revert if anyone makes a fuss. As part of removing the HangMonitor, I renamed a few things and reorganized the namespaces to not depend on a HangMonitor namespace. Hopefully this doesn't produce too much noise in the diff, it just seemed appropriate to move everything around rather than keep dangling vestiges of the old system. MozReview-Commit-ID: 8C8NFnOP5GU --HG-- extra : rebase_source : 59e4a6ced7d14d2a01c0b79e944078ea84cae523 --- dom/base/nsContentUtils.cpp | 19 +- dom/ipc/ContentChild.cpp | 11 +- dom/plugins/ipc/PluginModuleParent.cpp | 11 +- dom/plugins/ipc/PluginModuleParent.h | 4 +- ipc/mscom/MainThreadInvoker.cpp | 4 +- js/src/tests/user.js | 1 - modules/libpref/init/all.js | 6 - .../client/marionette_driver/geckoinstance.py | 3 - testing/marionette/components/marionette.js | 3 - .../BackgroundHangMonitor.cpp | 16 +- .../BackgroundHangMonitor.h | 4 +- .../backgroundhangmonitor/HangAnnotations.cpp | 102 +++++ .../backgroundhangmonitor/HangAnnotations.h | 75 ++++ .../backgroundhangmonitor/moz.build | 2 + .../components/telemetry/CombinedStacks.cpp | 1 - toolkit/components/telemetry/HangReports.cpp | 148 ------ toolkit/components/telemetry/HangReports.h | 90 ---- toolkit/components/telemetry/Telemetry.cpp | 165 ------- toolkit/components/telemetry/Telemetry.h | 9 - .../components/telemetry/TelemetrySession.jsm | 1 - .../telemetry/docs/data/main-ping.rst | 5 +- toolkit/components/telemetry/moz.build | 1 - toolkit/components/telemetry/nsITelemetry.idl | 8 - .../tests/unit/test_TelemetrySession.js | 2 +- toolkit/content/aboutTelemetry.js | 54 --- toolkit/content/aboutTelemetry.xhtml | 9 - toolkit/crashreporter/docs/index.rst | 8 - .../en-US/chrome/global/aboutTelemetry.dtd | 1 - .../chrome/global/aboutTelemetry.properties | 4 - widget/android/GeckoEditableSupport.h | 6 +- widget/android/nsAppShell.cpp | 4 +- widget/android/nsAppShell.h | 11 +- widget/android/nsWindow.cpp | 4 +- widget/cocoa/nsAppShell.mm | 13 +- widget/gtk/nsAppShell.cpp | 6 +- widget/windows/WinUtils.cpp | 4 +- widget/windows/nsAppShell.cpp | 8 +- widget/windows/nsWindow.cpp | 18 +- xpcom/build/XPCOMInit.cpp | 8 +- xpcom/threads/CPUUsageWatcher.cpp | 8 +- xpcom/threads/CPUUsageWatcher.h | 7 +- xpcom/threads/HangAnnotations.cpp | 175 -------- xpcom/threads/HangAnnotations.h | 127 ------ xpcom/threads/HangMonitor.cpp | 424 ------------------ xpcom/threads/HangMonitor.h | 58 --- xpcom/threads/moz.build | 4 - xpcom/threads/nsThread.cpp | 6 +- 47 files changed, 258 insertions(+), 1400 deletions(-) create mode 100644 toolkit/components/backgroundhangmonitor/HangAnnotations.cpp create mode 100644 toolkit/components/backgroundhangmonitor/HangAnnotations.h delete mode 100644 toolkit/components/telemetry/HangReports.cpp delete mode 100644 toolkit/components/telemetry/HangReports.h delete mode 100644 xpcom/threads/HangAnnotations.cpp delete mode 100644 xpcom/threads/HangAnnotations.h delete mode 100644 xpcom/threads/HangMonitor.cpp delete mode 100644 xpcom/threads/HangMonitor.h diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index ad38558aa0e5..394ffffe13ce 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -33,6 +33,7 @@ #include "mozilla/Attributes.h" #include "mozilla/AutoRestore.h" #include "mozilla/AutoTimelineMarker.h" +#include "mozilla/BackgroundHangMonitor.h" #include "mozilla/Base64.h" #include "mozilla/CheckedInt.h" #include "mozilla/DebugOnly.h" @@ -563,7 +564,7 @@ NS_IMPL_ISUPPORTS(nsContentUtils::nsContentUtilsReporter, nsIMemoryReporter) * user interaction status. */ class nsContentUtils::UserInteractionObserver final : public nsIObserver - , public HangMonitor::Annotator + , public BackgroundHangAnnotator { public: NS_DECL_ISUPPORTS @@ -571,7 +572,7 @@ public: void Init(); void Shutdown(); - void AnnotateHang(HangMonitor::HangAnnotations& aAnnotations) override; + void AnnotateHang(BackgroundHangAnnotations& aAnnotations) override; static Atomic sUserActive; @@ -10741,13 +10742,13 @@ nsContentUtils::UserInteractionObserver::Init() obs->AddObserver(this, kUserInteractionInactive, false); obs->AddObserver(this, kUserInteractionActive, false); - // We can't register ourselves as an annotator yet, as the HangMonitor hasn't - // started yet. It will have started by the time we have the chance to spin - // the event loop. + // We can't register ourselves as an annotator yet, as the + // BackgroundHangMonitor hasn't started yet. It will have started by the + // time we have the chance to spin the event loop. RefPtr self = this; NS_DispatchToMainThread( NS_NewRunnableFunction("nsContentUtils::UserInteractionObserver::Init", - [=]() { HangMonitor::RegisterAnnotator(*self); })); + [=]() { BackgroundHangMonitor::RegisterAnnotator(*self); })); } void @@ -10759,15 +10760,15 @@ nsContentUtils::UserInteractionObserver::Shutdown() obs->RemoveObserver(this, kUserInteractionActive); } - HangMonitor::UnregisterAnnotator(*this); + BackgroundHangMonitor::UnregisterAnnotator(*this); } /** - * NB: This function is always called by the HangMonitor thread. + * NB: This function is always called by the BackgroundHangMonitor thread. * Plan accordingly */ void -nsContentUtils::UserInteractionObserver::AnnotateHang(HangMonitor::HangAnnotations& aAnnotations) +nsContentUtils::UserInteractionObserver::AnnotateHang(BackgroundHangAnnotations& aAnnotations) { // NOTE: Only annotate the hang report if the user is known to be interacting. if (sUserActive) { diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 8d1ef9d6dd70..30365bda4eba 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -15,6 +15,7 @@ #include "HandlerServiceChild.h" #include "mozilla/Attributes.h" +#include "mozilla/BackgroundHangMonitor.h" #include "mozilla/LookAndFeel.h" #include "mozilla/Preferences.h" #include "mozilla/ProcessHangMonitorIPC.h" @@ -509,15 +510,15 @@ ConsoleListener::Observe(nsIConsoleMessage* aMessage) #ifdef NIGHTLY_BUILD /** - * The singleton of this class is registered with the HangMonitor as an + * The singleton of this class is registered with the BackgroundHangMonitor as an * annotator, so that the hang monitor can record whether or not there were * pending input events when the thread hung. */ class PendingInputEventHangAnnotator final - : public HangMonitor::Annotator + : public BackgroundHangAnnotator { public: - virtual void AnnotateHang(HangMonitor::HangAnnotations& aAnnotations) override + virtual void AnnotateHang(BackgroundHangAnnotations& aAnnotations) override { int32_t pending = ContentChild::GetSingleton()->GetPendingInputEvents(); if (pending > 0) { @@ -711,7 +712,7 @@ ContentChild::Init(MessageLoop* aIOLoop, // only affect a single thread. SystemGroup::Dispatch(TaskCategory::Other, NS_NewRunnableFunction("RegisterPendingInputEventHangAnnotator", [] { - HangMonitor::RegisterAnnotator( + BackgroundHangMonitor::RegisterAnnotator( PendingInputEventHangAnnotator::sSingleton); })); #endif @@ -3041,7 +3042,7 @@ ContentChild::ShutdownInternal() mShuttingDown = true; #ifdef NIGHTLY_BUILD - HangMonitor::UnregisterAnnotator(PendingInputEventHangAnnotator::sSingleton); + BackgroundHangMonitor::UnregisterAnnotator(PendingInputEventHangAnnotator::sSingleton); #endif if (mPolicy) { diff --git a/dom/plugins/ipc/PluginModuleParent.cpp b/dom/plugins/ipc/PluginModuleParent.cpp index 6355ab6cc48a..1fa3045c36b2 100644 --- a/dom/plugins/ipc/PluginModuleParent.cpp +++ b/dom/plugins/ipc/PluginModuleParent.cpp @@ -9,6 +9,7 @@ #include "base/process_util.h" #include "mozilla/Attributes.h" #include "mozilla/AutoRestore.h" +#include "mozilla/BackgroundHangMonitor.h" #include "mozilla/dom/ContentParent.h" #include "mozilla/dom/ContentChild.h" #include "mozilla/ipc/CrashReporterClient.h" @@ -656,7 +657,7 @@ PluginModuleChromeParent::PluginModuleChromeParent(const char* aFilePath, mSandboxLevel = aSandboxLevel; mRunID = GeckoChildProcessHost::GetUniqueID(); - mozilla::HangMonitor::RegisterAnnotator(*this); + mozilla::BackgroundHangMonitor::RegisterAnnotator(*this); } PluginModuleChromeParent::~PluginModuleChromeParent() @@ -709,7 +710,7 @@ PluginModuleChromeParent::~PluginModuleChromeParent() } #endif - mozilla::HangMonitor::UnregisterAnnotator(*this); + mozilla::BackgroundHangMonitor::UnregisterAnnotator(*this); } void @@ -988,16 +989,16 @@ PluginModuleChromeParent::ExitedCxxStack() } /** - * This function is always called by the HangMonitor thread. + * This function is always called by the BackgroundHangMonitor thread. */ void -PluginModuleChromeParent::AnnotateHang(mozilla::HangMonitor::HangAnnotations& aAnnotations) +PluginModuleChromeParent::AnnotateHang(mozilla::BackgroundHangAnnotations& aAnnotations) { uint32_t flags = mHangAnnotationFlags; if (flags) { /* We don't actually annotate anything specifically for kInPluginCall; we use it to determine whether to annotate other things. It will - be pretty obvious from the ChromeHang stack that we're in a plugin + be pretty obvious from the hang stack that we're in a plugin call when the hang occurred. */ if (flags & kHangUIShown) { aAnnotations.AddAnnotation(NS_LITERAL_STRING("HangUIShown"), diff --git a/dom/plugins/ipc/PluginModuleParent.h b/dom/plugins/ipc/PluginModuleParent.h index ec3c937e20c3..68308324a502 100644 --- a/dom/plugins/ipc/PluginModuleParent.h +++ b/dom/plugins/ipc/PluginModuleParent.h @@ -353,7 +353,7 @@ class PluginModuleContentParent : public PluginModuleParent class PluginModuleChromeParent : public PluginModuleParent - , public mozilla::HangMonitor::Annotator + , public mozilla::BackgroundHangAnnotator { friend class mozilla::ipc::CrashReporterHost; using TerminateChildProcessCallback = @@ -477,7 +477,7 @@ private: PluginInstanceParent* GetManagingInstance(mozilla::ipc::IProtocol* aProtocol); virtual void - AnnotateHang(mozilla::HangMonitor::HangAnnotations& aAnnotations) override; + AnnotateHang(mozilla::BackgroundHangAnnotations& aAnnotations) override; virtual bool ShouldContinueFromReplyTimeout() override; diff --git a/ipc/mscom/MainThreadInvoker.cpp b/ipc/mscom/MainThreadInvoker.cpp index fe7c9e07cf1f..ef5ded45716e 100644 --- a/ipc/mscom/MainThreadInvoker.cpp +++ b/ipc/mscom/MainThreadInvoker.cpp @@ -9,9 +9,9 @@ #include "GeckoProfiler.h" #include "MainThreadUtils.h" #include "mozilla/Assertions.h" +#include "mozilla/BackgroundHangMonitor.h" #include "mozilla/ClearOnShutdown.h" #include "mozilla/DebugOnly.h" -#include "mozilla/HangMonitor.h" #include "mozilla/mscom/SpinEvent.h" #include "mozilla/RefPtr.h" #include "mozilla/SystemGroup.h" @@ -181,7 +181,7 @@ MainThreadInvoker::Invoke(already_AddRefed&& aRunnable) MainThreadInvoker::MainThreadAPC(ULONG_PTR aParam) { AUTO_PROFILER_THREAD_WAKE; - mozilla::HangMonitor::NotifyActivity(mozilla::HangMonitor::kGeneralActivity); + mozilla::BackgroundHangMonitor().NotifyActivity(); MOZ_ASSERT(NS_IsMainThread()); auto runnable = reinterpret_cast(aParam); runnable->APCRun(); diff --git a/js/src/tests/user.js b/js/src/tests/user.js index 928cb740da1f..c50f01d74457 100755 --- a/js/src/tests/user.js +++ b/js/src/tests/user.js @@ -7,7 +7,6 @@ user_pref("security.fileuri.strict_origin_policy", false); user_pref("dom.allow_scripts_to_close_windows", true); user_pref("dom.disable_open_during_load", false); user_pref("dom.max_script_run_time", 0); -user_pref("hangmonitor.timeout", 0); user_pref("dom.max_chrome_script_run_time", 0); user_pref("javascript.allow.mailnews", true); user_pref("javascript.options.showInConsole", true); diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 9a08f688894e..6bac95ddab68 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -3110,12 +3110,6 @@ pref("input_event_queue.default_duration_per_event", 1); // required to process the following input events. pref("input_event_queue.count_for_prediction", 9); -// Hang monitor timeout after which we kill the browser, in seconds -// (0 is disabled) -// Disabled on all platforms per bug 705748 until the found issues are -// resolved. -pref("hangmonitor.timeout", 0); - pref("plugins.load_appdir_plugins", false); // If true, plugins will be click to play pref("plugins.click_to_play", false); diff --git a/testing/marionette/client/marionette_driver/geckoinstance.py b/testing/marionette/client/marionette_driver/geckoinstance.py index a72c39f81db5..88a7f4169bc5 100644 --- a/testing/marionette/client/marionette_driver/geckoinstance.py +++ b/testing/marionette/client/marionette_driver/geckoinstance.py @@ -90,9 +90,6 @@ class GeckoInstance(object): # Do not scan Wifi "geo.wifi.scan": False, - # No hang monitor - "hangmonitor.timeout": 0, - "javascript.options.showInConsole": True, # Enable Marionette component diff --git a/testing/marionette/components/marionette.js b/testing/marionette/components/marionette.js index cf6e2dd3431a..d37dbd17bb62 100644 --- a/testing/marionette/components/marionette.js +++ b/testing/marionette/components/marionette.js @@ -216,9 +216,6 @@ const RECOMMENDED_PREFS = new Map([ // Do not scan Wifi ["geo.wifi.scan", false], - // No hang monitor - ["hangmonitor.timeout", 0], - // Show chrome errors and warnings in the error console ["javascript.options.showInConsole", true], diff --git a/toolkit/components/backgroundhangmonitor/BackgroundHangMonitor.cpp b/toolkit/components/backgroundhangmonitor/BackgroundHangMonitor.cpp index c8b7d1d1ad27..33b1d0363927 100644 --- a/toolkit/components/backgroundhangmonitor/BackgroundHangMonitor.cpp +++ b/toolkit/components/backgroundhangmonitor/BackgroundHangMonitor.cpp @@ -202,9 +202,9 @@ public: // Stack of current hang HangStack mHangStack; // Annotations for the current hang - HangMonitor::HangAnnotations mAnnotations; + BackgroundHangAnnotations mAnnotations; // Annotators registered for this thread - HangMonitor::Observer::Annotators mAnnotators; + BackgroundHangAnnotators mAnnotators; // The name of the runnable which is hanging the current process nsCString mRunnableName; // The name of the thread which is being monitored @@ -488,12 +488,6 @@ BackgroundHangThread::ReportHang(TimeDuration aHangTime) // Recovered from a hang; called on the monitor thread // mManager->mLock IS locked - nsTArray annotations; - for (auto& annotation : mAnnotations) { - HangAnnotation annot(annotation.mName, annotation.mValue); - annotations.AppendElement(std::move(annot)); - } - HangDetails hangDetails( aHangTime, nsDependentCString(XRE_ChildProcessTypeToString(XRE_GetProcessType())), @@ -501,7 +495,7 @@ BackgroundHangThread::ReportHang(TimeDuration aHangTime) mThreadName, mRunnableName, std::move(mHangStack), - std::move(annotations) + std::move(mAnnotations) ); // If the hang processing thread exists, we can process the native stack @@ -767,7 +761,7 @@ BackgroundHangMonitor::NotifyWait() } bool -BackgroundHangMonitor::RegisterAnnotator(HangMonitor::Annotator& aAnnotator) +BackgroundHangMonitor::RegisterAnnotator(BackgroundHangAnnotator& aAnnotator) { #ifdef MOZ_ENABLE_BACKGROUND_HANG_MONITOR BackgroundHangThread* thisThread = BackgroundHangThread::FindThread(); @@ -781,7 +775,7 @@ BackgroundHangMonitor::RegisterAnnotator(HangMonitor::Annotator& aAnnotator) } bool -BackgroundHangMonitor::UnregisterAnnotator(HangMonitor::Annotator& aAnnotator) +BackgroundHangMonitor::UnregisterAnnotator(BackgroundHangAnnotator& aAnnotator) { #ifdef MOZ_ENABLE_BACKGROUND_HANG_MONITOR BackgroundHangThread* thisThread = BackgroundHangThread::FindThread(); diff --git a/toolkit/components/backgroundhangmonitor/BackgroundHangMonitor.h b/toolkit/components/backgroundhangmonitor/BackgroundHangMonitor.h index 10942a3e65e3..258003163ee8 100644 --- a/toolkit/components/backgroundhangmonitor/BackgroundHangMonitor.h +++ b/toolkit/components/backgroundhangmonitor/BackgroundHangMonitor.h @@ -189,7 +189,7 @@ public: * @param aAnnotator annotator to register * @return true if the annotator was registered, otherwise false. */ - static bool RegisterAnnotator(HangMonitor::Annotator& aAnnotator); + static bool RegisterAnnotator(BackgroundHangAnnotator& aAnnotator); /** * Unregister an annotator that was previously registered via @@ -197,7 +197,7 @@ public: * @param aAnnotator annotator to unregister * @return true if there are still remaining annotators registered */ - static bool UnregisterAnnotator(HangMonitor::Annotator& aAnnotator); + static bool UnregisterAnnotator(BackgroundHangAnnotator& aAnnotator); }; } // namespace mozilla diff --git a/toolkit/components/backgroundhangmonitor/HangAnnotations.cpp b/toolkit/components/backgroundhangmonitor/HangAnnotations.cpp new file mode 100644 index 000000000000..c80eeaedac9c --- /dev/null +++ b/toolkit/components/backgroundhangmonitor/HangAnnotations.cpp @@ -0,0 +1,102 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=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/. */ + +#include "mozilla/HangAnnotations.h" + +#include + +#include "MainThreadUtils.h" +#include "mozilla/DebugOnly.h" +#include "nsXULAppAPI.h" +#include "mozilla/BackgroundHangMonitor.h" + +namespace mozilla { + +void +BackgroundHangAnnotations::AddAnnotation(const nsString& aName, const int32_t aData) +{ + nsAutoString dataString; + dataString.AppendInt(aData); + AppendElement(HangAnnotation(aName, dataString)); +} + +void +BackgroundHangAnnotations::AddAnnotation(const nsString& aName, const double aData) +{ + nsAutoString dataString; + dataString.AppendFloat(aData); + AppendElement(HangAnnotation(aName, dataString)); +} + +void +BackgroundHangAnnotations::AddAnnotation(const nsString& aName, const nsString& aData) +{ + AppendElement(HangAnnotation(aName, aData)); +} + +void +BackgroundHangAnnotations::AddAnnotation(const nsString& aName, const nsCString& aData) +{ + NS_ConvertUTF8toUTF16 dataString(aData); + AppendElement(HangAnnotation(aName, dataString)); +} + +void +BackgroundHangAnnotations::AddAnnotation(const nsString& aName, const bool aData) +{ + if (aData) { + AppendElement(HangAnnotation(aName, NS_LITERAL_STRING("true"))); + } else { + AppendElement(HangAnnotation(aName, NS_LITERAL_STRING("false"))); + } +} + +BackgroundHangAnnotators::BackgroundHangAnnotators() + : mMutex("BackgroundHangAnnotators::mMutex") +{ + MOZ_COUNT_CTOR(BackgroundHangAnnotators); +} + +BackgroundHangAnnotators::~BackgroundHangAnnotators() +{ + MOZ_ASSERT(mAnnotators.empty()); + MOZ_COUNT_DTOR(BackgroundHangAnnotators); +} + +bool +BackgroundHangAnnotators::Register(BackgroundHangAnnotator& aAnnotator) +{ + MutexAutoLock lock(mMutex); + auto result = mAnnotators.insert(&aAnnotator); + return result.second; +} + +bool +BackgroundHangAnnotators::Unregister(BackgroundHangAnnotator& aAnnotator) +{ + MutexAutoLock lock(mMutex); + DebugOnly::size_type> numErased; + numErased = mAnnotators.erase(&aAnnotator); + MOZ_ASSERT(numErased == 1); + return mAnnotators.empty(); +} + +BackgroundHangAnnotations +BackgroundHangAnnotators::GatherAnnotations() +{ + BackgroundHangAnnotations annotations; + { // Scope for lock + MutexAutoLock lock(mMutex); + for (std::set::iterator i = mAnnotators.begin(), + e = mAnnotators.end(); + i != e; ++i) { + (*i)->AnnotateHang(annotations); + } + } + return annotations; +} + +} // namespace mozilla diff --git a/toolkit/components/backgroundhangmonitor/HangAnnotations.h b/toolkit/components/backgroundhangmonitor/HangAnnotations.h new file mode 100644 index 000000000000..a444d5e05176 --- /dev/null +++ b/toolkit/components/backgroundhangmonitor/HangAnnotations.h @@ -0,0 +1,75 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=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/. */ + +#ifndef mozilla_HangAnnotations_h +#define mozilla_HangAnnotations_h + +#include + +#include "mozilla/HangTypes.h" +#include "mozilla/MemoryReporting.h" +#include "mozilla/Mutex.h" +#include "mozilla/Vector.h" +#include "nsString.h" +#include "nsTArray.h" +#include "mozilla/ipc/IPDLParamTraits.h" + +namespace mozilla { + +/** + * This class extends nsTArray with some methods for adding + * annotations being reported by a registered hang Annotator. + */ +class BackgroundHangAnnotations : public nsTArray +{ +public: + void AddAnnotation(const nsString& aName, const int32_t aData); + void AddAnnotation(const nsString& aName, const double aData); + void AddAnnotation(const nsString& aName, const nsString& aData); + void AddAnnotation(const nsString& aName, const nsCString& aData); + void AddAnnotation(const nsString& aName, const bool aData); +}; + +class BackgroundHangAnnotator +{ +public: + /** + * NB: This function is always called by the BackgroundHangMonitor thread. + * Plan accordingly. + */ + virtual void AnnotateHang(BackgroundHangAnnotations& aAnnotations) = 0; +}; + +class BackgroundHangAnnotators +{ +public: + BackgroundHangAnnotators(); + ~BackgroundHangAnnotators(); + + bool Register(BackgroundHangAnnotator& aAnnotator); + bool Unregister(BackgroundHangAnnotator& aAnnotator); + + BackgroundHangAnnotations GatherAnnotations(); + +private: + Mutex mMutex; + std::set mAnnotators; +}; + +namespace ipc { + +template<> +struct IPDLParamTraits + : public IPDLParamTraits> +{ + typedef mozilla::BackgroundHangAnnotations paramType; +}; + +} // namespace ipc + +} // namespace mozilla + +#endif // mozilla_HangAnnotations_h diff --git a/toolkit/components/backgroundhangmonitor/moz.build b/toolkit/components/backgroundhangmonitor/moz.build index 243f81d2097d..383beaba7351 100644 --- a/toolkit/components/backgroundhangmonitor/moz.build +++ b/toolkit/components/backgroundhangmonitor/moz.build @@ -31,11 +31,13 @@ XPIDL_MODULE = 'backgroundhangmonitor' EXPORTS.mozilla += [ 'BackgroundHangMonitor.h', + 'HangAnnotations.h', 'HangDetails.h', ] UNIFIED_SOURCES += [ 'BackgroundHangMonitor.cpp', + 'HangAnnotations.cpp', 'HangDetails.cpp', ] diff --git a/toolkit/components/telemetry/CombinedStacks.cpp b/toolkit/components/telemetry/CombinedStacks.cpp index 83b97dea20a8..7253e17c8396 100644 --- a/toolkit/components/telemetry/CombinedStacks.cpp +++ b/toolkit/components/telemetry/CombinedStacks.cpp @@ -5,7 +5,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "CombinedStacks.h" -#include "HangAnnotations.h" #include "mozilla/HangAnnotations.h" #include "jsapi.h" diff --git a/toolkit/components/telemetry/HangReports.cpp b/toolkit/components/telemetry/HangReports.cpp deleted file mode 100644 index c47fe92391b8..000000000000 --- a/toolkit/components/telemetry/HangReports.cpp +++ /dev/null @@ -1,148 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=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/. */ - -#include "HangReports.h" - -namespace mozilla { -namespace Telemetry { - -using namespace HangMonitor; - -// This utility function generates a string key that is used to index the annotations -// in a hash map from |HangReports::AddHang|. -nsresult -ComputeAnnotationsKey(const HangAnnotations& aAnnotations, nsAString& aKeyOut) -{ - if (aAnnotations.IsEmpty()) { - return NS_ERROR_FAILURE; - } - - for (auto& annotation : aAnnotations) { - aKeyOut.Append(annotation.mName); - aKeyOut.Append(annotation.mValue); - } - return NS_OK; -} - -#if defined(MOZ_GECKO_PROFILER) -/** The maximum number of stacks that we're keeping for hang reports. */ -const size_t kMaxHangStacksKept = 50; - -void -HangReports::AddHang(const Telemetry::ProcessedStack& aStack, - uint32_t aDuration, - int32_t aSystemUptime, - int32_t aFirefoxUptime, - HangAnnotations&& aAnnotations) { - // Append the new stack to the stack's circular queue. - size_t hangIndex = mStacks.AddStack(aStack); - // Append the hang info at the same index, in mHangInfo. - HangInfo info = { aDuration, aSystemUptime, aFirefoxUptime }; - if (mHangInfo.size() < kMaxHangStacksKept) { - mHangInfo.push_back(info); - } else { - mHangInfo[hangIndex] = info; - // Remove any reference to the stack overwritten in the circular queue - // from the annotations. - PruneStackReferences(hangIndex); - } - - nsAutoString annotationsKey; - // Generate a key to index aAnnotations in the hash map. - nsresult rv = ComputeAnnotationsKey(aAnnotations, annotationsKey); - if (NS_FAILED(rv)) { - return; - } - - AnnotationInfo* annotationsEntry = mAnnotationInfo.Get(annotationsKey); - if (annotationsEntry) { - // If the key is already in the hash map, append the index of the chrome hang - // to its indices. - annotationsEntry->mHangIndices.AppendElement(hangIndex); - return; - } - - // If the key was not found, add the annotations to the hash map. - mAnnotationInfo.Put(annotationsKey, new AnnotationInfo(hangIndex, std::move(aAnnotations))); -} - -/** - * This function removes links to discarded chrome hangs stacks and prunes unused - * annotations. - */ -void -HangReports::PruneStackReferences(const size_t aRemovedStackIndex) { - // We need to adjust the indices that link annotations to chrome hangs. Since we - // removed a stack, we must remove all references to it and prune annotations - // linked to no stacks. - for (auto iter = mAnnotationInfo.Iter(); !iter.Done(); iter.Next()) { - nsTArray& stackIndices = iter.Data()->mHangIndices; - size_t toRemove = stackIndices.NoIndex; - for (size_t k = 0; k < stackIndices.Length(); k++) { - // Is this index referencing the removed stack? - if (stackIndices[k] == aRemovedStackIndex) { - toRemove = k; - break; - } - } - - // Remove the index referencing the old stack from the annotation. - if (toRemove != stackIndices.NoIndex) { - stackIndices.RemoveElementAt(toRemove); - } - - // If this annotation no longer references any stack, drop it. - if (!stackIndices.Length()) { - iter.Remove(); - } - } -} -#endif - -size_t -HangReports::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const { - size_t n = 0; - n += mStacks.SizeOfExcludingThis(); - // This is a crude approximation. See comment on - // CombinedStacks::SizeOfExcludingThis. - n += mHangInfo.capacity() * sizeof(HangInfo); - n += mAnnotationInfo.ShallowSizeOfExcludingThis(aMallocSizeOf); - n += mAnnotationInfo.Count() * sizeof(AnnotationInfo); - for (auto iter = mAnnotationInfo.ConstIter(); !iter.Done(); iter.Next()) { - n += iter.Key().SizeOfExcludingThisIfUnshared(aMallocSizeOf); - auto& annotations = iter.Data()->mAnnotations; - n += annotations.ShallowSizeOfExcludingThis(aMallocSizeOf); - } - return n; -} - -const CombinedStacks& -HangReports::GetStacks() const { - return mStacks; -} - -uint32_t -HangReports::GetDuration(unsigned aIndex) const { - return mHangInfo[aIndex].mDuration; -} - -int32_t -HangReports::GetSystemUptime(unsigned aIndex) const { - return mHangInfo[aIndex].mSystemUptime; -} - -int32_t -HangReports::GetFirefoxUptime(unsigned aIndex) const { - return mHangInfo[aIndex].mFirefoxUptime; -} - -const nsClassHashtable& -HangReports::GetAnnotationInfo() const { - return mAnnotationInfo; -} - -} // namespace Telemetry -} // namespace mozilla diff --git a/toolkit/components/telemetry/HangReports.h b/toolkit/components/telemetry/HangReports.h deleted file mode 100644 index bb3c9efc2f52..000000000000 --- a/toolkit/components/telemetry/HangReports.h +++ /dev/null @@ -1,90 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; 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 HangReports_h__ -#define HangReports_h__ - -#include -#include "mozilla/HangAnnotations.h" -#include "ProcessedStack.h" -#include "nsTArray.h" -#include "nsString.h" -#include "nsClassHashtable.h" -#include "CombinedStacks.h" - -namespace mozilla { -namespace Telemetry { - -nsresult -ComputeAnnotationsKey(const HangMonitor::HangAnnotations& aAnnotations, nsAString& aKeyOut); - -class HangReports { -public: - /** - * This struct encapsulates information for an individual ChromeHang annotation. - * mHangIndex is the index of the corresponding ChromeHang. - */ - struct AnnotationInfo { - AnnotationInfo(uint32_t aHangIndex, - HangMonitor::HangAnnotations&& aAnnotations) - : mAnnotations(std::move(aAnnotations)) - { - mHangIndices.AppendElement(aHangIndex); - } - AnnotationInfo(AnnotationInfo&& aOther) - : mHangIndices(aOther.mHangIndices) - , mAnnotations(std::move(aOther.mAnnotations)) - {} - ~AnnotationInfo() = default; - AnnotationInfo& operator=(AnnotationInfo&& aOther) - { - mHangIndices = aOther.mHangIndices; - mAnnotations = std::move(aOther.mAnnotations); - return *this; - } - // To save memory, a single AnnotationInfo can be associated to multiple chrome - // hangs. The following array holds the index of each related chrome hang. - nsTArray mHangIndices; - HangMonitor::HangAnnotations mAnnotations; - - private: - // Force move constructor - AnnotationInfo(const AnnotationInfo& aOther) = delete; - void operator=(const AnnotationInfo& aOther) = delete; - }; - size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const; -#if defined(MOZ_GECKO_PROFILER) - void AddHang(const Telemetry::ProcessedStack& aStack, uint32_t aDuration, - int32_t aSystemUptime, int32_t aFirefoxUptime, - HangMonitor::HangAnnotations&& aAnnotations); - void PruneStackReferences(const size_t aRemovedStackIndex); -#endif - uint32_t GetDuration(unsigned aIndex) const; - int32_t GetSystemUptime(unsigned aIndex) const; - int32_t GetFirefoxUptime(unsigned aIndex) const; - const nsClassHashtable& GetAnnotationInfo() const; - const CombinedStacks& GetStacks() const; -private: - /** - * This struct encapsulates the data for an individual ChromeHang, excluding - * annotations. - */ - struct HangInfo { - // Hang duration (in seconds) - uint32_t mDuration; - // System uptime (in minutes) at the time of the hang - int32_t mSystemUptime; - // Firefox uptime (in minutes) at the time of the hang - int32_t mFirefoxUptime; - }; - std::vector mHangInfo; - nsClassHashtable mAnnotationInfo; - CombinedStacks mStacks; -}; - -} // namespace Telemetry -} // namespace mozilla - -#endif // CombinedStacks_h__ diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 0897e8f13ab2..0853106fe1be 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -82,13 +82,11 @@ #include "mozilla/IOInterposer.h" #include "mozilla/PoisonIOInterposer.h" #include "mozilla/StartupTimeline.h" -#include "mozilla/HangMonitor.h" #if defined(XP_WIN) #include "mozilla/WinDllServices.h" #endif #include "nsNativeCharsetUtils.h" #include "nsProxyRelease.h" -#include "HangReports.h" #if defined(MOZ_GECKO_PROFILER) #include "shared-libraries.h" @@ -102,7 +100,6 @@ namespace { using namespace mozilla; -using namespace mozilla::HangMonitor; using Telemetry::Common::AutoHashtable; using Telemetry::Common::ToJSString; using Telemetry::Common::GetCurrentProduct; @@ -110,9 +107,7 @@ using Telemetry::Common::SetCurrentProduct; using Telemetry::Common::SupportedProduct; using mozilla::dom::Promise; using mozilla::dom::AutoJSAPI; -using mozilla::Telemetry::HangReports; using mozilla::Telemetry::CombinedStacks; -using mozilla::Telemetry::ComputeAnnotationsKey; using mozilla::Telemetry::TelemetryIOInterposeObserver; #if defined(MOZ_GECKO_PROFILER) @@ -149,13 +144,6 @@ public: static void ShutdownTelemetry(); static void RecordSlowStatement(const nsACString &sql, const nsACString &dbName, uint32_t delay); -#if defined(MOZ_GECKO_PROFILER) - static void RecordChromeHang(uint32_t aDuration, - Telemetry::ProcessedStack &aStack, - int32_t aSystemUptime, - int32_t aFirefoxUptime, - HangAnnotations&& aAnnotations); -#endif #if defined(MOZ_GECKO_PROFILER) static void DoStackCapture(const nsACString& aKey); #endif @@ -205,8 +193,6 @@ private: AutoHashtable mPrivateSQL; AutoHashtable mSanitizedSQL; Mutex mHashMutex; - HangReports mHangReports; - Mutex mHangReportsMutex; Atomic mCanRecordBase; Atomic mCanRecordExtended; @@ -495,7 +481,6 @@ TelemetryImpl::AsyncFetchTelemetryData(nsIFetchTelemetryDataCallback *aCallback) TelemetryImpl::TelemetryImpl() : mHashMutex("Telemetry::mHashMutex") - , mHangReportsMutex("Telemetry::mHangReportsMutex") , mCanRecordBase(false) , mCanRecordExtended(false) , mCachedTelemetryData(false) @@ -513,7 +498,6 @@ TelemetryImpl::~TelemetryImpl() { // This is still racey as access to these collections is guarded using sTelemetry. // We will fix this in bug 1367344. MutexAutoLock hashLock(mHashMutex); - MutexAutoLock hangReportsLock(mHangReportsMutex); } void @@ -652,122 +636,6 @@ TelemetryImpl::GetMaximalNumberOfConcurrentThreads(uint32_t *ret) return NS_OK; } -NS_IMETHODIMP -TelemetryImpl::GetChromeHangs(JSContext *cx, JS::MutableHandle ret) -{ - MutexAutoLock hangReportMutex(mHangReportsMutex); - - const CombinedStacks& stacks = mHangReports.GetStacks(); - JS::Rooted fullReportObj(cx, CreateJSStackObject(cx, stacks)); - if (!fullReportObj) { - return NS_ERROR_FAILURE; - } - - ret.setObject(*fullReportObj); - - JS::Rooted durationArray(cx, JS_NewArrayObject(cx, 0)); - JS::Rooted systemUptimeArray(cx, JS_NewArrayObject(cx, 0)); - JS::Rooted firefoxUptimeArray(cx, JS_NewArrayObject(cx, 0)); - JS::Rooted annotationsArray(cx, JS_NewArrayObject(cx, 0)); - if (!durationArray || !systemUptimeArray || !firefoxUptimeArray || - !annotationsArray) { - return NS_ERROR_FAILURE; - } - - bool ok = JS_DefineProperty(cx, fullReportObj, "durations", - durationArray, JSPROP_ENUMERATE); - if (!ok) { - return NS_ERROR_FAILURE; - } - - ok = JS_DefineProperty(cx, fullReportObj, "systemUptime", - systemUptimeArray, JSPROP_ENUMERATE); - if (!ok) { - return NS_ERROR_FAILURE; - } - - ok = JS_DefineProperty(cx, fullReportObj, "firefoxUptime", - firefoxUptimeArray, JSPROP_ENUMERATE); - if (!ok) { - return NS_ERROR_FAILURE; - } - - ok = JS_DefineProperty(cx, fullReportObj, "annotations", annotationsArray, - JSPROP_ENUMERATE); - if (!ok) { - return NS_ERROR_FAILURE; - } - - - const size_t length = stacks.GetStackCount(); - for (size_t i = 0; i < length; ++i) { - if (!JS_DefineElement(cx, durationArray, i, mHangReports.GetDuration(i), - JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - if (!JS_DefineElement(cx, systemUptimeArray, i, mHangReports.GetSystemUptime(i), - JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - if (!JS_DefineElement(cx, firefoxUptimeArray, i, mHangReports.GetFirefoxUptime(i), - JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - - size_t annotationIndex = 0; - const nsClassHashtable& annotationInfo = - mHangReports.GetAnnotationInfo(); - - for (auto iter = annotationInfo.ConstIter(); !iter.Done(); iter.Next()) { - const HangReports::AnnotationInfo* info = iter.Data(); - - JS::Rooted keyValueArray(cx, JS_NewArrayObject(cx, 0)); - if (!keyValueArray) { - return NS_ERROR_FAILURE; - } - - // Create an array containing all the indices of the chrome hangs relative to this - // annotation. - JS::Rooted indicesArray(cx); - if (!mozilla::dom::ToJSValue(cx, info->mHangIndices, &indicesArray)) { - return NS_ERROR_OUT_OF_MEMORY; - } - - // We're saving the annotation as [[indices], {annotation-data}], so add the indices - // array as the first element of that structure. - if (!JS_DefineElement(cx, keyValueArray, 0, indicesArray, JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - - // Create the annotations object... - JS::Rooted jsAnnotation(cx, JS_NewPlainObject(cx)); - if (!jsAnnotation) { - return NS_ERROR_FAILURE; - } - - for (auto& annot : info->mAnnotations) { - JS::RootedValue jsValue(cx); - jsValue.setString(JS_NewUCStringCopyN(cx, annot.mValue.get(), annot.mValue.Length())); - if (!JS_DefineUCProperty(cx, jsAnnotation, annot.mName.get(), annot.mName.Length(), - jsValue, JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - } - - // ... and append it after the indices array. - if (!JS_DefineElement(cx, keyValueArray, 1, jsAnnotation, JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - if (!JS_DefineElement(cx, annotationsArray, annotationIndex++, - keyValueArray, JSPROP_ENUMERATE)) { - return NS_ERROR_FAILURE; - } - } - } - - return NS_OK; -} - NS_IMETHODIMP TelemetryImpl::SnapshotCapturedStacks(bool clear, JSContext *cx, JS::MutableHandle ret) { @@ -1587,22 +1455,6 @@ TelemetryImpl::RecordIceCandidates(const uint32_t iceCandidateBitmask, } #if defined(MOZ_GECKO_PROFILER) -void -TelemetryImpl::RecordChromeHang(uint32_t aDuration, - Telemetry::ProcessedStack &aStack, - int32_t aSystemUptime, - int32_t aFirefoxUptime, - HangAnnotations&& aAnnotations) -{ - if (!sTelemetry || !TelemetryHistogram::CanRecordExtended()) - return; - - MutexAutoLock hangReportMutex(sTelemetry->mHangReportsMutex); - - sTelemetry->mHangReports.AddHang(aStack, aDuration, - aSystemUptime, aFirefoxUptime, - std::move(aAnnotations)); -} void TelemetryImpl::DoStackCapture(const nsACString& aKey) { @@ -1899,10 +1751,6 @@ TelemetryImpl::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) n += mPrivateSQL.SizeOfExcludingThis(aMallocSizeOf); n += mSanitizedSQL.SizeOfExcludingThis(aMallocSizeOf); } - { // Scope for mHangReportsMutex lock - MutexAutoLock lock(mHangReportsMutex); - n += mHangReports.SizeOfExcludingThis(aMallocSizeOf); - } // It's a bit gross that we measure this other stuff that lives outside of // TelemetryImpl... oh well. @@ -2160,22 +2008,9 @@ void Init() } #if defined(MOZ_GECKO_PROFILER) -void RecordChromeHang(uint32_t duration, - ProcessedStack &aStack, - int32_t aSystemUptime, - int32_t aFirefoxUptime, - HangAnnotations&& aAnnotations) -{ - TelemetryImpl::RecordChromeHang(duration, aStack, - aSystemUptime, aFirefoxUptime, - std::move(aAnnotations)); -} - void CaptureStack(const nsACString& aKey) { -#ifdef MOZ_GECKO_PROFILER TelemetryImpl::DoStackCapture(aKey); -#endif } #endif diff --git a/toolkit/components/telemetry/Telemetry.h b/toolkit/components/telemetry/Telemetry.h index 29b5982baeeb..9e47314f0a3a 100644 --- a/toolkit/components/telemetry/Telemetry.h +++ b/toolkit/components/telemetry/Telemetry.h @@ -29,9 +29,6 @@ *****************************************************************************/ namespace mozilla { -namespace HangMonitor { - class HangAnnotations; -} // namespace HangMonitor namespace Telemetry { struct HistogramAccumulation; @@ -488,12 +485,6 @@ class ProcessedStack; * @param aAnnotations - Any annotations to be added to the report */ #if defined(MOZ_GECKO_PROFILER) -void RecordChromeHang(uint32_t aDuration, - ProcessedStack &aStack, - int32_t aSystemUptime, - int32_t aFirefoxUptime, - mozilla::HangMonitor::HangAnnotations&& aAnnotations); - /** * Record the current thread's call stack on demand. Note that, the stack is * only captured once. Subsequent calls result in incrementing the capture diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index e94b9cbfc916..b625f19be4f7 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -1211,7 +1211,6 @@ var Impl = { // Add extended set measurements common to chrome & content processes if (Telemetry.canRecordExtended) { - payloadObj.chromeHangs = protect(() => Telemetry.chromeHangs); payloadObj.log = []; payloadObj.webrtc = protect(() => Telemetry.webrtcStats); } diff --git a/toolkit/components/telemetry/docs/data/main-ping.rst b/toolkit/components/telemetry/docs/data/main-ping.rst index d2ae2d39e458..188daf358b90 100644 --- a/toolkit/components/telemetry/docs/data/main-ping.rst +++ b/toolkit/components/telemetry/docs/data/main-ping.rst @@ -61,7 +61,7 @@ Structure: // The following properties may all be null if we fail to collect them. histograms: {...}, keyedHistograms: {...}, - chromeHangs: {...}, + chromeHangs: {...}, // removed in firefox 62 threadHangStats: [...], // obsolete in firefox 57, use the 'bhr' ping capturedStacks: {...}, log: [...], // obsolete in firefox 61, use Event Telemetry or Scalars @@ -349,6 +349,9 @@ Structure: chromeHangs ----------- +As of Firefox 62, chromeHangs has been removed. Please look to the bhr ping for +similar functionality. + Contains the statistics about the hangs happening exclusively on the main thread of the parent process. Precise C++ stacks are reported. This is only available on Nightly Release on Windows, when building using "--enable-profiling" switch. Some limits are applied: diff --git a/toolkit/components/telemetry/moz.build b/toolkit/components/telemetry/moz.build index 336ce772b458..bf267821d536 100644 --- a/toolkit/components/telemetry/moz.build +++ b/toolkit/components/telemetry/moz.build @@ -63,7 +63,6 @@ EXPORTS.mozilla += [ SOURCES += [ 'CombinedStacks.cpp', - 'HangReports.cpp', 'ipc/TelemetryIPC.cpp', 'ipc/TelemetryIPCAccumulator.cpp', 'ProcessedStack.cpp', diff --git a/toolkit/components/telemetry/nsITelemetry.idl b/toolkit/components/telemetry/nsITelemetry.idl index bc1413d75277..fd13f11d41b7 100644 --- a/toolkit/components/telemetry/nsITelemetry.idl +++ b/toolkit/components/telemetry/nsITelemetry.idl @@ -128,14 +128,6 @@ interface nsITelemetry : nsISupports */ readonly attribute uint32_t maximalNumberOfConcurrentThreads; - /* - * An array of chrome hang reports. Each element is a hang report represented - * as an object containing the hang duration, call stack PCs and information - * about modules in memory. - */ - [implicit_jscontext] - readonly attribute jsval chromeHangs; - /* * Record the current thread's call stack on demand. Note that, the stack is * only captured at the first call. All subsequent calls result in incrementing diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index 46df9ec2db97..637dce78fb3c 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -1829,7 +1829,7 @@ add_task(async function test_schedulerNothingDue() { add_task(async function test_pingExtendedStats() { const EXTENDED_PAYLOAD_FIELDS = [ - "chromeHangs", "log", "slowSQL", "fileIOReports", "lateWrites", + "log", "slowSQL", "fileIOReports", "lateWrites", "addonDetails", "webrtc" ]; diff --git a/toolkit/content/aboutTelemetry.js b/toolkit/content/aboutTelemetry.js index 544f340d8dd7..b46607a1b61e 100644 --- a/toolkit/content/aboutTelemetry.js +++ b/toolkit/content/aboutTelemetry.js @@ -999,32 +999,6 @@ function SymbolicationRequest_fetchSymbols() { this.symbolRequest.send(requestJSON); }; -var ChromeHangs = { - - symbolRequest: null, - - /** - * Renders raw chrome hang data - */ - render: function ChromeHangs_render(chromeHangs) { - setHasData("chrome-hangs-section", !!chromeHangs); - if (!chromeHangs) { - return; - } - - let stacks = chromeHangs.stacks; - let memoryMap = chromeHangs.memoryMap; - let durations = chromeHangs.durations; - - StackRenderer.renderStacks("chrome-hangs", stacks, memoryMap, - (index) => this.renderHangHeader(index, durations)); - }, - - renderHangHeader: function ChromeHangs_renderHangHeader(aIndex, aDurations) { - StackRenderer.renderHeader("chrome-hangs", [aIndex + 1, aDurations[aIndex]]); - } -}; - var CapturedStacks = { symbolRequest: null, @@ -1227,7 +1201,6 @@ var Search = { // A list of ids of sections that do not support search. blacklist: [ "late-writes-section", - "chrome-hangs-section", "raw-payload-section" ], @@ -1994,30 +1967,6 @@ function setupListeners() { Settings.detachObservers(); }, {once: true}); - document.getElementById("chrome-hangs-fetch-symbols").addEventListener("click", - function() { - if (!gPingData) { - return; - } - - let hangs = gPingData.payload.chromeHangs; - let req = new SymbolicationRequest("chrome-hangs", - ChromeHangs.renderHangHeader, - hangs.memoryMap, - hangs.stacks, - hangs.durations); - req.fetchSymbols(); - }); - - document.getElementById("chrome-hangs-hide-symbols").addEventListener("click", - function() { - if (!gPingData) { - return; - } - - ChromeHangs.render(gPingData.payload.chromeHangs); - }); - document.getElementById("captured-stacks-fetch-symbols").addEventListener("click", function() { if (!gPingData) { @@ -2406,9 +2355,6 @@ function displayRichPingData(ping, updatePayloadList) { LateWritesSingleton.renderLateWrites(payload.lateWrites); - // Show chrome hang stacks - ChromeHangs.render(payload.chromeHangs); - // Show simple measurements SimpleMeasurements.render(payload); diff --git a/toolkit/content/aboutTelemetry.xhtml b/toolkit/content/aboutTelemetry.xhtml index e536add45987..c078f31dc95d 100644 --- a/toolkit/content/aboutTelemetry.xhtml +++ b/toolkit/content/aboutTelemetry.xhtml @@ -66,9 +66,6 @@
&aboutTelemetry.slowSqlSection;
-
- &aboutTelemetry.chromeHangsSection; -
&aboutTelemetry.addonDetailsSection;
@@ -194,12 +191,6 @@
-
- &aboutTelemetry.fetchStackSymbols; - &aboutTelemetry.hideStackSymbols; -
-
-
&aboutTelemetry.fetchStackSymbols; &aboutTelemetry.hideStackSymbols; diff --git a/toolkit/crashreporter/docs/index.rst b/toolkit/crashreporter/docs/index.rst index 93f59085ee4f..b98cbf63bc21 100644 --- a/toolkit/crashreporter/docs/index.rst +++ b/toolkit/crashreporter/docs/index.rst @@ -199,14 +199,6 @@ unit. Before submission, the filenames of the files are linked: - **uuid.dmp** - *plugin process dump file* - **uuid-.dmp** - *other process dump file as listed in additional_minidumps* -Browser Hangs -============= - -There is a feature of Firefox that will crash Firefox if it stops processing -messages after a certain period of time. This feature doesn't work well and is -disabled by default. See ``xpcom/threads/HangMonitor.cpp``. Hang crashes -are annotated with ``Hang=1``. - about:crashes ============= diff --git a/toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd b/toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd index a3d0bd336531..d94a93db4761 100644 --- a/toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd +++ b/toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd @@ -37,7 +37,6 @@ - diff --git a/toolkit/locales/en-US/chrome/global/aboutTelemetry.properties b/toolkit/locales/en-US/chrome/global/aboutTelemetry.properties index 8f2e2dacf5e4..90b1df2496fd 100644 --- a/toolkit/locales/en-US/chrome/global/aboutTelemetry.properties +++ b/toolkit/locales/en-US/chrome/global/aboutTelemetry.properties @@ -81,10 +81,6 @@ keysHeader = Property namesHeader = Name valuesHeader = Value -# LOCALIZATION NOTE(chrome-hangs-title): -# - %1$S is replaced by the number of the hang -# - %2$S is replaced by the duration of the hang -chrome-hangs-title = Hang Report #%1$S (%2$S seconds) # LOCALIZATION NOTE(captured-stacks-title): # - %1$S is replaced by the string key for this stack # - %2$S is replaced by the number of times this stack was captured diff --git a/widget/android/GeckoEditableSupport.h b/widget/android/GeckoEditableSupport.h index 9c27f1a332b8..dfafdb589a23 100644 --- a/widget/android/GeckoEditableSupport.h +++ b/widget/android/GeckoEditableSupport.h @@ -145,15 +145,15 @@ public: { explicit IMEEvent(Functor&& l) : nsAppShell::LambdaEvent(std::move(l)) {} - nsAppShell::Event::Type ActivityType() const override + bool IsUiEvent() const override { using GES = GeckoEditableSupport; if (this->lambda.IsTarget(&GES::OnKeyEvent) || this->lambda.IsTarget(&GES::OnImeReplaceText) || this->lambda.IsTarget(&GES::OnImeUpdateComposition)) { - return nsAppShell::Event::Type::kUIActivity; + return true; } - return nsAppShell::Event::Type::kGeneralActivity; + return false; } void Run() override diff --git a/widget/android/nsAppShell.cpp b/widget/android/nsAppShell.cpp index 440797d3af0f..6654af45e107 100644 --- a/widget/android/nsAppShell.cpp +++ b/widget/android/nsAppShell.cpp @@ -726,7 +726,7 @@ nsAppShell::ProcessNextNativeEvent(bool mayWait) AUTO_PROFILER_LABEL("nsAppShell::ProcessNextNativeEvent:Wait", IDLE); - mozilla::HangMonitor::Suspend(); + mozilla::BackgroundHangMonitor().NotifyWait(); curEvent = mEventQueue.Pop(/* mayWait */ true); } @@ -735,7 +735,7 @@ nsAppShell::ProcessNextNativeEvent(bool mayWait) if (!curEvent) return false; - mozilla::HangMonitor::NotifyActivity(curEvent->ActivityType()); + mozilla::BackgroundHangMonitor().NotifyActivity(); curEvent->Run(); return true; diff --git a/widget/android/nsAppShell.h b/widget/android/nsAppShell.h index 9a3fe4fb5124..cc3bd9056af2 100644 --- a/widget/android/nsAppShell.h +++ b/widget/android/nsAppShell.h @@ -8,7 +8,7 @@ #include -#include "mozilla/HangMonitor.h" +#include "mozilla/BackgroundHangMonitor.h" #include "mozilla/LinkedList.h" #include "mozilla/Monitor.h" #include "mozilla/Move.h" @@ -36,8 +36,6 @@ class nsAppShell : public: struct Event : mozilla::LinkedListElement { - typedef mozilla::HangMonitor::ActivityType Type; - static uint64_t GetTime() { timespec time; @@ -64,9 +62,9 @@ public: queue.insertBack(this); } - virtual Type ActivityType() const + virtual bool IsUIEvent() const { - return Type::kGeneralActivity; + return false; } }; @@ -248,8 +246,7 @@ protected: } #ifdef EARLY_BETA_OR_EARLIER - const size_t latencyType = (event->ActivityType() == - Event::Type::kUIActivity) ? LATENCY_UI : LATENCY_OTHER; + const size_t latencyType = event->IsUIEvent() ? LATENCY_UI : LATENCY_OTHER; const uint64_t latency = Event::GetTime() - event->mPostTime; sLatencyCount[latencyType]++; diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index 796111ccb3e9..33b6d9ad122b 100644 --- a/widget/android/nsWindow.cpp +++ b/widget/android/nsWindow.cpp @@ -353,9 +353,9 @@ class nsWindow::NPZCSupport final return mLambda(window); } - nsAppShell::Event::Type ActivityType() const override + bool IsUIEvent() const override { - return nsAppShell::Event::Type::kUIActivity; + return true; } }; diff --git a/widget/cocoa/nsAppShell.mm b/widget/cocoa/nsAppShell.mm index a9eda60811ab..83a08ab8e516 100644 --- a/widget/cocoa/nsAppShell.mm +++ b/widget/cocoa/nsAppShell.mm @@ -30,7 +30,7 @@ #include "nsChildView.h" #include "nsToolkit.h" #include "TextInputHandler.h" -#include "mozilla/HangMonitor.h" +#include "mozilla/BackgroundHangMonitor.h" #include "GeckoProfiler.h" #include "ScreenHelperCocoa.h" #include "mozilla/widget/ScreenManager.h" @@ -136,7 +136,8 @@ static bool gAppShellMethodsSwizzled = false; // into non-idle code. So we basically unset the IDLE category from the inside. AUTO_PROFILER_LABEL("-[GeckoNSApplication sendEvent:]", OTHER); - mozilla::HangMonitor::NotifyActivity(); + mozilla::BackgroundHangMonitor().NotifyActivity(); + if ([anEvent type] == NSApplicationDefined && [anEvent subtype] == kEventSubtypeTrace) { mozilla::SignalTracerThread(); @@ -172,12 +173,12 @@ static bool gAppShellMethodsSwizzled = false; AUTO_PROFILER_LABEL("-[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]", IDLE); if (expiration) { - mozilla::HangMonitor::Suspend(); + mozilla::BackgroundHangMonitor().NotifyWait(); } NSEvent* nextEvent = [super nextEventMatchingMask:mask untilDate:expiration inMode:mode dequeue:flag]; if (expiration) { - mozilla::HangMonitor::NotifyActivity(); + mozilla::BackgroundHangMonitor().NotifyActivity(); } return nextEvent; } @@ -613,7 +614,7 @@ nsAppShell::ProcessNextNativeEvent(bool aMayWait) EventTargetRef eventDispatcherTarget = GetEventDispatcherTarget(); if (aMayWait) { - mozilla::HangMonitor::Suspend(); + mozilla::BackgroundHangMonitor().NotifyWait(); } // Only call -[NSApp sendEvent:] (and indirectly send user-input events to @@ -639,7 +640,7 @@ nsAppShell::ProcessNextNativeEvent(bool aMayWait) inMode:currentMode dequeue:YES]; if (nextEvent) { - mozilla::HangMonitor::NotifyActivity(); + mozilla::BackgroundHangMonitor().NotifyActivity(); [NSApp sendEvent:nextEvent]; eventProcessed = true; } diff --git a/widget/gtk/nsAppShell.cpp b/widget/gtk/nsAppShell.cpp index bba47d093964..778eac97c1a3 100644 --- a/widget/gtk/nsAppShell.cpp +++ b/widget/gtk/nsAppShell.cpp @@ -14,7 +14,7 @@ #include "nsWindow.h" #include "mozilla/Logging.h" #include "prenv.h" -#include "mozilla/HangMonitor.h" +#include "mozilla/BackgroundHangMonitor.h" #include "mozilla/Unused.h" #include "mozilla/WidgetUtils.h" #include "GeckoProfiler.h" @@ -46,14 +46,14 @@ static GPollFunc sPollFunc; static gint PollWrapper(GPollFD *ufds, guint nfsd, gint timeout_) { - mozilla::HangMonitor::Suspend(); + mozilla::BackgroundHangMonitor().NotifyWait(); gint result; { AUTO_PROFILER_LABEL("PollWrapper", IDLE); AUTO_PROFILER_THREAD_SLEEP; result = (*sPollFunc)(ufds, nfsd, timeout_); } - mozilla::HangMonitor::NotifyActivity(); + mozilla::BackgroundHangMonitor().NotifyActivity(); return result; } diff --git a/widget/windows/WinUtils.cpp b/widget/windows/WinUtils.cpp index 83f9403a5fcf..548d0af18d01 100644 --- a/widget/windows/WinUtils.cpp +++ b/widget/windows/WinUtils.cpp @@ -15,10 +15,10 @@ #include "nsWindowDefs.h" #include "KeyboardLayout.h" #include "mozilla/ArrayUtils.h" +#include "mozilla/BackgroundHangMonitor.h" #include "mozilla/dom/MouseEventBinding.h" #include "mozilla/gfx/2D.h" #include "mozilla/gfx/DataSurfaceHelpers.h" -#include "mozilla/HangMonitor.h" #include "mozilla/Preferences.h" #include "mozilla/RefPtr.h" #include "mozilla/WindowsVersion.h" @@ -764,7 +764,7 @@ WinUtils::WaitForMessage(DWORD aTimeoutMs) // We executed an APC that would have woken up the hang monitor. Since // there are no more APCs pending and we are now going to sleep again, // we should notify the hang monitor. - mozilla::HangMonitor::Suspend(); + mozilla::BackgroundHangMonitor().NotifyWait(); } continue; } diff --git a/widget/windows/nsAppShell.cpp b/widget/windows/nsAppShell.cpp index 31b530971b59..3d1e67e8d65b 100644 --- a/widget/windows/nsAppShell.cpp +++ b/widget/windows/nsAppShell.cpp @@ -15,7 +15,7 @@ #include "nsString.h" #include "WinIMEHandler.h" #include "mozilla/widget/AudioSession.h" -#include "mozilla/HangMonitor.h" +#include "mozilla/BackgroundHangMonitor.h" #include "nsIDOMWakeLockListener.h" #include "nsIPowerManagerService.h" #include "mozilla/StaticPtr.h" @@ -529,9 +529,7 @@ nsAppShell::ProcessNextNativeEvent(bool mayWait) } else { // If we had UI activity we would be processing it now so we know we // have either kUIActivity or kActivityNoUIAVail. - mozilla::HangMonitor::NotifyActivity( - uiMessage ? mozilla::HangMonitor::kUIActivity : - mozilla::HangMonitor::kActivityNoUIAVail); + mozilla::BackgroundHangMonitor().NotifyActivity(); if (msg.message >= WM_KEYFIRST && msg.message <= WM_KEYLAST && IMEHandler::ProcessRawKeyMessage(msg)) { @@ -552,7 +550,7 @@ nsAppShell::ProcessNextNativeEvent(bool mayWait) } } else if (mayWait) { // Block and wait for any posted application message - mozilla::HangMonitor::Suspend(); + mozilla::BackgroundHangMonitor().NotifyWait(); { AUTO_PROFILER_LABEL("nsAppShell::ProcessNextNativeEvent::Wait", IDLE); AUTO_PROFILER_THREAD_SLEEP; diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index 1998fd71a447..56ab907d5216 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -182,7 +182,7 @@ #include "nsIContent.h" -#include "mozilla/HangMonitor.h" +#include "mozilla/BackgroundHangMonitor.h" #include "WinIMEHandler.h" #include "npapi.h" @@ -4957,20 +4957,6 @@ DisplaySystemMenu(HWND hWnd, nsSizeMode sizeMode, bool isRtl, int32_t x, int32_t return false; } -inline static mozilla::HangMonitor::ActivityType ActivityTypeForMessage(UINT msg) -{ - if ((msg >= WM_KEYFIRST && msg <= WM_IME_KEYLAST) || - (msg >= WM_MOUSEFIRST && msg <= WM_MOUSELAST) || - (msg >= MOZ_WM_MOUSEWHEEL_FIRST && msg <= MOZ_WM_MOUSEWHEEL_LAST) || - (msg >= NS_WM_IMEFIRST && msg <= NS_WM_IMELAST)) { - return mozilla::HangMonitor::kUIActivity; - } - - // This may not actually be right, but we don't want to reset the timer if - // we're not actually processing a UI message. - return mozilla::HangMonitor::kActivityUIAVail; -} - // The WndProc procedure for all nsWindows in this toolkit. This merely catches // exceptions and passes the real work to WindowProcInternal. See bug 587406 // and http://msdn.microsoft.com/en-us/library/ms633573%28VS.85%29.aspx @@ -4978,7 +4964,7 @@ LRESULT CALLBACK nsWindow::WindowProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM { mozilla::ipc::CancelCPOWs(); - HangMonitor::NotifyActivity(ActivityTypeForMessage(msg)); + BackgroundHangMonitor().NotifyActivity(); return mozilla::CallWindowProcCrashProtected(WindowProcInternal, hWnd, msg, wParam, lParam); } diff --git a/xpcom/build/XPCOMInit.cpp b/xpcom/build/XPCOMInit.cpp index e61799bfd6fa..dd1975f919ee 100644 --- a/xpcom/build/XPCOMInit.cpp +++ b/xpcom/build/XPCOMInit.cpp @@ -107,7 +107,6 @@ extern nsresult nsStringInputStreamConstructor(nsISupports*, REFNSIID, void**); #include #include "mozilla/Services.h" #include "mozilla/Omnijar.h" -#include "mozilla/HangMonitor.h" #include "mozilla/ScriptPreloader.h" #include "mozilla/SystemGroup.h" #include "mozilla/Telemetry.h" @@ -739,7 +738,6 @@ NS_InitXPCOM2(nsIServiceManager** aResult, mozilla::Telemetry::Init(); - mozilla::HangMonitor::Startup(); mozilla::BackgroundHangMonitor::Startup(); const MessageLoop* const loop = MessageLoop::current(); @@ -798,7 +796,6 @@ NS_InitMinimalXPCOM() SharedThreadPool::InitStatics(); mozilla::Telemetry::Init(); - mozilla::HangMonitor::Startup(); mozilla::BackgroundHangMonitor::Startup(); return NS_OK; @@ -864,7 +861,7 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) { // Make sure the hang monitor is enabled for shutdown. - HangMonitor::NotifyActivity(); + BackgroundHangMonitor().NotifyActivity(); if (!NS_IsMainThread()) { MOZ_CRASH("Shutdown on wrong thread"); @@ -937,7 +934,7 @@ ShutdownXPCOM(nsIServiceManager* aServMgr) NS_ProcessPendingEvents(thread); - HangMonitor::NotifyActivity(); + BackgroundHangMonitor().NotifyActivity(); // Late-write checks needs to find the profile directory, so it has to // be initialized before mozilla::services::Shutdown or (because of @@ -1090,7 +1087,6 @@ ShutdownXPCOM(nsIServiceManager* aServMgr) Omnijar::CleanUp(); - HangMonitor::Shutdown(); BackgroundHangMonitor::Shutdown(); delete sMainHangMonitor; diff --git a/xpcom/threads/CPUUsageWatcher.cpp b/xpcom/threads/CPUUsageWatcher.cpp index e00a4761d289..6ab32b44ffd0 100644 --- a/xpcom/threads/CPUUsageWatcher.cpp +++ b/xpcom/threads/CPUUsageWatcher.cpp @@ -187,7 +187,7 @@ CPUUsageWatcher::Init() CPUUsageWatcher* self = this; NS_DispatchToMainThread( NS_NewRunnableFunction("CPUUsageWatcher::Init", - [=]() { HangMonitor::RegisterAnnotator(*self); })); + [=]() { BackgroundHangMonitor::RegisterAnnotator(*self); })); return Ok(); } @@ -196,7 +196,7 @@ void CPUUsageWatcher::Uninit() { if (mInitialized) { - HangMonitor::UnregisterAnnotator(*this); + BackgroundHangMonitor::UnregisterAnnotator(*this); } mInitialized = false; } @@ -239,7 +239,7 @@ CPUUsageWatcher::CollectCPUUsage() } void -CPUUsageWatcher::AnnotateHang(HangMonitor::HangAnnotations& aAnnotations) { +CPUUsageWatcher::AnnotateHang(BackgroundHangAnnotations& aAnnotations) { if (!mInitialized) { return; } @@ -265,7 +265,7 @@ CPUUsageWatcher::CollectCPUUsage() return Ok(); } -void CPUUsageWatcher::AnnotateHang(HangMonitor::HangAnnotations& aAnnotations) {} +void CPUUsageWatcher::AnnotateHang(BackgroundHangAnnotations& aAnnotations) {} #endif // CPU_USAGE_WATCHER_ACTIVE diff --git a/xpcom/threads/CPUUsageWatcher.h b/xpcom/threads/CPUUsageWatcher.h index 5e8e58981237..d1a1aec9f36d 100644 --- a/xpcom/threads/CPUUsageWatcher.h +++ b/xpcom/threads/CPUUsageWatcher.h @@ -10,6 +10,7 @@ #include #include "mozilla/HangAnnotations.h" +#include "mozilla/BackgroundHangMonitor.h" // We only support OSX and Windows, because on Linux we're forced to read // from /proc/stat in order to get global CPU values. We would prefer to not @@ -31,13 +32,13 @@ enum CPUUsageWatcherError : uint8_t }; class CPUUsageHangAnnotator - : public HangMonitor::Annotator + : public BackgroundHangAnnotator { public: }; class CPUUsageWatcher - : public HangMonitor::Annotator + : public BackgroundHangAnnotator { public: #ifdef CPU_USAGE_WATCHER_ACTIVE @@ -61,7 +62,7 @@ public: // usage values between now and the last time it was called. Result CollectCPUUsage(); - void AnnotateHang(HangMonitor::HangAnnotations& aAnnotations) final; + void AnnotateHang(BackgroundHangAnnotations& aAnnotations) final; private: #ifdef CPU_USAGE_WATCHER_ACTIVE bool mInitialized; diff --git a/xpcom/threads/HangAnnotations.cpp b/xpcom/threads/HangAnnotations.cpp deleted file mode 100644 index f6765dc3d167..000000000000 --- a/xpcom/threads/HangAnnotations.cpp +++ /dev/null @@ -1,175 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=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/. */ - -#include "mozilla/HangAnnotations.h" - -#include - -#include "MainThreadUtils.h" -#include "mozilla/DebugOnly.h" -#include "nsXULAppAPI.h" -#include "mozilla/BackgroundHangMonitor.h" - -namespace mozilla { -namespace HangMonitor { - -// Chrome hang annotators. This can go away once BHR has completely replaced -// ChromeHangs. -static StaticAutoPtr gChromehangAnnotators; - -void -HangAnnotations::AddAnnotation(const nsAString& aName, const int32_t aData) -{ - nsAutoString dataString; - dataString.AppendInt(aData); - AppendElement(Annotation(aName, dataString)); -} - -void -HangAnnotations::AddAnnotation(const nsAString& aName, const double aData) -{ - nsAutoString dataString; - dataString.AppendFloat(aData); - AppendElement(Annotation(aName, dataString)); -} - -void -HangAnnotations::AddAnnotation(const nsAString& aName, const nsAString& aData) -{ - AppendElement(Annotation(aName, aData)); -} - -void -HangAnnotations::AddAnnotation(const nsAString& aName, const nsACString& aData) -{ - NS_ConvertUTF8toUTF16 dataString(aData); - AppendElement(Annotation(aName, dataString)); -} - -void -HangAnnotations::AddAnnotation(const nsAString& aName, const bool aData) -{ - if (aData) { - AppendElement(Annotation(aName, NS_LITERAL_STRING("true"))); - } else { - AppendElement(Annotation(aName, NS_LITERAL_STRING("false"))); - } -} - -namespace Observer { - -Annotators::Annotators() - : mMutex("HangMonitor::Annotators::mMutex") -{ - MOZ_COUNT_CTOR(Annotators); -} - -Annotators::~Annotators() -{ - MOZ_ASSERT(mAnnotators.empty()); - MOZ_COUNT_DTOR(Annotators); -} - -bool -Annotators::Register(Annotator& aAnnotator) -{ - MutexAutoLock lock(mMutex); - auto result = mAnnotators.insert(&aAnnotator); - return result.second; -} - -bool -Annotators::Unregister(Annotator& aAnnotator) -{ - MutexAutoLock lock(mMutex); - DebugOnly::size_type> numErased; - numErased = mAnnotators.erase(&aAnnotator); - MOZ_ASSERT(numErased == 1); - return mAnnotators.empty(); -} - -HangAnnotations -Annotators::GatherAnnotations() -{ - HangAnnotations annotations; - { // Scope for lock - MutexAutoLock lock(mMutex); - for (std::set::iterator i = mAnnotators.begin(), - e = mAnnotators.end(); - i != e; ++i) { - (*i)->AnnotateHang(annotations); - } - } - return annotations; -} - -} // namespace Observer - -void -RegisterAnnotator(Annotator& aAnnotator) -{ - BackgroundHangMonitor::RegisterAnnotator(aAnnotator); - // We still register annotators for ChromeHangs - if (NS_IsMainThread() && - GeckoProcessType_Default == XRE_GetProcessType()) { - if (!gChromehangAnnotators) { - gChromehangAnnotators = new Observer::Annotators(); - } - gChromehangAnnotators->Register(aAnnotator); - } -} - -void -UnregisterAnnotator(Annotator& aAnnotator) -{ - BackgroundHangMonitor::UnregisterAnnotator(aAnnotator); - // We still register annotators for ChromeHangs - if (NS_IsMainThread() && - GeckoProcessType_Default == XRE_GetProcessType()) { - if (gChromehangAnnotators->Unregister(aAnnotator)) { - gChromehangAnnotators = nullptr; - } - } -} - -HangAnnotations -ChromeHangAnnotatorCallout() -{ - if (!gChromehangAnnotators) { - return HangAnnotations(); - } - return gChromehangAnnotators->GatherAnnotations(); -} - -} // namespace HangMonitor -} // namespace mozilla - -namespace IPC { - -using mozilla::HangMonitor::Annotation; - -void -ParamTraits::Write(Message* aMsg, const Annotation& aParam) -{ - WriteParam(aMsg, aParam.mName); - WriteParam(aMsg, aParam.mValue); -} - -bool -ParamTraits::Read(const Message* aMsg, - PickleIterator* aIter, - Annotation* aResult) -{ - if (!ReadParam(aMsg, aIter, &aResult->mName)) { - return false; - } - if (!ReadParam(aMsg, aIter, &aResult->mValue)) { - return false; - } - return true; -} - -} // namespace IPC diff --git a/xpcom/threads/HangAnnotations.h b/xpcom/threads/HangAnnotations.h deleted file mode 100644 index 84a76d76ec0d..000000000000 --- a/xpcom/threads/HangAnnotations.h +++ /dev/null @@ -1,127 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=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/. */ - -#ifndef mozilla_HangAnnotations_h -#define mozilla_HangAnnotations_h - -#include - -#include "ipc/IPCMessageUtils.h" -#include "mozilla/MemoryReporting.h" -#include "mozilla/Mutex.h" -#include "mozilla/Vector.h" -#include "nsString.h" -#include "nsTArray.h" - -namespace mozilla { -namespace HangMonitor { - -/** - * This type represents an individual hang annotation. - */ -class Annotation -{ -public: - Annotation() {} - Annotation(const nsAString& aName, const nsAString& aValue) - : mName(aName), mValue(aValue) - {} - - nsString mName; - nsString mValue; -}; - -/** - * This class extends nsTArray with some methods for adding - * annotations being reported by a registered hang Annotator. - */ -class HangAnnotations : public nsTArray -{ -public: - void AddAnnotation(const nsAString& aName, const int32_t aData); - void AddAnnotation(const nsAString& aName, const double aData); - void AddAnnotation(const nsAString& aName, const nsAString& aData); - void AddAnnotation(const nsAString& aName, const nsACString& aData); - void AddAnnotation(const nsAString& aName, const bool aData); -}; - -class Annotator -{ -public: - /** - * NB: This function is always called by the HangMonitor thread. - * Plan accordingly. - */ - virtual void AnnotateHang(HangAnnotations& aAnnotations) = 0; -}; - -/** - * Registers an Annotator to be called when a hang is detected. - * @param aAnnotator Reference to an object that implements the - * HangMonitor::Annotator interface. - */ -void RegisterAnnotator(Annotator& aAnnotator); - -/** - * Registers an Annotator that was previously registered via RegisterAnnotator. - * @param aAnnotator Reference to an object that implements the - * HangMonitor::Annotator interface. - */ -void UnregisterAnnotator(Annotator& aAnnotator); - -/** - * Gathers annotations. This function should be called by ChromeHangs. - * @return HangAnnotations object. - */ -HangAnnotations ChromeHangAnnotatorCallout(); - -namespace Observer { - -class Annotators -{ -public: - Annotators(); - ~Annotators(); - - bool Register(Annotator& aAnnotator); - bool Unregister(Annotator& aAnnotator); - - HangAnnotations GatherAnnotations(); - -private: - Mutex mMutex; - std::set mAnnotators; -}; - -} // namespace Observer - -} // namespace HangMonitor -} // namespace mozilla - -namespace IPC { - -template<> -class ParamTraits - : public ParamTraits> -{ -public: - typedef mozilla::HangMonitor::HangAnnotations paramType; -}; - -template<> -class ParamTraits -{ -public: - typedef mozilla::HangMonitor::Annotation paramType; - static void Write(Message* aMsg, const paramType& aParam); - static bool Read(const Message* aMsg, - PickleIterator* aIter, - paramType* aResult); -}; - -} // namespace IPC - -#endif // mozilla_HangAnnotations_h diff --git a/xpcom/threads/HangMonitor.cpp b/xpcom/threads/HangMonitor.cpp deleted file mode 100644 index fd3abb19bf38..000000000000 --- a/xpcom/threads/HangMonitor.cpp +++ /dev/null @@ -1,424 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=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/. */ - -#include "mozilla/HangMonitor.h" - -#include "mozilla/Atomics.h" -#include "mozilla/BackgroundHangMonitor.h" -#include "mozilla/Monitor.h" -#include "mozilla/Preferences.h" -#include "mozilla/ProcessedStack.h" -#include "mozilla/Telemetry.h" -#include "mozilla/StaticPtr.h" -#include "mozilla/UniquePtr.h" -#include "nsExceptionHandler.h" -#include "nsReadableUtils.h" -#include "nsThreadUtils.h" -#include "mozilla/StackWalk.h" -#include "nsThreadUtils.h" -#include "nsXULAppAPI.h" -#include "GeckoProfiler.h" - -#ifdef XP_WIN -#include -#endif - -#if defined(MOZ_GECKO_PROFILER) && defined(MOZ_PROFILING) && defined(XP_WIN) - #define REPORT_CHROME_HANGS -#endif - -namespace mozilla { -namespace HangMonitor { - -/** - * A flag which may be set from within a debugger to disable the hang - * monitor. - */ -volatile bool gDebugDisableHangMonitor = false; - -const char kHangMonitorPrefName[] = "hangmonitor.timeout"; - -#ifdef REPORT_CHROME_HANGS -const char kTelemetryPrefName[] = "toolkit.telemetry.enabled"; -#endif - -// Monitor protects gShutdown and gTimeout, but not gTimestamp which rely on -// being atomically set by the processor; synchronization doesn't really matter -// in this use case. -Monitor* gMonitor; - -// The timeout preference, in seconds. -int32_t gTimeout; - -PRThread* gThread; - -// Set when shutdown begins to signal the thread to exit immediately. -bool gShutdown; - -// The timestamp of the last event notification, or PR_INTERVAL_NO_WAIT if -// we're currently not processing events. -Atomic gTimestamp(PR_INTERVAL_NO_WAIT); - -#ifdef REPORT_CHROME_HANGS -// Main thread ID used in reporting chrome hangs under Windows -static HANDLE winMainThreadHandle = nullptr; - -// Default timeout for reporting chrome hangs to Telemetry (5 seconds) -static const int32_t DEFAULT_CHROME_HANG_INTERVAL = 5; - -// Maximum number of PCs to gather from the stack -static const int32_t MAX_CALL_STACK_PCS = 400; -#endif - -// PrefChangedFunc -void -PrefChanged(const char*, void*) -{ - int32_t newval = Preferences::GetInt(kHangMonitorPrefName); -#ifdef REPORT_CHROME_HANGS - // Monitor chrome hangs on the profiling branch if Telemetry enabled - if (newval == 0) { - bool telemetryEnabled = Preferences::GetBool(kTelemetryPrefName); - if (telemetryEnabled) { - newval = DEFAULT_CHROME_HANG_INTERVAL; - } - } -#endif - MonitorAutoLock lock(*gMonitor); - if (newval != gTimeout) { - gTimeout = newval; - lock.Notify(); - } -} - -void -Crash() -{ - if (gDebugDisableHangMonitor) { - return; - } - -#ifdef XP_WIN - if (::IsDebuggerPresent()) { - return; - } -#endif - - // If you change this, you must also deal with the threadsafety of AnnotateCrashReport in - // non-chrome processes! - if (GeckoProcessType_Default == XRE_GetProcessType()) { - CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("Hang"), - NS_LITERAL_CSTRING("1")); - CrashReporter::SetMinidumpAnalysisAllThreads(); - } - - MOZ_CRASH("HangMonitor triggered"); -} - -#ifdef REPORT_CHROME_HANGS - -static void -ChromeStackWalker(uint32_t aFrameNumber, void* aPC, void* aSP, void* aClosure) -{ - MOZ_ASSERT(aClosure); - std::vector* stack = - static_cast*>(aClosure); - if (stack->size() == MAX_CALL_STACK_PCS) { - return; - } - MOZ_ASSERT(stack->size() < MAX_CALL_STACK_PCS); - stack->push_back(reinterpret_cast(aPC)); -} - -static void -GetChromeHangReport(Telemetry::ProcessedStack& aStack, - int32_t& aSystemUptime, - int32_t& aFirefoxUptime) -{ - MOZ_ASSERT(winMainThreadHandle); - - // The thread we're about to suspend might have the alloc lock - // so allocate ahead of time - std::vector rawStack; - rawStack.reserve(MAX_CALL_STACK_PCS); - - DWORD ret = ::SuspendThread(winMainThreadHandle); - bool suspended = false; - if (ret != (DWORD)-1) { - // SuspendThread is asynchronous, so the thread may still be running. Use - // GetThreadContext to ensure it's really suspended. - // See https://blogs.msdn.microsoft.com/oldnewthing/20150205-00/?p=44743. - CONTEXT context; - context.ContextFlags = CONTEXT_CONTROL; - if (::GetThreadContext(winMainThreadHandle, &context)) { - suspended = true; - } - } - - if (!suspended) { - if (ret != (DWORD)-1) { - MOZ_ALWAYS_TRUE(::ResumeThread(winMainThreadHandle) != DWORD(-1)); - } - return; - } - - MozStackWalkThread(ChromeStackWalker, /* skipFrames */ 0, /* maxFrames */ 0, - &rawStack, winMainThreadHandle, nullptr); - ret = ::ResumeThread(winMainThreadHandle); - if (ret == (DWORD)-1) { - return; - } - aStack = Telemetry::GetStackAndModules(rawStack); - - // Record system uptime (in minutes) at the time of the hang - aSystemUptime = ((GetTickCount() / 1000) - (gTimeout * 2)) / 60; - - // Record Firefox uptime (in minutes) at the time of the hang - bool error; - TimeStamp processCreation = TimeStamp::ProcessCreation(&error); - if (!error) { - TimeDuration td = TimeStamp::Now() - processCreation; - aFirefoxUptime = (static_cast(td.ToSeconds()) - (gTimeout * 2)) / 60; - } else { - aFirefoxUptime = -1; - } -} - -#endif - -void -ThreadMain(void*) -{ - AUTO_PROFILER_REGISTER_THREAD("Hang Monitor"); - NS_SetCurrentThreadName("Hang Monitor"); - - MonitorAutoLock lock(*gMonitor); - - // In order to avoid issues with the hang monitor incorrectly triggering - // during a general system stop such as sleeping, the monitor thread must - // run twice to trigger hang protection. - PRIntervalTime lastTimestamp = 0; - int waitCount = 0; - -#ifdef REPORT_CHROME_HANGS - Telemetry::ProcessedStack stack; - int32_t systemUptime = -1; - int32_t firefoxUptime = -1; - HangAnnotations annotations; -#endif - - while (true) { - if (gShutdown) { - return; // Exit the thread - } - - // avoid rereading the volatile value in this loop - PRIntervalTime timestamp = gTimestamp; - - PRIntervalTime now = PR_IntervalNow(); - - if (timestamp != PR_INTERVAL_NO_WAIT && - now < timestamp) { - // 32-bit overflow, reset for another waiting period - timestamp = 1; // lowest legal PRInterval value - } - - if (timestamp != PR_INTERVAL_NO_WAIT && - timestamp == lastTimestamp && - gTimeout > 0) { - ++waitCount; -#ifdef REPORT_CHROME_HANGS - // Capture the chrome-hang stack + Firefox & system uptimes after - // the minimum hang duration has been reached (not when the hang ends) - if (waitCount == 2) { - GetChromeHangReport(stack, systemUptime, firefoxUptime); - annotations = ChromeHangAnnotatorCallout(); - } -#else - // This is the crash-on-hang feature. - // See bug 867313 for the quirk in the waitCount comparison - if (waitCount >= 2) { - int32_t delay = - int32_t(PR_IntervalToSeconds(now - timestamp)); - if (delay >= gTimeout) { - MonitorAutoUnlock unlock(*gMonitor); - Crash(); - } - } -#endif - } else { -#ifdef REPORT_CHROME_HANGS - if (waitCount >= 2) { - uint32_t hangDuration = PR_IntervalToSeconds(now - lastTimestamp); - Telemetry::RecordChromeHang(hangDuration, stack, systemUptime, - firefoxUptime, std::move(annotations)); - stack.Clear(); - } -#endif - lastTimestamp = timestamp; - waitCount = 0; - } - - TimeDuration timeout; - if (gTimeout <= 0) { - timeout = TimeDuration::Forever(); - } else { - timeout = TimeDuration::FromMilliseconds(gTimeout * 500); - } - lock.Wait(timeout); - } -} - -void -Startup() -{ - if (GeckoProcessType_Default != XRE_GetProcessType() && - GeckoProcessType_Content != XRE_GetProcessType()) { - return; - } - - MOZ_ASSERT(!gMonitor, "Hang monitor already initialized"); - gMonitor = new Monitor("HangMonitor"); - - Preferences::RegisterCallback(PrefChanged, kHangMonitorPrefName); - PrefChanged(nullptr, nullptr); - -#ifdef REPORT_CHROME_HANGS - Preferences::RegisterCallback(PrefChanged, kTelemetryPrefName); - winMainThreadHandle = - OpenThread(THREAD_ALL_ACCESS, FALSE, GetCurrentThreadId()); - if (!winMainThreadHandle) { - return; - } -#endif - - // Don't actually start measuring hangs until we hit the main event loop. - // This potentially misses a small class of really early startup hangs, - // but avoids dealing with some xpcshell tests and other situations which - // start XPCOM but don't ever start the event loop. - Suspend(); - - gThread = PR_CreateThread(PR_USER_THREAD, - ThreadMain, - nullptr, PR_PRIORITY_LOW, PR_GLOBAL_THREAD, - PR_JOINABLE_THREAD, 0); -} - -void -Shutdown() -{ - if (GeckoProcessType_Default != XRE_GetProcessType() && - GeckoProcessType_Content != XRE_GetProcessType()) { - return; - } - - MOZ_ASSERT(gMonitor, "Hang monitor not started"); - - { - // Scope the lock we're going to delete later - MonitorAutoLock lock(*gMonitor); - gShutdown = true; - lock.Notify(); - } - - // thread creation could theoretically fail - if (gThread) { - PR_JoinThread(gThread); - gThread = nullptr; - } - - delete gMonitor; - gMonitor = nullptr; -} - -static bool -IsUIMessageWaiting() -{ -#ifndef XP_WIN - return false; -#else - // There should never be mouse, keyboard, or IME messages in a message queue - // in the content process, so don't waste time making multiple PeekMessage - // calls. - if (GeckoProcessType_Content == XRE_GetProcessType()) { - return false; - } -#define NS_WM_IMEFIRST WM_IME_SETCONTEXT -#define NS_WM_IMELAST WM_IME_KEYUP - BOOL haveUIMessageWaiting = FALSE; - MSG msg; - haveUIMessageWaiting |= ::PeekMessageW(&msg, nullptr, WM_KEYFIRST, - WM_IME_KEYLAST, PM_NOREMOVE); - haveUIMessageWaiting |= ::PeekMessageW(&msg, nullptr, NS_WM_IMEFIRST, - NS_WM_IMELAST, PM_NOREMOVE); - haveUIMessageWaiting |= ::PeekMessageW(&msg, nullptr, WM_MOUSEFIRST, - WM_MOUSELAST, PM_NOREMOVE); - return haveUIMessageWaiting; -#endif -} - -void -NotifyActivity(ActivityType aActivityType) -{ - MOZ_ASSERT(NS_IsMainThread(), - "HangMonitor::Notify called from off the main thread."); - - // Determine the activity type more specifically - if (aActivityType == kGeneralActivity) { - aActivityType = IsUIMessageWaiting() ? kActivityUIAVail : - kActivityNoUIAVail; - } - - // Calculate the cumulative amount of lag time since the last UI message - static uint32_t cumulativeUILagMS = 0; - switch (aActivityType) { - case kActivityNoUIAVail: - cumulativeUILagMS = 0; - break; - case kActivityUIAVail: - case kUIActivity: - if (gTimestamp != PR_INTERVAL_NO_WAIT) { - cumulativeUILagMS += PR_IntervalToMilliseconds(PR_IntervalNow() - - gTimestamp); - } - break; - default: - break; - } - - // This is not a locked activity because PRTimeStamp is a 32-bit quantity - // which can be read/written atomically, and we don't want to pay locking - // penalties here. - gTimestamp = PR_IntervalNow(); - - // If we have UI activity we should reset the timer and report it - if (aActivityType == kUIActivity) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::EVENTLOOP_UI_ACTIVITY_EXP_MS, - cumulativeUILagMS); - cumulativeUILagMS = 0; - } - - if (gThread && !gShutdown) { - mozilla::BackgroundHangMonitor().NotifyActivity(); - } -} - -void -Suspend() -{ - MOZ_ASSERT(NS_IsMainThread(), - "HangMonitor::Suspend called from off the main thread."); - - // Because gTimestamp changes this resets the wait count. - gTimestamp = PR_INTERVAL_NO_WAIT; - - if (gThread && !gShutdown) { - mozilla::BackgroundHangMonitor().NotifyWait(); - } -} - -} // namespace HangMonitor -} // namespace mozilla diff --git a/xpcom/threads/HangMonitor.h b/xpcom/threads/HangMonitor.h deleted file mode 100644 index fd0e6ff8390c..000000000000 --- a/xpcom/threads/HangMonitor.h +++ /dev/null @@ -1,58 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=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/. */ - -#ifndef mozilla_HangMonitor_h -#define mozilla_HangMonitor_h - -namespace mozilla { -namespace HangMonitor { - -/** - * Signifies the type of activity in question -*/ -enum ActivityType -{ - /* There is activity and it is known to be UI related activity. */ - kUIActivity, - - /* There is non UI activity and no UI activity is pending */ - kActivityNoUIAVail, - - /* There is non UI activity and UI activity is known to be pending */ - kActivityUIAVail, - - /* There is non UI activity and UI activity pending is unknown */ - kGeneralActivity -}; - -/** - * Start monitoring hangs. Should be called by the XPCOM startup process only. - */ -void Startup(); - -/** - * Stop monitoring hangs and join the thread. - */ -void Shutdown(); - -/** - * Notify the hang monitor of activity which will reset its internal timer. - * - * @param activityType The type of activity being reported. - * @see ActivityType - */ -void NotifyActivity(ActivityType activityType = kGeneralActivity); - -/* - * Notify the hang monitor that the browser is now idle and no detection should - * be done. - */ -void Suspend(); - -} // namespace HangMonitor -} // namespace mozilla - -#endif // mozilla_HangMonitor_h diff --git a/xpcom/threads/moz.build b/xpcom/threads/moz.build index f7abd87fc849..6743ce779c23 100644 --- a/xpcom/threads/moz.build +++ b/xpcom/threads/moz.build @@ -44,8 +44,6 @@ EXPORTS.mozilla += [ 'CPUUsageWatcher.h', 'DeadlockDetector.h', 'EventQueue.h', - 'HangAnnotations.h', - 'HangMonitor.h', 'IdleTaskRunner.h', 'LazyIdleThread.h', 'MainThreadIdlePeriod.h', @@ -82,8 +80,6 @@ UNIFIED_SOURCES += [ 'CooperativeThreadPool.cpp', 'CPUUsageWatcher.cpp', 'EventQueue.cpp', - 'HangAnnotations.cpp', - 'HangMonitor.cpp', 'InputEventStatistics.cpp', 'LabeledEventQueue.cpp', 'LazyIdleThread.cpp', diff --git a/xpcom/threads/nsThread.cpp b/xpcom/threads/nsThread.cpp index c6ca5069acf2..192e9832f3ec 100644 --- a/xpcom/threads/nsThread.cpp +++ b/xpcom/threads/nsThread.cpp @@ -21,10 +21,10 @@ #include "nsCOMPtr.h" #include "nsQueryObject.h" #include "pratom.h" +#include "mozilla/BackgroundHangMonitor.h" #include "mozilla/CycleCollectedJSContext.h" #include "mozilla/Logging.h" #include "nsIObserverService.h" -#include "mozilla/HangMonitor.h" #include "mozilla/IOInterposer.h" #include "mozilla/ipc/MessageChannel.h" #include "mozilla/ipc/BackgroundChild.h" @@ -1001,7 +1001,7 @@ nsThread::ProcessNextEvent(bool aMayWait, bool* aResult) LOG(("THRD(%p) running [%p]\n", this, event.get())); if (MAIN_THREAD == mIsMainThread) { - HangMonitor::NotifyActivity(); + BackgroundHangMonitor().NotifyActivity(); } bool schedulerLoggingEnabled = GetSchedulerLoggingEnabled(); @@ -1256,7 +1256,7 @@ nsThread::DoMainThreadSpecificProcessing(bool aReallyWait) ipc::CancelCPOWs(); if (aReallyWait) { - HangMonitor::Suspend(); + BackgroundHangMonitor().NotifyWait(); } // Fire a memory pressure notification, if one is pending. From 20d9cf4517dbc1a9c4985706210c5eb5ddeaa06a Mon Sep 17 00:00:00 2001 From: layely Date: Sun, 3 Jun 2018 05:04:48 +0000 Subject: [PATCH 23/51] Bug 1458018 - Add style for multiselected tabs. r=jaws MozReview-Commit-ID: Ead7pIfHBJP --HG-- extra : rebase_source : 72d469d6d4ab12e5179a4acd279f84b7ba0993b7 --- browser/base/content/tabbrowser.css | 4 -- browser/base/content/tabbrowser.js | 15 ++++-- browser/base/content/tabbrowser.xml | 27 ++++++++-- browser/base/content/test/tabs/browser.ini | 1 + ...owser_multiselect_tabs_positional_attrs.js | 54 +++++++++++++++++++ browser/themes/shared/tabs.inc.css | 19 +++++-- browser/themes/windows/compacttheme.css | 7 +-- 7 files changed, 111 insertions(+), 16 deletions(-) create mode 100644 browser/base/content/test/tabs/browser_multiselect_tabs_positional_attrs.js diff --git a/browser/base/content/tabbrowser.css b/browser/base/content/tabbrowser.css index 69d71543978f..e6ca476899a9 100644 --- a/browser/base/content/tabbrowser.css +++ b/browser/base/content/tabbrowser.css @@ -36,10 +36,6 @@ white-space: nowrap; } -.tab-label[multiselected] { - font-weight: bold; -} - .tab-label-container { overflow: hidden; } diff --git a/browser/base/content/tabbrowser.js b/browser/base/content/tabbrowser.js index 671741c2947b..bf744c36073f 100644 --- a/browser/base/content/tabbrowser.js +++ b/browser/base/content/tabbrowser.js @@ -3624,13 +3624,17 @@ window._gBrowser = { return SessionStore.duplicateTab(window, aTab, 0, aRestoreTabImmediately); }, - addToMultiSelectedTabs(aTab) { + addToMultiSelectedTabs(aTab, skipPositionalAttributes) { if (aTab.multiselected) { return; } aTab.setAttribute("multiselected", "true"); this._multiSelectedTabsSet.add(aTab); + + if (!skipPositionalAttributes) { + this.tabContainer._setPositionalAttributes(); + } }, /** @@ -3652,8 +3656,9 @@ window._gBrowser = { [indexOfTab1, indexOfTab2] : [indexOfTab2, indexOfTab1]; for (let i = lowerIndex; i <= higherIndex; i++) { - this.addToMultiSelectedTabs(tabs[i]); + this.addToMultiSelectedTabs(tabs[i], true); } + this.tabContainer._setPositionalAttributes(); }, removeFromMultiSelectedTabs(aTab) { @@ -3661,10 +3666,11 @@ window._gBrowser = { return; } aTab.removeAttribute("multiselected"); + this.tabContainer._setPositionalAttributes(); this._multiSelectedTabsSet.delete(aTab); }, - clearMultiSelectedTabs() { + clearMultiSelectedTabs(updatePositionalAttributes) { const selectedTabs = ChromeUtils.nondeterministicGetWeakSetKeys(this._multiSelectedTabsSet); for (let tab of selectedTabs) { if (tab.isConnected && tab.multiselected) { @@ -3672,6 +3678,9 @@ window._gBrowser = { } } this._multiSelectedTabsSet = new WeakSet(); + if (updatePositionalAttributes) { + this.tabContainer._setPositionalAttributes(); + } }, get multiSelectedTabsCount() { diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml index 748b75c405fe..7367f2169c1c 100644 --- a/browser/base/content/tabbrowser.xml +++ b/browser/base/content/tabbrowser.xml @@ -297,6 +297,17 @@ if (hoveredTab) { hoveredTab._mouseenter(); } + + // Update before-multiselected attributes. + // gBrowser may not be initialized yet, so avoid using it + for (let i = 0; i < visibleTabs.length - 1; i++) { + let tab = visibleTabs[i]; + let nextTab = visibleTabs[i + 1]; + tab.removeAttribute("before-multiselected"); + if (nextTab.multiselected) { + tab.setAttribute("before-multiselected", "true"); + } + } ]]> @@ -1545,7 +1556,7 @@ - @@ -1582,7 +1593,7 @@ onunderflow="this.removeAttribute('textoverflow');" flex="1"> + + + return this.getAttribute("before-multiselected") == "true"; + +