From 57d1edb3ac3bc896492e894dda314b29859121de Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Mon, 11 Sep 2017 21:40:36 +0200 Subject: [PATCH 01/42] Bug 1393044 - Remove a warning message in nsGlobalWindow::GetParentInternal(), r=erahm --- dom/base/nsGlobalWindow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index e843cb6f3a77..b3dac08fa942 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -12764,7 +12764,7 @@ nsGlobalWindow::GetParentInternal() if (IsInnerWindow()) { nsGlobalWindow* outer = GetOuterWindowInternal(); if (!outer) { - NS_WARNING("No outer window available!"); + // No outer window available! return nullptr; } return outer->GetParentInternal(); From f95681c50f20d7f57636bf5fb2360ce8d37d8147 Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Mon, 11 Sep 2017 13:47:38 -0600 Subject: [PATCH 02/42] Bug 1398907: Handler path should be written using length of null-terminated string in bytes, not size of the buffer; r=eeejay MozReview-Commit-ID: 64Zv3obsQie --- ipc/mscom/oop/Handler.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ipc/mscom/oop/Handler.cpp b/ipc/mscom/oop/Handler.cpp index 0081d6432686..0753acc48ce3 100644 --- a/ipc/mscom/oop/Handler.cpp +++ b/ipc/mscom/oop/Handler.cpp @@ -355,9 +355,12 @@ Handler::Register(REFCLSID aClsid) return HRESULT_FROM_WIN32(lastError); } + // The result of GetModuleFileName excludes the null terminator + DWORD valueSizeWithNullInBytes = (size + 1) * sizeof(wchar_t); + result = RegSetValueEx(inprocHandlerKey, L"", 0, REG_EXPAND_SZ, reinterpret_cast(absLibPath), - sizeof(absLibPath)); + valueSizeWithNullInBytes); if (result != ERROR_SUCCESS) { Unregister(aClsid); return HRESULT_FROM_WIN32(result); From 98e80300bbee46a050ed1dd976b0ec46fe23355c Mon Sep 17 00:00:00 2001 From: Michal Novotny Date: Mon, 11 Sep 2017 22:16:13 +0200 Subject: [PATCH 03/42] Bug 1398847 - Enabling RCWN causes tp6_facebook regression, r=valentin For some reason, triggering network directly from MaybeRaceCacheWithNetwork() causes performance regression of tp6_facebook tests. This patch changes it so that an event is posted instead. The patch also adds network.http.rcwn.min_wait_before_racing_ms preference which can be used by users to avoid immediate racing. --- modules/libpref/init/all.js | 1 + netwerk/protocol/http/nsHttpChannel.cpp | 102 +++++++++++++++--------- netwerk/protocol/http/nsHttpChannel.h | 3 +- 3 files changed, 69 insertions(+), 37 deletions(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 5cdb1c6d0e2f..b0e01b2a253a 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1835,6 +1835,7 @@ pref("network.http.rcwn.cache_queue_priority_threshold", 2); // is smaller than this size. pref("network.http.rcwn.small_resource_size_kb", 256); +pref("network.http.rcwn.min_wait_before_racing_ms", 0); pref("network.http.rcwn.max_wait_before_racing_ms", 500); // The ratio of the transaction count for the focused window and the count of diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index f430769d643d..35a0783ae4c7 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -128,6 +128,7 @@ static bool sRCWNEnabled = false; static uint32_t sRCWNQueueSizeNormal = 50; static uint32_t sRCWNQueueSizePriority = 10; static uint32_t sRCWNSmallResourceSizeKB = 256; +static uint32_t sRCWNMinWaitMs = 0; static uint32_t sRCWNMaxWaitMs = 500; // True if the local cache should be bypassed when processing a request. @@ -625,7 +626,7 @@ nsHttpChannel::ConnectOnTailUnblock() Unused << ReadFromCache(true); } - return TriggerNetwork(0); + return TriggerNetwork(); } nsresult @@ -4513,7 +4514,7 @@ nsHttpChannel::OnCacheEntryAvailableInternal(nsICacheEntry *entry, Unused << ReadFromCache(true); } - return TriggerNetwork(0); + return TriggerNetwork(); } nsresult @@ -6134,6 +6135,7 @@ nsHttpChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *context) Preferences::AddUintVarCache(&sRCWNQueueSizeNormal, "network.http.rcwn.cache_queue_normal_threshold"); Preferences::AddUintVarCache(&sRCWNQueueSizePriority, "network.http.rcwn.cache_queue_priority_threshold"); Preferences::AddUintVarCache(&sRCWNSmallResourceSizeKB, "network.http.rcwn.small_resource_size_kb"); + Preferences::AddUintVarCache(&sRCWNMinWaitMs, "network.http.rcwn.min_wait_before_racing_ms"); Preferences::AddUintVarCache(&sRCWNMaxWaitMs, "network.http.rcwn.max_wait_before_racing_ms"); } @@ -9302,11 +9304,12 @@ nsHttpChannel::Test_triggerDelayedOpenCacheEntry() } nsresult -nsHttpChannel::TriggerNetwork(int32_t aTimeout) +nsHttpChannel::TriggerNetworkWithDelay(uint32_t aDelay) { MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread"); - LOG(("nsHttpChannel::TriggerNetwork [this=%p]\n", this)); + LOG(("nsHttpChannel::TriggerNetworkWithDelay [this=%p, delay=%u]\n", + this, aDelay)); if (mCanceled) { LOG((" channel was canceled.\n")); @@ -9320,36 +9323,64 @@ nsHttpChannel::TriggerNetwork(int32_t aTimeout) return NS_OK; } - if (!aTimeout) { - mNetworkTriggered = true; - if (mNetworkTriggerTimer) { - mNetworkTriggerTimer->Cancel(); - mNetworkTriggerTimer = nullptr; - } + if (!aDelay) { + // We cannot call TriggerNetwork() directly here, because it would + // cause performance regression in tp6 tests, see bug 1398847. + return NS_DispatchToMainThread( + NewRunnableMethod("net::nsHttpChannel::TriggerNetworkWithDelay", + this, &nsHttpChannel::TriggerNetwork), + NS_DISPATCH_NORMAL); + } - // If we are waiting for a proxy request, that means we can't trigger - // the next step just yet. We need for mConnectionInfo to be non-null - // before we call TryHSTSPriming. OnProxyAvailable will trigger - // BeginConnect, and Connect will call TryHSTSPriming even if it's - // for the cache callbacks. - if (mProxyRequest) { - LOG((" proxy request in progress. Delaying network trigger.\n")); - mWaitingForProxy = true; - return NS_OK; - } + if (!mNetworkTriggerTimer) { + mNetworkTriggerTimer = do_CreateInstance(NS_TIMER_CONTRACTID); + } + mNetworkTriggerTimer->InitWithCallback(this, aDelay, nsITimer::TYPE_ONE_SHOT); + return NS_OK; +} - if (mCacheAsyncOpenCalled && !mOnCacheAvailableCalled) { - mRaceCacheWithNetwork = true; - } +nsresult +nsHttpChannel::TriggerNetwork() +{ + MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread"); - LOG((" triggering network\n")); - return TryHSTSPriming(); + LOG(("nsHttpChannel::TriggerNetwork [this=%p]\n", this)); + + if (mCanceled) { + LOG((" channel was canceled.\n")); + return mStatus; } - LOG((" setting timer to trigger network: %d ms\n", aTimeout)); - mNetworkTriggerTimer = do_CreateInstance(NS_TIMER_CONTRACTID); - mNetworkTriggerTimer->InitWithCallback(this, aTimeout, nsITimer::TYPE_ONE_SHOT); - return NS_OK; + // If a network request has already gone out, there is no point in + // doing this again. + if (mNetworkTriggered) { + LOG((" network already triggered. Returning.\n")); + return NS_OK; + } + + mNetworkTriggered = true; + if (mNetworkTriggerTimer) { + mNetworkTriggerTimer->Cancel(); + mNetworkTriggerTimer = nullptr; + } + + // If we are waiting for a proxy request, that means we can't trigger + // the next step just yet. We need for mConnectionInfo to be non-null + // before we call TryHSTSPriming. OnProxyAvailable will trigger + // BeginConnect, and Connect will call TryHSTSPriming even if it's + // for the cache callbacks. + if (mProxyRequest) { + LOG((" proxy request in progress. Delaying network trigger.\n")); + mWaitingForProxy = true; + return NS_OK; + } + + if (mCacheAsyncOpenCalled && !mOnCacheAvailableCalled) { + mRaceCacheWithNetwork = true; + } + + LOG((" triggering network\n")); + return TryHSTSPriming(); } nsresult @@ -9380,23 +9411,22 @@ nsHttpChannel::MaybeRaceCacheWithNetwork() // We use microseconds in CachePerfStats but we need milliseconds // for TriggerNetwork. mRaceDelay /= 1000; - if (mRaceDelay > sRCWNMaxWaitMs) { - mRaceDelay = sRCWNMaxWaitMs; - } } - MOZ_ASSERT(sRCWNEnabled, "The pref must be truned on."); + mRaceDelay = clamped(mRaceDelay, sRCWNMinWaitMs, sRCWNMaxWaitMs); + + MOZ_ASSERT(sRCWNEnabled, "The pref must be turned on."); LOG(("nsHttpChannel::MaybeRaceCacheWithNetwork [this=%p, delay=%u]\n", this, mRaceDelay)); - return TriggerNetwork(mRaceDelay); + return TriggerNetworkWithDelay(mRaceDelay); } NS_IMETHODIMP nsHttpChannel::Test_triggerNetwork(int32_t aTimeout) { MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread"); - return TriggerNetwork(aTimeout); + return TriggerNetworkWithDelay(aTimeout); } NS_IMETHODIMP @@ -9406,7 +9436,7 @@ nsHttpChannel::Notify(nsITimer *aTimer) if (aTimer == mCacheOpenTimer) { return Test_triggerDelayedOpenCacheEntry(); } else if (aTimer == mNetworkTriggerTimer) { - return TriggerNetwork(0); + return TriggerNetwork(); } else { MOZ_CRASH("Unknown timer"); } diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h index 2059044a0ca8..ab2d28dc42ae 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -722,7 +722,8 @@ class nsHttpChannel final : public HttpBaseChannel // with the cache fetch, and proceeds to do so. nsresult MaybeRaceCacheWithNetwork(); - nsresult TriggerNetwork(int32_t aTimeout); + nsresult TriggerNetworkWithDelay(uint32_t aDelay); + nsresult TriggerNetwork(); void CancelNetworkRequest(nsresult aStatus); // Timer used to delay the network request, or to trigger the network // request if retrieving the cache entry takes too long. From 5ecd08ebb6d61522cd5f4ed61faa3ebaba22974c Mon Sep 17 00:00:00 2001 From: Kannan Vijayan Date: Mon, 11 Sep 2017 16:21:03 -0400 Subject: [PATCH 04/42] Bug 1376717 - Do not crash on failed profiler table lookups for jitcode during report generation. r=mstange --- js/src/jit/JitcodeMap.cpp | 9 ++++++--- js/src/jit/JitcodeMap.h | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/js/src/jit/JitcodeMap.cpp b/js/src/jit/JitcodeMap.cpp index aeeace13a9ac..af1d6c5271fd 100644 --- a/js/src/jit/JitcodeMap.cpp +++ b/js/src/jit/JitcodeMap.cpp @@ -1658,14 +1658,17 @@ JS_PUBLIC_API(void) JS::ForEachProfiledFrame(JSContext* cx, void* addr, ForEachProfiledFrameOp& op) { js::jit::JitcodeGlobalTable* table = cx->runtime()->jitRuntime()->getJitcodeGlobalTable(); - js::jit::JitcodeGlobalEntry& entry = table->lookupInfallible(addr); + js::jit::JitcodeGlobalEntry* entry = table->lookup(addr); + + if (!entry) + return; // Extract the stack for the entry. Assume maximum inlining depth is <64 const char* labels[64]; - uint32_t depth = entry.callStackAtAddr(cx->runtime(), addr, labels, 64); + uint32_t depth = entry->callStackAtAddr(cx->runtime(), addr, labels, 64); MOZ_ASSERT(depth < 64); for (uint32_t i = depth; i != 0; i--) { - JS::ForEachProfiledFrameOp::FrameHandle handle(cx->runtime(), entry, addr, labels[i - 1], i - 1); + JS::ForEachProfiledFrameOp::FrameHandle handle(cx->runtime(), *entry, addr, labels[i - 1], i - 1); op(handle); } } diff --git a/js/src/jit/JitcodeMap.h b/js/src/jit/JitcodeMap.h index 66d66975bb79..7f277444bae7 100644 --- a/js/src/jit/JitcodeMap.h +++ b/js/src/jit/JitcodeMap.h @@ -1042,7 +1042,7 @@ class JitcodeGlobalTable return skiplistSize_ == 0; } - const JitcodeGlobalEntry* lookup(void* ptr) { + JitcodeGlobalEntry* lookup(void* ptr) { return lookupInternal(ptr); } From 96f297c09231167b03763f40cdb87ce264f0f04a Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Mon, 11 Sep 2017 14:35:01 -0600 Subject: [PATCH 05/42] Bug 1398922: Add AccessibleMarshal.dll registration to updater; r=mhowell MozReview-Commit-ID: 1xmE4oi2q2u --- browser/installer/windows/nsis/shared.nsh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/browser/installer/windows/nsis/shared.nsh b/browser/installer/windows/nsis/shared.nsh index e7ef009859c5..6c8069be0324 100755 --- a/browser/installer/windows/nsis/shared.nsh +++ b/browser/installer/windows/nsis/shared.nsh @@ -113,9 +113,12 @@ RmDir /r /REBOOTOK "$INSTDIR\${TO_BE_DELETED}" - ; Register AccessibleHandler.dll with COM (this writes to HKLM) + ; Register AccessibleHandler.dll with COM (this requires write access to HKLM) ${RegisterAccessibleHandler} + ; Register AccessibleMarshal.dll with COM (this requires write access to HKLM) + ${RegisterAccessibleMarshal} + !ifdef MOZ_MAINTENANCE_SERVICE Call IsUserAdmin Pop $R0 @@ -1014,6 +1017,11 @@ !macroend !define RegisterAccessibleHandler "!insertmacro RegisterAccessibleHandler" +!macro RegisterAccessibleMarshal + ${RegisterDLL} "$INSTDIR\AccessibleMarshal.dll" +!macroend +!define RegisterAccessibleMarshal "!insertmacro RegisterAccessibleMarshal" + ; Removes various registry entries for reasons noted below (does not use SHCTX). !macro RemoveDeprecatedKeys StrCpy $0 "SOFTWARE\Classes" From 51e210b8a83a4ce0ff08e74cff304267bb3c7b73 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Mon, 11 Sep 2017 23:57:22 +0300 Subject: [PATCH 06/42] Bug 1398896 - Check the cycle collection budget more often, r=mccr8 --- xpcom/base/nsCycleCollector.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp index fafaca94ef81..41946bc9dd64 100644 --- a/xpcom/base/nsCycleCollector.cpp +++ b/xpcom/base/nsCycleCollector.cpp @@ -2290,7 +2290,7 @@ CCGraphBuilder::DoneAddingRoots() MOZ_NEVER_INLINE bool CCGraphBuilder::BuildGraph(SliceBudget& aBudget) { - const intptr_t kNumNodesBetweenTimeChecks = 1000; + const intptr_t kNumNodesBetweenTimeChecks = 500; const intptr_t kStep = SliceBudget::CounterReset / kNumNodesBetweenTimeChecks; MOZ_ASSERT(mCurrNode); From 317f8d883bd39423991a41361bd8691f4bdd6511 Mon Sep 17 00:00:00 2001 From: Michal Novotny Date: Mon, 11 Sep 2017 23:31:58 +0200 Subject: [PATCH 07/42] Bug 1398941 - test_race_cache_with_network.js test is failing after landing bug 1398847, r=KWierso, CLOSED TREE --- netwerk/test/unit/xpcshell.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/netwerk/test/unit/xpcshell.ini b/netwerk/test/unit/xpcshell.ini index 2a95f12e4ce8..23dc06f61cea 100644 --- a/netwerk/test/unit/xpcshell.ini +++ b/netwerk/test/unit/xpcshell.ini @@ -390,6 +390,8 @@ skip-if = os == "android" [test_rusturl.js] [test_trackingProtection_annotateChannels.js] [test_race_cache_with_network.js] +# temporarily disabled because it is failing after landing bug 1398847 +skip-of = true [test_channel_priority.js] [test_bug1312774_http1.js] [test_1351443-missing-NewChannel2.js] From d2bf7b4be9c3071c1b4936c465d1aaae57dc825a Mon Sep 17 00:00:00 2001 From: Michal Novotny Date: Mon, 11 Sep 2017 23:37:36 +0200 Subject: [PATCH 08/42] Bug 1398941 - test_race_cache_with_network.js test is failing after landing bug 1398847, r=KWierso, CLOSED TREE --- netwerk/test/unit/xpcshell.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netwerk/test/unit/xpcshell.ini b/netwerk/test/unit/xpcshell.ini index 23dc06f61cea..86df466dfe2d 100644 --- a/netwerk/test/unit/xpcshell.ini +++ b/netwerk/test/unit/xpcshell.ini @@ -391,7 +391,7 @@ skip-if = os == "android" [test_trackingProtection_annotateChannels.js] [test_race_cache_with_network.js] # temporarily disabled because it is failing after landing bug 1398847 -skip-of = true +skip-if = true [test_channel_priority.js] [test_bug1312774_http1.js] [test_1351443-missing-NewChannel2.js] From f032533d96e4a501f9692ced4ccbabb1b175affb Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Fri, 8 Sep 2017 13:44:32 -0700 Subject: [PATCH 09/42] Bug 1396366: Make sure the URLPreloader cache is only written once. r=erahm MozReview-Commit-ID: FA1BPQ5c6nP --- js/xpconnect/loader/ScriptPreloader.cpp | 7 ++++--- js/xpconnect/loader/URLPreloader.cpp | 20 ++++++++++++++++++++ js/xpconnect/loader/URLPreloader.h | 3 +++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/js/xpconnect/loader/ScriptPreloader.cpp b/js/xpconnect/loader/ScriptPreloader.cpp index 1a0879d8ff63..94464798de94 100644 --- a/js/xpconnect/loader/ScriptPreloader.cpp +++ b/js/xpconnect/loader/ScriptPreloader.cpp @@ -601,8 +601,6 @@ ScriptPreloader::WriteCache() { MOZ_ASSERT(!NS_IsMainThread()); - Unused << URLPreloader::GetSingleton().WriteCache(); - if (!mDataPrepared && !mSaveComplete) { MOZ_ASSERT(!mBlockedOnSyncDispatch); mBlockedOnSyncDispatch = true; @@ -695,7 +693,10 @@ ScriptPreloader::Run() mal.Wait(10000); } - auto result = WriteCache(); + auto result = URLPreloader::GetSingleton().WriteCache(); + Unused << NS_WARN_IF(result.isErr()); + + result = WriteCache(); Unused << NS_WARN_IF(result.isErr()); result = mChildCache->WriteCache(); diff --git a/js/xpconnect/loader/URLPreloader.cpp b/js/xpconnect/loader/URLPreloader.cpp index 10dba23b1b6b..e7ddaeae216a 100644 --- a/js/xpconnect/loader/URLPreloader.cpp +++ b/js/xpconnect/loader/URLPreloader.cpp @@ -202,6 +202,20 @@ URLPreloader::WriteCache() { MOZ_ASSERT(!NS_IsMainThread()); + // The script preloader might call us a second time, if it has to re-write + // its cache after a cache flush. We don't care about cache flushes, since + // our cache doesn't store any file data, only paths. And we currently clear + // our cached file list after the first write, which means that a second + // write would (aside from breaking the invariant that we never touch + // mCachedURLs off-main-thread after the first write, and trigger a data + // race) mean we get no pre-loading on the next startup. + if (mCacheWritten) { + return Ok(); + } + mCacheWritten = true; + + LOG(Debug, "Writing cache..."); + nsCOMPtr cacheFile; MOZ_TRY_VAR(cacheFile, GetCacheFile(NS_LITERAL_STRING("-new.bin"))); @@ -256,6 +270,8 @@ URLPreloader::Cleanup() Result URLPreloader::ReadCache(LinkedList& pendingURLs) { + LOG(Debug, "Reading cache..."); + nsCOMPtr cacheFile; MOZ_TRY_VAR(cacheFile, FindCacheFile()); @@ -301,6 +317,8 @@ URLPreloader::ReadCache(LinkedList& pendingURLs) while (!buf.finished()) { CacheKey key(buf); + LOG(Debug, "Cached file: %s %s", key.TypeString(), key.mPath.get()); + auto entry = mCachedURLs.LookupOrAdd(key, key); entry->mResultCode = NS_ERROR_NOT_INITIALIZED; @@ -385,6 +403,8 @@ URLPreloader::BackgroundReadFiles() nsresult rv = NS_OK; + LOG(Debug, "Background reading %s file %s", entry->TypeString(), entry->mPath.get()); + if (entry->mType == entry->TypeFile) { auto result = entry->Read(); if (result.isErr()) { diff --git a/js/xpconnect/loader/URLPreloader.h b/js/xpconnect/loader/URLPreloader.h index 0dfef7284739..db5a1d7ed379 100644 --- a/js/xpconnect/loader/URLPreloader.h +++ b/js/xpconnect/loader/URLPreloader.h @@ -303,6 +303,9 @@ class URLPreloader final : public nsIObserver bool mStartupFinished = false; bool mReaderInitialized = false; + // Only to be accessed from the cache write thread. + bool mCacheWritten = false; + // The prefix URLs for files in the GRE and App omni jar archives. nsCString mGREPrefix; nsCString mAppPrefix; From ba3421dadf81346b4edeef6a6a4941c6e03446ef Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 10 Sep 2017 13:14:12 -0700 Subject: [PATCH 10/42] Bug 1391707: Part 0 - Remove Task.jsm support from DeferredTask. r=florian MozReview-Commit-ID: LEbrPt0uae0 --- toolkit/modules/DeferredTask.jsm | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/toolkit/modules/DeferredTask.jsm b/toolkit/modules/DeferredTask.jsm index c160833923fc..2d75b45ab96c 100644 --- a/toolkit/modules/DeferredTask.jsm +++ b/toolkit/modules/DeferredTask.jsm @@ -90,8 +90,6 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils", "resource://gre/modules/PromiseUtils.jsm"); -XPCOMUtils.defineLazyModuleGetter(this, "Task", - "resource://gre/modules/Task.jsm"); const Timer = Components.Constructor("@mozilla.org/timer;1", "nsITimer", "initWithCallback"); @@ -102,8 +100,8 @@ const Timer = Components.Constructor("@mozilla.org/timer;1", "nsITimer", * Sets up a task whose execution can be triggered after a delay. * * @param aTaskFn - * Function or generator function to execute. This argument is passed to - * the "Task.spawn" method every time the task should be executed. This + * Function to execute. If the function returns a promise, the task is + * not considered complete until that promise resolves. This * task is never re-entered while running. * @param aDelayMs * Time between executions, in milliseconds. Multiple attempts to run @@ -115,6 +113,10 @@ const Timer = Components.Constructor("@mozilla.org/timer;1", "nsITimer", this.DeferredTask = function(aTaskFn, aDelayMs) { this._taskFn = aTaskFn; this._delayMs = aDelayMs; + + if (aTaskFn.isGenerator()) { + Cu.reportError(new Error("Unexpected generator function passed to DeferredTask")); + } } this.DeferredTask.prototype = { @@ -301,12 +303,7 @@ this.DeferredTask.prototype = { */ async _runTask() { try { - let result = this._taskFn(); - if (Object.prototype.toString.call(result) == "[object Generator]") { - await Task.spawn(result); // eslint-disable-line mozilla/no-task - } else { - await result; - } + await this._taskFn(); } catch (ex) { Cu.reportError(ex); } From 7d330872bb828bbd5f2c026cb00f530082cd5338 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Fri, 1 Sep 2017 16:39:14 -0700 Subject: [PATCH 11/42] Bug 1391707: Part 1 - Use idle dispatch in DeferredTask. r=florian MozReview-Commit-ID: Ktlu71aIcRZ --- devtools/server/performance/profiler.js | 2 +- toolkit/modules/DeferredTask.jsm | 17 +++++++++++++--- toolkit/modules/JSONFile.jsm | 1 - toolkit/modules/PromiseUtils.jsm | 27 +++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/devtools/server/performance/profiler.js b/devtools/server/performance/profiler.js index ccdc9b7ca7c5..5ca630813686 100644 --- a/devtools/server/performance/profiler.js +++ b/devtools/server/performance/profiler.js @@ -387,7 +387,7 @@ const ProfilerManager = (function () { if (this._profilerStatusSubscribers > 0 && nsIProfilerModule.IsActive()) { if (!this._poller) { this._poller = new DeferredTask(this._emitProfilerStatus.bind(this), - this._profilerStatusInterval); + this._profilerStatusInterval, 0); } this._poller.arm(); } else if (this._poller) { diff --git a/toolkit/modules/DeferredTask.jsm b/toolkit/modules/DeferredTask.jsm index 2d75b45ab96c..ca49dafe3765 100644 --- a/toolkit/modules/DeferredTask.jsm +++ b/toolkit/modules/DeferredTask.jsm @@ -109,10 +109,15 @@ const Timer = Components.Constructor("@mozilla.org/timer;1", "nsITimer", * inactivity is guaranteed to pass between multiple executions of the * task, except on finalization, when the task may restart immediately * after the previous execution finished. + * @param aIdleTimeoutMs + * The maximum time to wait for an idle slot on the main thread after + * aDelayMs have elapsed. If omitted, waits indefinitely for an idle + * callback. */ -this.DeferredTask = function(aTaskFn, aDelayMs) { +this.DeferredTask = function(aTaskFn, aDelayMs, aIdleTimeoutMs) { this._taskFn = aTaskFn; this._delayMs = aDelayMs; + this._timeoutMs = aIdleTimeoutMs; if (aTaskFn.isGenerator()) { Cu.reportError(new Error("Unexpected generator function passed to DeferredTask")); @@ -299,11 +304,17 @@ this.DeferredTask.prototype = { }, /** - * Executes the associated task and catches exceptions. + * Executes the associated task in an idle callback and catches exceptions. */ async _runTask() { try { - await this._taskFn(); + // If we're being finalized, execute the task immediately, so we don't + // risk blocking async shutdown longer than necessary. + if (this._finalized || this._timeoutMs === 0) { + await this._taskFn(); + } else { + await PromiseUtils.idleDispatch(this._taskFn, this._timeoutMs); + } } catch (ex) { Cu.reportError(ex); } diff --git a/toolkit/modules/JSONFile.jsm b/toolkit/modules/JSONFile.jsm index be97ac04ffc3..4ab995b5d691 100644 --- a/toolkit/modules/JSONFile.jsm +++ b/toolkit/modules/JSONFile.jsm @@ -37,7 +37,6 @@ this.EXPORTED_SYMBOLS = [ const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components; Cu.import("resource://gre/modules/XPCOMUtils.jsm"); -Cu.import("resource://gre/modules/Services.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown", "resource://gre/modules/AsyncShutdown.jsm"); diff --git a/toolkit/modules/PromiseUtils.jsm b/toolkit/modules/PromiseUtils.jsm index dbc51a3d9f77..f790a9495bbb 100644 --- a/toolkit/modules/PromiseUtils.jsm +++ b/toolkit/modules/PromiseUtils.jsm @@ -6,6 +6,7 @@ this.EXPORTED_SYMBOLS = ["PromiseUtils"]; +Components.utils.import("resource://gre/modules/Services.jsm"); Components.utils.import("resource://gre/modules/Timer.jsm"); this.PromiseUtils = { @@ -18,6 +19,32 @@ this.PromiseUtils = { defer() { return new Deferred(); }, + + /** + * Requests idle dispatch to the main thread for the given callback, + * and returns a promise which resolves to the callback's return value + * when it has been executed. + * + * @param {function} callback + * @param {integer} [timeout] + * An optional timeout, after which the callback will be + * executed immediately if idle dispatch has not yet occurred. + * + * @returns {Promise} + */ + idleDispatch(callback, timeout = 0) { + return new Promise((resolve, reject) => { + Services.tm.idleDispatchToMainThread( + () => { + try { + resolve(callback()); + } catch (e) { + reject(e); + } + }, + timeout); + }); + }, } /** From 341f33f8268e1cf7237941d05912229171ac1f5d Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Fri, 1 Sep 2017 14:58:34 -0700 Subject: [PATCH 12/42] Bug 1391707: Part 2 - Use idle dispatch in DeferredSave. r=florian MozReview-Commit-ID: Fffz9Qgom52 --- toolkit/mozapps/extensions/DeferredSave.jsm | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/toolkit/mozapps/extensions/DeferredSave.jsm b/toolkit/mozapps/extensions/DeferredSave.jsm index c9b0db4ad633..a78d37594822 100644 --- a/toolkit/mozapps/extensions/DeferredSave.jsm +++ b/toolkit/mozapps/extensions/DeferredSave.jsm @@ -188,7 +188,7 @@ this.DeferredSave.prototype = { this.logger.debug("Starting timer"); if (!this._timer) this._timer = MakeTimer(); - this._timer.initWithCallback(() => this._deferredSave(), + this._timer.initWithCallback(() => this._timerCallback(), this._delay, Ci.nsITimer.TYPE_ONE_SHOT); }, @@ -212,6 +212,10 @@ this.DeferredSave.prototype = { return this._pending.promise; }, + _timerCallback() { + Services.tm.idleDispatchToMainThread(() => this._deferredSave()); + }, + _deferredSave() { let pending = this._pending; this._pending = null; From f3044dc9790324f7b599896c69bcbe62cd2ab7e1 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 10 Sep 2017 15:39:28 -0700 Subject: [PATCH 13/42] Bug 1398630: Part 1 - Remove/cleanup some old ExtensionUtils helpers. r=zombie MozReview-Commit-ID: FeLUjH7pkiB --- .../components/extensions/ExtensionCommon.jsm | 18 +++--- .../components/extensions/ExtensionParent.jsm | 5 +- .../extensions/ExtensionPermissions.jsm | 4 +- .../components/extensions/ExtensionUtils.jsm | 57 +------------------ toolkit/components/extensions/Schemas.jsm | 3 +- 5 files changed, 18 insertions(+), 69 deletions(-) diff --git a/toolkit/components/extensions/ExtensionCommon.jsm b/toolkit/components/extensions/ExtensionCommon.jsm index 545c8afdddf6..3786b209e840 100644 --- a/toolkit/components/extensions/ExtensionCommon.jsm +++ b/toolkit/components/extensions/ExtensionCommon.jsm @@ -46,7 +46,6 @@ var { getConsole, getInnerWindowID, getUniqueId, - instanceOf, } = ExtensionUtils; XPCOMUtils.defineLazyGetter(this, "console", getConsole); @@ -377,8 +376,10 @@ class BaseContext { return error; } let message, fileName; - if (instanceOf(error, "Object") || error instanceof ExtensionError || - (typeof error == "object" && this.principal.subsumes(Cu.getObjectPrincipal(error)))) { + if (error && typeof error === "object" && + (ChromeUtils.getClassName(error) === "Object" || + error instanceof ExtensionError || + this.principal.subsumes(Cu.getObjectPrincipal(error)))) { message = error.message; fileName = error.fileName; } else { @@ -1450,6 +1451,9 @@ LocaleData.prototype = { addLocale(locale, messages, extension) { let result = new Map(); + let isPlainObject = obj => (obj && typeof obj === "object" && + ChromeUtils.getClassName(obj) === "Object"); + // Chrome does not document the semantics of its localization // system very well. It handles replacements by pre-processing // messages, replacing |$[a-zA-Z0-9@_]+$| tokens with the value of their @@ -1461,7 +1465,7 @@ LocaleData.prototype = { // 1. It also accepts |$| followed by any number of sequential // digits, but refuses to process a localized string which // provides more than 9 substitutions. - if (!instanceOf(messages, "Object")) { + if (!isPlainObject(messages)) { extension.packagingError(`Invalid locale data for ${locale}`); return result; } @@ -1469,7 +1473,7 @@ LocaleData.prototype = { for (let key of Object.keys(messages)) { let msg = messages[key]; - if (!instanceOf(msg, "Object") || typeof(msg.message) != "string") { + if (!isPlainObject(msg) || typeof(msg.message) != "string") { extension.packagingError(`Invalid locale message data for ${locale}, message ${JSON.stringify(key)}`); continue; } @@ -1477,7 +1481,7 @@ LocaleData.prototype = { // Substitutions are case-insensitive, so normalize all of their names // to lower-case. let placeholders = new Map(); - if (instanceOf(msg.placeholders, "Object")) { + if (isPlainObject(msg.placeholders)) { for (let key of Object.keys(msg.placeholders)) { placeholders.set(key.toLowerCase(), msg.placeholders[key]); } @@ -1485,7 +1489,7 @@ LocaleData.prototype = { let replacer = (match, name) => { let replacement = placeholders.get(name.toLowerCase()); - if (instanceOf(replacement, "Object") && "content" in replacement) { + if (isPlainObject(replacement) && "content" in replacement) { return replacement.content; } return ""; diff --git a/toolkit/components/extensions/ExtensionParent.jsm b/toolkit/components/extensions/ExtensionParent.jsm index cd540dff4c3a..3fcba2c52afa 100644 --- a/toolkit/components/extensions/ExtensionParent.jsm +++ b/toolkit/components/extensions/ExtensionParent.jsm @@ -52,7 +52,6 @@ var { defineLazyGetter, promiseDocumentLoaded, promiseEvent, - promiseFileContents, promiseObserved, } = ExtensionUtils; @@ -1435,9 +1434,9 @@ StartupCache = { async _readData() { let result = new Map(); try { - let data = await promiseFileContents(this.file); + let {buffer} = await OS.File.read(this.file); - result = aomStartup.decodeBlob(data); + result = aomStartup.decodeBlob(buffer); } catch (e) { if (!e.becauseNoSuchFile) { Cu.reportError(e); diff --git a/toolkit/components/extensions/ExtensionPermissions.jsm b/toolkit/components/extensions/ExtensionPermissions.jsm index 278de977de46..ffb336e4031b 100644 --- a/toolkit/components/extensions/ExtensionPermissions.jsm +++ b/toolkit/components/extensions/ExtensionPermissions.jsm @@ -27,8 +27,8 @@ async function _lazyInit() { prefs.data = {}; try { - let blob = await ExtensionUtils.promiseFileContents(path); - prefs.data = JSON.parse(new TextDecoder().decode(blob)); + let {buffer} = await OS.File.read(path); + prefs.data = JSON.parse(new TextDecoder().decode(buffer)); } catch (e) { if (!e.becauseNoSuchFile) { Cu.reportError(e); diff --git a/toolkit/components/extensions/ExtensionUtils.jsm b/toolkit/components/extensions/ExtensionUtils.jsm index db49950bd10a..5835cda3b83b 100644 --- a/toolkit/components/extensions/ExtensionUtils.jsm +++ b/toolkit/components/extensions/ExtensionUtils.jsm @@ -37,11 +37,6 @@ function getUniqueId() { return `${nextId++}-${uniqueProcessID}`; } -async function promiseFileContents(path) { - let res = await OS.File.read(path); - return res.buffer; -} - /** * An Error subclass for which complete error messages are always passed @@ -63,55 +58,11 @@ function runSafeSyncWithoutClone(f, ...args) { } } -// Run a function and report exceptions. -function runSafeWithoutClone(f, ...args) { - if (typeof(f) != "function") { - dump(`Extension error: expected function\n${filterStack(Error())}`); - return; - } - - Promise.resolve().then(() => { - runSafeSyncWithoutClone(f, ...args); - }); -} - -// Run a function, cloning arguments into context.cloneScope, and -// report exceptions. |f| is expected to be in context.cloneScope. -function runSafeSync(context, f, ...args) { - if (context.unloaded) { - Cu.reportError("runSafeSync called after context unloaded"); - return; - } - - try { - args = Cu.cloneInto(args, context.cloneScope); - } catch (e) { - Cu.reportError(e); - dump(`runSafe failure: cloning into ${context.cloneScope}: ${e}\n\n${filterStack(Error())}`); - } - return runSafeSyncWithoutClone(f, ...args); -} - -// Run a function, cloning arguments into context.cloneScope, and -// report exceptions. |f| is expected to be in context.cloneScope. -function runSafe(context, f, ...args) { - try { - args = Cu.cloneInto(args, context.cloneScope); - } catch (e) { - Cu.reportError(e); - dump(`runSafe failure: cloning into ${context.cloneScope}: ${e}\n\n${filterStack(Error())}`); - } - if (context.unloaded) { - dump(`runSafe failure: context is already unloaded ${filterStack(new Error())}\n`); - return undefined; - } - return runSafeWithoutClone(f, ...args); -} - // Return true if the given value is an instance of the given // native type. function instanceOf(value, type) { - return {}.toString.call(value) == `[object ${type}]`; + return (value && typeof value === "object" && + ChromeUtils.getClassName(value) === type); } /** @@ -684,12 +635,8 @@ this.ExtensionUtils = { promiseDocumentLoaded, promiseDocumentReady, promiseEvent, - promiseFileContents, promiseObserved, - runSafe, - runSafeSync, runSafeSyncWithoutClone, - runSafeWithoutClone, withHandlingUserInput, DefaultMap, DefaultWeakMap, diff --git a/toolkit/components/extensions/Schemas.jsm b/toolkit/components/extensions/Schemas.jsm index 461c5ed464d4..b129a7b6be07 100644 --- a/toolkit/components/extensions/Schemas.jsm +++ b/toolkit/components/extensions/Schemas.jsm @@ -21,7 +21,6 @@ Cu.import("resource://gre/modules/ExtensionUtils.jsm"); var { DefaultMap, DefaultWeakMap, - instanceOf, } = ExtensionUtils; XPCOMUtils.defineLazyModuleGetter(this, "ExtensionParent", @@ -1627,7 +1626,7 @@ class ObjectType extends Type { } } - if (!instanceOf(value, this.isInstanceOf)) { + if (ChromeUtils.getClassName(value) !== this.isInstanceOf) { return context.error(`Object must be an instance of ${this.isInstanceOf}`, `be an instance of ${this.isInstanceOf}`); } From 34be6906e02fdd668c2c13fe602cd3192961f4f6 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 10 Sep 2017 15:39:49 -0700 Subject: [PATCH 14/42] Bug 1398630: Part 2 - Avoid unnecessary Map/Set lookups. r=zombie We currently call has() every time we do a DefaultMap/DefaultWeakMap lookup, which unfortunately shows up a lot in profiles. We only actually need to check, though, if get() returns an undefined value. Similar things in other places, where we only need to do a has() call if another operation fails. MozReview-Commit-ID: 9qFWsb4vlZj --- .../components/extensions/ExtensionUtils.jsm | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/toolkit/components/extensions/ExtensionUtils.jsm b/toolkit/components/extensions/ExtensionUtils.jsm index 5835cda3b83b..fdf82483a1d9 100644 --- a/toolkit/components/extensions/ExtensionUtils.jsm +++ b/toolkit/components/extensions/ExtensionUtils.jsm @@ -76,10 +76,12 @@ class DefaultWeakMap extends WeakMap { } get(key) { - if (!this.has(key)) { - this.set(key, this.defaultConstructor(key)); + let value = super.get(key); + if (value === undefined && !this.has(key)) { + value = this.defaultConstructor(key); + this.set(key, value); } - return super.get(key); + return value; } } @@ -90,10 +92,12 @@ class DefaultMap extends Map { } get(key) { - if (!this.has(key)) { - this.set(key, this.defaultConstructor(key)); + let value = super.get(key); + if (value === undefined && !this.has(key)) { + value = this.defaultConstructor(key); + this.set(key, value); } - return super.get(key); + return value; } } @@ -138,11 +142,13 @@ class EventEmitter { * The listener to call when events are emitted. */ on(event, listener) { - if (!this[LISTENERS].has(event)) { - this[LISTENERS].set(event, new Set()); + let listeners = this[LISTENERS].get(event); + if (!listeners) { + listeners = new Set(); + this[LISTENERS].set(event, listeners); } - this[LISTENERS].get(event).add(listener); + listeners.add(listener); } /** @@ -154,9 +160,8 @@ class EventEmitter { * The listener function to remove. */ off(event, listener) { - if (this[LISTENERS].has(event)) { - let set = this[LISTENERS].get(event); - + let set = this.listeners.get(event); + if (set) { set.delete(listener); set.delete(this[ONCE_MAP].get(listener)); if (!set.size) { @@ -255,7 +260,7 @@ class LimitedSet extends Set { } add(item) { - if (!this.has(item) && this.size >= this.limit + this.slop) { + if (this.size >= this.limit + this.slop && !this.has(item)) { this.truncate(this.limit - 1); } super.add(item); From e37c07842c4f267184db12a9a1d222f61a84833d Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 14:53:20 -0700 Subject: [PATCH 15/42] Bug 1398630: Part 3 - Use document.docShell rather than longer/slower XPC paths. r=zombie MozReview-Commit-ID: 5oD0Uvv1pvx --- browser/components/extensions/ext-browser.js | 4 +--- browser/components/extensions/ext-tabs.js | 4 +--- toolkit/components/extensions/ExtensionCommon.jsm | 3 +-- toolkit/components/extensions/ExtensionPageChild.jsm | 3 +-- toolkit/components/extensions/ExtensionParent.jsm | 3 +-- toolkit/components/extensions/ext-tabs-base.js | 5 ++--- 6 files changed, 7 insertions(+), 15 deletions(-) diff --git a/browser/components/extensions/ext-browser.js b/browser/components/extensions/ext-browser.js index 6e07a791c0be..2a28e4408760 100644 --- a/browser/components/extensions/ext-browser.js +++ b/browser/components/extensions/ext-browser.js @@ -517,9 +517,7 @@ class TabTracker extends TabTrackerBase { if (browser.ownerDocument.documentURI === "about:addons") { // When we're loaded into a inside about:addons, we need to go up // one more level. - browser = browser.ownerGlobal.QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIDocShell) - .chromeEventHandler; + browser = browser.ownerDocument.docShell.chromeEventHandler; } let result = { diff --git a/browser/components/extensions/ext-tabs.js b/browser/components/extensions/ext-tabs.js index b8745cad82e5..b0963433fc20 100644 --- a/browser/components/extensions/ext-tabs.js +++ b/browser/components/extensions/ext-tabs.js @@ -734,9 +734,7 @@ this.tabs = class extends ExtensionAPI { // For non-remote browsers, this event is dispatched on the document // rather than on the . if (browser instanceof Ci.nsIDOMDocument) { - browser = browser.defaultView.QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIDocShell) - .chromeEventHandler; + browser = browser.docShell.chromeEventHandler; } let {gBrowser} = browser.ownerGlobal; diff --git a/toolkit/components/extensions/ExtensionCommon.jsm b/toolkit/components/extensions/ExtensionCommon.jsm index 3786b209e840..b6cdfeb93d80 100644 --- a/toolkit/components/extensions/ExtensionCommon.jsm +++ b/toolkit/components/extensions/ExtensionCommon.jsm @@ -195,8 +195,7 @@ class BaseContext { setContentWindow(contentWindow) { let {document} = contentWindow; - let docShell = contentWindow.QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIDocShell); + let {docShell} = document; this.innerWindowID = getInnerWindowID(contentWindow); this.messageManager = docShell.QueryInterface(Ci.nsIInterfaceRequestor) diff --git a/toolkit/components/extensions/ExtensionPageChild.jsm b/toolkit/components/extensions/ExtensionPageChild.jsm index e14ae461cd1f..c80dc8063397 100644 --- a/toolkit/components/extensions/ExtensionPageChild.jsm +++ b/toolkit/components/extensions/ExtensionPageChild.jsm @@ -363,8 +363,7 @@ ExtensionPageChild = { throw new Error("An extension context was already initialized for this frame"); } - let mm = contentWindow.QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIDocShell) + let mm = contentWindow.document.docShell .QueryInterface(Ci.nsIInterfaceRequestor) .getInterface(Ci.nsIContentFrameMessageManager); diff --git a/toolkit/components/extensions/ExtensionParent.jsm b/toolkit/components/extensions/ExtensionParent.jsm index 3fcba2c52afa..ee0d558fd960 100644 --- a/toolkit/components/extensions/ExtensionParent.jsm +++ b/toolkit/components/extensions/ExtensionParent.jsm @@ -484,8 +484,7 @@ class ExtensionPageContextParent extends ProxyContextParent { // The window that contains this context. This may change due to moving tabs. get xulWindow() { let win = this.xulBrowser.ownerGlobal; - return win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDocShell) - .QueryInterface(Ci.nsIDocShellTreeItem).rootTreeItem + return win.document.docShell.rootTreeItem .QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindow); } diff --git a/toolkit/components/extensions/ext-tabs-base.js b/toolkit/components/extensions/ext-tabs-base.js index b57d30ca877d..b1dff49bc0bb 100644 --- a/toolkit/components/extensions/ext-tabs-base.js +++ b/toolkit/components/extensions/ext-tabs-base.js @@ -694,9 +694,8 @@ class WindowBase { * @readonly */ get xulWindow() { - return this.window.QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIDocShell) - .treeOwner.QueryInterface(Ci.nsIInterfaceRequestor) + return this.window.document.docShell.treeOwner + .QueryInterface(Ci.nsIInterfaceRequestor) .getInterface(Ci.nsIXULWindow); } From 60387c4c6e3e0301b2efbc554b3453e8fbf994e7 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 10 Sep 2017 15:33:54 -0700 Subject: [PATCH 16/42] Bug 1398630: Part 4 - Use getWinUtils everywhere we use DOMWindowUtils. r=zombie MozReview-Commit-ID: FroMQF9Tiz1 --- toolkit/components/extensions/ExtensionChild.jsm | 4 ++-- toolkit/components/extensions/ExtensionCommon.jsm | 5 ++--- toolkit/components/extensions/ext-tabs-base.js | 2 -- toolkit/components/extensions/ext-theme.js | 8 +++++--- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/toolkit/components/extensions/ExtensionChild.jsm b/toolkit/components/extensions/ExtensionChild.jsm index 80f08793aa34..642c80ff3fca 100644 --- a/toolkit/components/extensions/ExtensionChild.jsm +++ b/toolkit/components/extensions/ExtensionChild.jsm @@ -44,6 +44,7 @@ const { defineLazyGetter, getMessageManager, getUniqueId, + getWinUtils, withHandlingUserInput, } = ExtensionUtils; @@ -684,8 +685,7 @@ class ProxyAPIImplementation extends SchemaAPIInterface { callAsyncFunction(args, callback, requireUserInput) { if (requireUserInput) { let context = this.childApiManager.context; - let winUtils = context.contentWindow.getInterface(Ci.nsIDOMWindowUtils); - if (!winUtils.isHandlingUserInput) { + if (!getWinUtils(context.contentWindow).isHandlingUserInput) { let err = new context.cloneScope.Error(`${this.path} may only be called from a user input handler`); return context.wrapPromise(Promise.reject(err), callback); } diff --git a/toolkit/components/extensions/ExtensionCommon.jsm b/toolkit/components/extensions/ExtensionCommon.jsm index b6cdfeb93d80..9a91be29773a 100644 --- a/toolkit/components/extensions/ExtensionCommon.jsm +++ b/toolkit/components/extensions/ExtensionCommon.jsm @@ -46,6 +46,7 @@ var { getConsole, getInnerWindowID, getUniqueId, + getWinUtils, } = ExtensionUtils; XPCOMUtils.defineLazyGetter(this, "console", getConsole); @@ -668,9 +669,7 @@ class LocalAPIImplementation extends SchemaAPIInterface { let promise; try { if (requireUserInput) { - let winUtils = this.context.contentWindow - .getInterface(Ci.nsIDOMWindowUtils); - if (!winUtils.isHandlingUserInput) { + if (!getWinUtils(this.context.contentWindow).isHandlingUserInput) { throw new ExtensionError(`${this.name} may only be called from a user input handler`); } } diff --git a/toolkit/components/extensions/ext-tabs-base.js b/toolkit/components/extensions/ext-tabs-base.js index b1dff49bc0bb..59739090571f 100644 --- a/toolkit/components/extensions/ext-tabs-base.js +++ b/toolkit/components/extensions/ext-tabs-base.js @@ -1206,8 +1206,6 @@ class WindowTrackerBase extends EventEmitter { }); this._windowIds = new DefaultWeakMap(window => { - window.QueryInterface(Ci.nsIInterfaceRequestor); - return getWinUtils(window).outerWindowID; }); } diff --git a/toolkit/components/extensions/ext-theme.js b/toolkit/components/extensions/ext-theme.js index 298dd81bcdb2..919b59b10c7b 100644 --- a/toolkit/components/extensions/ext-theme.js +++ b/toolkit/components/extensions/ext-theme.js @@ -11,6 +11,10 @@ XPCOMUtils.defineLazyGetter(this, "gThemesEnabled", () => { return Services.prefs.getBoolPref("extensions.webextensions.themes.enabled"); }); +var { + getWinUtils, +} = ExtensionUtils; + const ICONS = Services.prefs.getStringPref("extensions.webextensions.themes.icons.buttons", "").split(","); /** Class representing a theme. */ @@ -43,9 +47,7 @@ class Theme { */ load(details, targetWindow) { if (targetWindow) { - this.lwtStyles.window = targetWindow - .QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIDOMWindowUtils).outerWindowID; + this.lwtStyles.window = getWinUtils(targetWindow).outerWindowID; } if (details.colors) { From cc6c48d0953bef131f44a517b66c39fab649b656 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 10 Sep 2017 15:35:46 -0700 Subject: [PATCH 17/42] Bug 1398630: Part 5 - User iteration helpers for nsISimpleEnumerator. r=zombie MozReview-Commit-ID: Iw25XozakK0 --- browser/components/extensions/ext-browsingData.js | 5 +---- toolkit/components/extensions/ext-cookies.js | 3 +-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/browser/components/extensions/ext-browsingData.js b/browser/components/extensions/ext-browsingData.js index 109ec9601487..e313319333a0 100644 --- a/browser/components/extensions/ext-browsingData.js +++ b/browser/components/extensions/ext-browsingData.js @@ -50,10 +50,7 @@ const clearCookies = async function(options) { if (options.since || options.hostnames) { // Iterate through the cookies and delete any created after our cutoff. - let cookiesEnum = cookieMgr.enumerator; - while (cookiesEnum.hasMoreElements()) { - let cookie = cookiesEnum.getNext().QueryInterface(Ci.nsICookie2); - + for (const cookie of XPCOMUtils.IterSimpleEnumerator(cookieMgr.enumerator, Ci.nsICookie2)) { if ((!options.since || cookie.creationTime >= PlacesUtils.toPRTime(options.since)) && (!options.hostnames || options.hostnames.includes(cookie.host.replace(/^\./, "")))) { // This cookie was created after our cutoff, clear it. diff --git a/toolkit/components/extensions/ext-cookies.js b/toolkit/components/extensions/ext-cookies.js index 99682b389f45..03c73b346e8a 100644 --- a/toolkit/components/extensions/ext-cookies.js +++ b/toolkit/components/extensions/ext-cookies.js @@ -260,8 +260,7 @@ const query = function* (detailsIn, props, context) { return true; } - while (enumerator.hasMoreElements()) { - let cookie = enumerator.getNext().QueryInterface(Ci.nsICookie2); + for (const cookie of XPCOMUtils.IterSimpleEnumerator(enumerator, Ci.nsICookie2)) { if (matches(cookie)) { yield {cookie, isPrivate, storeId}; } From d53c54d3889f2c57591f1f85bb933c7f77a1fecd Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 10 Sep 2017 15:36:57 -0700 Subject: [PATCH 18/42] Bug 1398630: Part 6 - Avoid some avoidable uses of nsIURI. r=zombie MozReview-Commit-ID: 18Wd3buFM38 --- toolkit/components/extensions/Extension.jsm | 2 +- toolkit/components/extensions/ext-cookies.js | 21 ++++++++++---------- toolkit/components/extensions/ext-runtime.js | 4 ++-- toolkit/components/extensions/ext-toolkit.js | 2 ++ 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index fba75b1c8c6a..80490345e8bc 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -382,7 +382,7 @@ this.ExtensionData = class { async readDirectory(path) { if (this.rootURI instanceof Ci.nsIFileURL) { - let uri = Services.io.newURI(this.rootURI.resolve("./" + path)); + let uri = Services.io.newURI("./" + path, null, this.rootURI); let fullPath = uri.QueryInterface(Ci.nsIFileURL).file.path; let iter = new OS.File.DirectoryIterator(fullPath); diff --git a/toolkit/components/extensions/ext-cookies.js b/toolkit/components/extensions/ext-cookies.js index 03c73b346e8a..9788f605034d 100644 --- a/toolkit/components/extensions/ext-cookies.js +++ b/toolkit/components/extensions/ext-cookies.js @@ -172,15 +172,15 @@ const query = function* (detailsIn, props, context) { // We can use getCookiesFromHost for faster searching. let enumerator; - let uri; + let url; let originAttributes = { userContextId, privateBrowsingId: isPrivate ? 1 : 0, }; if ("url" in details) { try { - uri = Services.io.newURI(details.url).QueryInterface(Ci.nsIURL); - enumerator = Services.cookies.getCookiesFromHost(uri.host, originAttributes); + url = new URL(details.url); + enumerator = Services.cookies.getCookiesFromHost(url.host, originAttributes); } catch (ex) { // This often happens for about: URLs return; @@ -211,21 +211,20 @@ const query = function* (detailsIn, props, context) { // URL path is a substring of the cookie path, so it matches if, and // only if, the next character is a path delimiter. - let pathDelimiters = ["/", "?", "#", ";"]; - return pathDelimiters.includes(path[cookiePath.length]); + return path[cookiePath.length] === "/"; } // "Restricts the retrieved cookies to those that would match the given URL." - if (uri) { - if (!domainMatches(uri.host)) { + if (url) { + if (!domainMatches(url.host)) { return false; } - if (cookie.isSecure && uri.scheme != "https") { + if (cookie.isSecure && url.protocol != "https:") { return false; } - if (!pathMatches(uri.pathQueryRef)) { + if (!pathMatches(url.path)) { return false; } } @@ -290,7 +289,7 @@ this.cookies = class extends ExtensionAPI { }, set: function(details) { - let uri = Services.io.newURI(details.url).QueryInterface(Ci.nsIURL); + let uri = Services.io.newURI(details.url); let path; if (details.path !== null) { @@ -300,7 +299,7 @@ this.cookies = class extends ExtensionAPI { // Set-Cookie header. In the case of an omitted path, the cookie // service uses the directory path of the requesting URL, ignoring // any filename or query parameters. - path = uri.directory; + path = uri.QueryInterface(Ci.nsIURL).directory; } let name = details.name !== null ? details.name : ""; diff --git a/toolkit/components/extensions/ext-runtime.js b/toolkit/components/extensions/ext-runtime.js index feeeb982d1b7..2b3a6b357b9d 100644 --- a/toolkit/components/extensions/ext-runtime.js +++ b/toolkit/components/extensions/ext-runtime.js @@ -124,12 +124,12 @@ this.runtime = class extends ExtensionAPI { let uri; try { - uri = Services.io.newURI(url); + uri = new URL(url); } catch (e) { return Promise.reject({message: `Invalid URL: ${JSON.stringify(url)}`}); } - if (uri.scheme != "http" && uri.scheme != "https") { + if (uri.protocol != "http:" && uri.protocol != "https:") { return Promise.reject({message: "url must have the scheme http or https"}); } diff --git a/toolkit/components/extensions/ext-toolkit.js b/toolkit/components/extensions/ext-toolkit.js index df07662b9863..98ff48379a35 100644 --- a/toolkit/components/extensions/ext-toolkit.js +++ b/toolkit/components/extensions/ext-toolkit.js @@ -15,6 +15,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "ContextualIdentityService", "resource://gre/modules/ContextualIdentityService.jsm"); +Cu.importGlobalProperties(["URL"]); + Cu.import("resource://gre/modules/ExtensionCommon.jsm"); global.EventEmitter = ExtensionUtils.EventEmitter; From 139c5edc3f812f537c77ae8da386900ca59721c4 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 10 Sep 2017 15:37:08 -0700 Subject: [PATCH 19/42] Bug 1398630: Part 7 - Random cleanup. r=zombie MozReview-Commit-ID: LibtXDKXrnA --- toolkit/components/extensions/Extension.jsm | 1 - toolkit/components/extensions/ExtensionCommon.jsm | 3 +-- toolkit/components/extensions/ExtensionUtils.jsm | 9 ++------- toolkit/components/extensions/MessageChannel.jsm | 5 +---- 4 files changed, 4 insertions(+), 14 deletions(-) diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index 80490345e8bc..f9389e15f93d 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -1714,7 +1714,6 @@ this.Langpack = class extends ExtensionData { chromeEntries.push(["locale", alias, language, path[platform]]); } } - } } return chromeEntries; diff --git a/toolkit/components/extensions/ExtensionCommon.jsm b/toolkit/components/extensions/ExtensionCommon.jsm index 9a91be29773a..9c73c15baad7 100644 --- a/toolkit/components/extensions/ExtensionCommon.jsm +++ b/toolkit/components/extensions/ExtensionCommon.jsm @@ -23,7 +23,6 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm"); XPCOMUtils.defineLazyModuleGetters(this, { ConsoleAPI: "resource://gre/modules/Console.jsm", MessageChannel: "resource://gre/modules/MessageChannel.jsm", - Preferences: "resource://gre/modules/Preferences.jsm", PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm", Schemas: "resource://gre/modules/Schemas.jsm", }); @@ -1504,7 +1503,7 @@ LocaleData.prototype = { }, get acceptLanguages() { - let result = Preferences.get("intl.accept_languages", "", Ci.nsIPrefLocalizedString); + let result = Services.prefs.getComplexValue("intl.accept_languages", Ci.nsIPrefLocalizedString).data; return result.split(/\s*,\s*/g); }, diff --git a/toolkit/components/extensions/ExtensionUtils.jsm b/toolkit/components/extensions/ExtensionUtils.jsm index fdf82483a1d9..dbfa3f2127f9 100644 --- a/toolkit/components/extensions/ExtensionUtils.jsm +++ b/toolkit/components/extensions/ExtensionUtils.jsm @@ -6,10 +6,7 @@ this.EXPORTED_SYMBOLS = ["ExtensionUtils"]; -const Ci = Components.interfaces; -const Cc = Components.classes; -const Cu = Components.utils; -const Cr = Components.results; +const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://gre/modules/XPCOMUtils.jsm"); @@ -302,9 +299,7 @@ function promiseDocumentLoaded(doc) { } return new Promise(resolve => { - doc.defaultView.addEventListener("load", function(event) { - resolve(doc); - }, {once: true}); + doc.defaultView.addEventListener("load", () => resolve(doc), {once: true}); }); } diff --git a/toolkit/components/extensions/MessageChannel.jsm b/toolkit/components/extensions/MessageChannel.jsm index 8ec8dce2f2d3..0581ff44d3d1 100644 --- a/toolkit/components/extensions/MessageChannel.jsm +++ b/toolkit/components/extensions/MessageChannel.jsm @@ -100,10 +100,7 @@ this.EXPORTED_SYMBOLS = ["MessageChannel"]; /* globals MessageChannel */ -const Ci = Components.interfaces; -const Cc = Components.classes; -const Cu = Components.utils; -const Cr = Components.results; +const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; Cu.import("resource://gre/modules/AppConstants.jsm"); Cu.import("resource://gre/modules/ExtensionUtils.jsm"); From c2886dcebf9e4244cedfae5b1255cbced6c9d077 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 10 Sep 2017 18:37:52 -0700 Subject: [PATCH 20/42] Bug 1398642: Several minor WebRequest optimizations. r=mixedpuppy MozReview-Commit-ID: 4vsDScMkyzA --- toolkit/modules/addons/WebRequest.jsm | 65 +++++++++++---------------- 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/toolkit/modules/addons/WebRequest.jsm b/toolkit/modules/addons/WebRequest.jsm index d89ae60454f6..d41f02d84052 100644 --- a/toolkit/modules/addons/WebRequest.jsm +++ b/toolkit/modules/addons/WebRequest.jsm @@ -86,8 +86,7 @@ class HeaderChanger { } toArray() { - return Array.from(this.originalHeaders, - ([key, {name, value}]) => ({name, value})); + return Array.from(this.originalHeaders.values()); } validateHeaders(headers) { @@ -673,7 +672,7 @@ HttpObserverManager = { return false; } - return WebRequestCommon.urlMatches(uri, filter.urls); + return !filter.urls || filter.urls.matches(uri); }, get resultsMap() { @@ -682,20 +681,21 @@ HttpObserverManager = { return this.resultsMap; }, - maybeError({channel}, extraData = null) { - if (!(extraData && extraData.error) && channel.securityInfo) { - let securityInfo = channel.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo); + maybeError({channel}) { + // FIXME: Move to ChannelWrapper. + + let {securityInfo} = channel; + if (securityInfo instanceof Ci.nsITransportSecurityInfo) { if (NSSErrorsService.isNSSErrorCode(securityInfo.errorCode)) { let nsresult = NSSErrorsService.getXPCOMFromNSSError(securityInfo.errorCode); - extraData = {error: NSSErrorsService.getErrorMessage(nsresult)}; + return {error: NSSErrorsService.getErrorMessage(nsresult)}; } } - if (!(extraData && extraData.error)) { - if (!Components.isSuccessCode(channel.status)) { - extraData = {error: this.resultsMap.get(channel.status) || "NS_ERROR_NET_UNKNOWN"}; - } + + if (!Components.isSuccessCode(channel.status)) { + return {error: this.resultsMap.get(channel.status) || "NS_ERROR_NET_UNKNOWN"}; } - return extraData; + return null; }, errorCheck(channel) { @@ -806,7 +806,7 @@ HttpObserverManager = { } let data = Object.assign({}, commonData); - if (registerFilter) { + if (registerFilter && opts.blocking) { this.registerChannel(channel, opts); } @@ -843,28 +843,21 @@ HttpObserverManager = { }, async applyChanges(kind, channel, handlerResults, requestHeaders, responseHeaders) { - let asyncHandlers = handlerResults.filter(({result}) => isThenable(result)); - let isAsync = asyncHandlers.length > 0; - let shouldResume = false; + let shouldResume = !channel.suspended; try { - if (isAsync) { - shouldResume = !channel.suspended; - channel.suspended = true; - - for (let value of asyncHandlers) { + for (let {opts, result} of handlerResults) { + if (isThenable(result)) { + channel.suspended = true; try { - value.result = await value.result; + result = await result; } catch (e) { Cu.reportError(e); - value.result = {}; + continue; + } + if (!result || typeof result !== "object") { + continue; } - } - } - - for (let {opts, result} of handlerResults) { - if (!result || typeof result !== "object") { - continue; } if (result.cancel) { @@ -893,21 +886,17 @@ HttpObserverManager = { responseHeaders.applyChanges(result.responseHeaders); } - if (kind === "authRequired" && opts.blocking && result.authCredentials) { - if (channel.authPromptCallback) { - channel.authPromptCallback(result.authCredentials); - } + if (kind === "authRequired" && result.authCredentials && channel.authPromptCallback) { + channel.authPromptCallback(result.authCredentials); } } // If a listener did not cancel the request or provide credentials, we // forward the auth request to the base handler. - if (kind === "authRequired") { - if (channel.authPromptForward) { - channel.authPromptForward(); - } + if (kind === "authRequired" && channel.authPromptForward) { + channel.authPromptForward(); } - if (kind === "modify") { + if (kind === "modify" && this.listeners.afterModify.size) { await this.runChannelListener(channel, "afterModify"); } } catch (e) { From e87b47fe0a101e0a8fdd7cb59c58684670636df3 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Mon, 11 Sep 2017 18:32:44 -0400 Subject: [PATCH 21/42] Bug 1389021 - Explicitly shutdown the CompositorManagerChild during ContentChild::ActorDestroy. r=me It has been observed on nightly and beta that the compositer thread fails to shutdown gracefully due to lingering references. From what can be determined, it appears as if the content process references are what are keeping it alive. The shutdown of CompositorBridgeChild was altered because a top level protocol was added above it in a previous change in bug 1365927. This protocol tree is ultimately what is keeping the thread alive. As such, this patch adds an explicit shutdown of the protocol, to ensure it gets released in a timely manner. This change will be backed out if it appears to have no effect on the crash rate in nightly 57. --- dom/ipc/ContentChild.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 93550f61be26..4e1d69b16ee1 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -2310,6 +2310,8 @@ ContentChild::ActorDestroy(ActorDestroyReason why) } #ifndef NS_FREE_PERMANENT_DATA + CompositorManagerChild::Shutdown(); + // In release builds, there's no point in the content process // going through the full XPCOM shutdown path, because it doesn't // keep persistent state. From 14b550078dea065f61032ac7838add7026dfa17c Mon Sep 17 00:00:00 2001 From: Eric Rahm Date: Tue, 5 Sep 2017 12:26:51 -0700 Subject: [PATCH 22/42] Bug 1396958 - Make eTLD cache thread-safe. r=valentin Restrict the MRU cache for eTLD lookups to main thread only. This allows off main thread lookups, but they will just take a slower path. --- netwerk/dns/nsEffectiveTLDService.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/netwerk/dns/nsEffectiveTLDService.cpp b/netwerk/dns/nsEffectiveTLDService.cpp index 1950de316447..88b318bc95e7 100644 --- a/netwerk/dns/nsEffectiveTLDService.cpp +++ b/netwerk/dns/nsEffectiveTLDService.cpp @@ -212,9 +212,10 @@ nsEffectiveTLDService::GetBaseDomainInternal(nsCString &aHostname, if (result == PR_SUCCESS) return NS_ERROR_HOST_IS_IP_ADDRESS; - // Lookup in the cache if this is a normal query. + // Lookup in the cache if this is a normal query. This is restricted to + // main thread-only as the cache is not thread-safe. TLDCacheEntry* entry = nullptr; - if (aAdditionalParts == 1) { + if (aAdditionalParts == 1 && NS_IsMainThread()) { if (LookupForAdd(aHostname, &entry)) { // There was a match, just return the cached value. aBaseDomain = entry->mBaseDomain; From 8f38a19d029984173e797ec35a67328b3f25828b Mon Sep 17 00:00:00 2001 From: Alexander Surkov Date: Mon, 11 Sep 2017 19:05:38 -0400 Subject: [PATCH 23/42] Bug 1379808 - Intermittent browser_test_zoom_text.js failure, wrong height and y, r=eeejay --- accessible/tests/browser/bounds/browser.ini | 2 +- accessible/tests/mochitest/common.js | 12 ++++++++++++ accessible/tests/mochitest/layout.js | 18 ++++++++++++------ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/accessible/tests/browser/bounds/browser.ini b/accessible/tests/browser/bounds/browser.ini index f11c3678fe6e..1170dbcd1b01 100644 --- a/accessible/tests/browser/bounds/browser.ini +++ b/accessible/tests/browser/bounds/browser.ini @@ -8,4 +8,4 @@ support-files = [browser_test_zoom.js] [browser_test_zoom_text.js] -skip-if = true # Bug 1372296, Bug 1379808, Bug 1391453 +skip-if = e10s && os == 'win' # bug 1372296 diff --git a/accessible/tests/mochitest/common.js b/accessible/tests/mochitest/common.js index ef24ed614264..33aa24c616b0 100644 --- a/accessible/tests/mochitest/common.js +++ b/accessible/tests/mochitest/common.js @@ -191,6 +191,18 @@ function isObject(aObj, aExpectedObj, aMsg) { "', expected '" + prettyName(aExpectedObj) + "'"); } +/** + * is() function checking the expected value is within the range. + */ +function isWithin(aExpected, aGot, aWithin, aMsg) { + if (Math.abs(aGot - aExpected) <= aWithin) { + ok(true, `${aMsg} - Got ${aGot}`); + } else { + ok(false, + `${aMsg} - Got ${aGot}, expected ${aExpected} with error of ${aWithin}`); + } +} + // ////////////////////////////////////////////////////////////////////////////// // Helpers for getting DOM node/accessible diff --git a/accessible/tests/mochitest/layout.js b/accessible/tests/mochitest/layout.js index a8274cd6aa9b..1d82affbd9dc 100644 --- a/accessible/tests/mochitest/layout.js +++ b/accessible/tests/mochitest/layout.js @@ -150,13 +150,18 @@ function testTextBounds(aID, aStartOffset, aEndOffset, aRect, aCoordOrigin) { var hyperText = getAccessible(aID, [nsIAccessibleText]); hyperText.getRangeExtents(aStartOffset, aEndOffset, xObj, yObj, widthObj, heightObj, aCoordOrigin); + + // x is(xObj.value, expectedX, "Wrong x coordinate of text between offsets (" + aStartOffset + ", " + aEndOffset + ") for " + prettyName(aID)); - is(yObj.value, expectedY, - "Wrong y coordinate of text between offsets (" + aStartOffset + ", " + - aEndOffset + ") for " + prettyName(aID)); + // y + isWithin(yObj.value, expectedY, 1, + `y coord of text between offsets (${aStartOffset}, ${aEndOffset}) ` + + `for ${prettyName(aID)}`); + + // Width var msg = "Wrong width of text between offsets (" + aStartOffset + ", " + aEndOffset + ") for " + prettyName(aID); if (widthObj.value == expectedWidth) @@ -164,9 +169,10 @@ function testTextBounds(aID, aStartOffset, aEndOffset, aRect, aCoordOrigin) { else todo(false, msg); // fails on some windows machines - is(heightObj.value, expectedHeight, - "Wrong height of text between offsets (" + aStartOffset + ", " + - aEndOffset + ") for " + prettyName(aID)); + // Height + isWithin(heightObj.value, expectedHeight, 1, + `height of text between offsets (${aStartOffset}, ${aEndOffset}) ` + + `for ${prettyName(aID)}`); } /** From 7be1f3428f8661125feee5ef598a76a51a77ffb9 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 16:09:58 -0700 Subject: [PATCH 24/42] Bug 1398630: Follow-up: Fix typo. r=me MozReview-Commit-ID: 7Wv2WPWRC4L --- toolkit/components/extensions/ExtensionUtils.jsm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/components/extensions/ExtensionUtils.jsm b/toolkit/components/extensions/ExtensionUtils.jsm index dbfa3f2127f9..a3ed895de6df 100644 --- a/toolkit/components/extensions/ExtensionUtils.jsm +++ b/toolkit/components/extensions/ExtensionUtils.jsm @@ -157,7 +157,7 @@ class EventEmitter { * The listener function to remove. */ off(event, listener) { - let set = this.listeners.get(event); + let set = this[LISTENERS].get(event); if (set) { set.delete(listener); set.delete(this[ONCE_MAP].get(listener)); From fc4fec2e843649f7927703c60c068361c3930aaa Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 17:03:20 -0700 Subject: [PATCH 25/42] Bug 1398630: Follow-up: Fix another typo. r=me MozReview-Commit-ID: HmaqWzfLVGa --- toolkit/components/extensions/ext-cookies.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/components/extensions/ext-cookies.js b/toolkit/components/extensions/ext-cookies.js index 9788f605034d..cd9b5c975a35 100644 --- a/toolkit/components/extensions/ext-cookies.js +++ b/toolkit/components/extensions/ext-cookies.js @@ -224,7 +224,7 @@ const query = function* (detailsIn, props, context) { return false; } - if (!pathMatches(url.path)) { + if (!pathMatches(url.pathname)) { return false; } } From 3b6f5b7b0cc2f693bac4e2404ffea666d843dce5 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 17:05:11 -0700 Subject: [PATCH 26/42] Bug 1391707: Follow-up: Update DeferredTask tests not to use generator functions. r=me MozReview-Commit-ID: 8ertwxW0rRv --- toolkit/modules/tests/xpcshell/test_DeferredTask.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/toolkit/modules/tests/xpcshell/test_DeferredTask.js b/toolkit/modules/tests/xpcshell/test_DeferredTask.js index 528e91c95819..c46dd468b9c7 100644 --- a/toolkit/modules/tests/xpcshell/test_DeferredTask.js +++ b/toolkit/modules/tests/xpcshell/test_DeferredTask.js @@ -191,18 +191,6 @@ add_test(function test_arm_async_function() { deferredTask.arm(); }); -/** - * Checks that "arm" accepts a Task.jsm generator function. - */ -add_test(function test_arm_async_generator() { - let deferredTask = new DeferredTask(function* () { - yield Promise.resolve(); - run_next_test(); - }, 50); - - deferredTask.arm(); -}); - /** * Checks that an armed task can be disarmed. */ From b58cc3cfd54fb0c78ad14eb891d0c42078583b89 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Mon, 11 Sep 2017 19:39:48 -0400 Subject: [PATCH 27/42] Bug 1398890 - Use 8 chunks for Windows reftest-gpu. r=jmaher --- taskcluster/ci/test/tests.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/taskcluster/ci/test/tests.yml b/taskcluster/ci/test/tests.yml index 984656bc4141..cd3a1e73d642 100644 --- a/taskcluster/ci/test/tests.yml +++ b/taskcluster/ci/test/tests.yml @@ -1147,6 +1147,11 @@ reftest-gpu: description: "Reftest GPU run" suite: reftest/reftest-gpu treeherder-symbol: tc-R(Rg) + chunks: + by-test-platform: + # Remove special casing when debug isn't using BBB anymore + windows7-32.*/debug: 1 + default: 8 run-on-projects: by-test-platform: windows10.*: [] @@ -1162,7 +1167,11 @@ reftest-gpu: mozharness: script: desktop_unittest.py no-read-buildbot-config: true - chunked: false + chunked: + # Remove special casing when debug isn't using BBB anymore + by-test-platform: + windows7-32.*/debug: false + default: true config: by-test-platform: windows.*: From 4d2609641098f5a6069c1d3e2e6c4d0273563b0a Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Mon, 11 Sep 2017 20:25:16 -0400 Subject: [PATCH 28/42] Bug 1386410 - Re-enable browser_toolbox_custom_host.js now that the new console frontend is riding the trains. r=bgrins --- devtools/client/framework/test/browser.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/devtools/client/framework/test/browser.ini b/devtools/client/framework/test/browser.ini index 5bb2631169cf..72befdd79bf6 100644 --- a/devtools/client/framework/test/browser.ini +++ b/devtools/client/framework/test/browser.ini @@ -72,7 +72,6 @@ skip-if = debug # Bug 1282269 [browser_target_remote.js] [browser_target_support.js] [browser_toolbox_custom_host.js] -skip-if = true # Bug 1386410 [browser_toolbox_dynamic_registration.js] [browser_toolbox_getpanelwhenready.js] [browser_toolbox_highlight.js] From 0493373f1b9165eec9533223875687b31b28ba24 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 18:43:35 -0700 Subject: [PATCH 29/42] Bug 1391707: Follow-up: Skip idle dispatch in passwordManager.js for increasing the failure rate of an intermittent. r=me --- toolkit/components/passwordmgr/content/passwordManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/components/passwordmgr/content/passwordManager.js b/toolkit/components/passwordmgr/content/passwordManager.js index d330c3d34f2b..b3b3fecb96cf 100644 --- a/toolkit/components/passwordmgr/content/passwordManager.js +++ b/toolkit/components/passwordmgr/content/passwordManager.js @@ -127,7 +127,7 @@ let signonsTreeView = { // Coalesce invalidations to avoid repeated flickering. _invalidateTask: new DeferredTask(() => { signonsTree.treeBoxObject.invalidateColumn(signonsTree.columns.siteCol); - }, 10), + }, 10, 0), _lastSelectedRanges: [], selection: null, From 4be75fe268ae0a44f64a4ffe7fb953e1ec2bd88d Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 18:48:53 -0700 Subject: [PATCH 30/42] Bug 1391707: Follow-up: Skip idle dispatch during tests in TelemetrySession.jsm for increasing the failure rate of an intermittent. r=me MozReview-Commit-ID: CT6qLG0edG9 --- toolkit/components/telemetry/TelemetrySession.jsm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index 2d22d0477c77..86655677490c 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -1541,7 +1541,8 @@ var Impl = { if (Telemetry.canRecordExtended) { GCTelemetry.init(); } - }, testing ? TELEMETRY_TEST_DELAY : TELEMETRY_DELAY); + }, testing ? TELEMETRY_TEST_DELAY : TELEMETRY_DELAY, + testing ? 0 : undefined); delayedTask.arm(); }, From c928dfcad0609f45127d4ffa55c758f2e9beeda2 Mon Sep 17 00:00:00 2001 From: Bryce Van Dyk Date: Tue, 22 Aug 2017 15:28:00 +1200 Subject: [PATCH 31/42] Bug 1378826 - Add test for removal of video tracks during recording. r=jesup MozReview-Commit-ID: 7IGx27Z2jsN --- dom/media/test/crashtests/1378826.html | 46 +++++++++++++++++++++++ dom/media/test/crashtests/crashtests.list | 1 + 2 files changed, 47 insertions(+) create mode 100644 dom/media/test/crashtests/1378826.html diff --git a/dom/media/test/crashtests/1378826.html b/dom/media/test/crashtests/1378826.html new file mode 100644 index 000000000000..e1913cd0f5df --- /dev/null +++ b/dom/media/test/crashtests/1378826.html @@ -0,0 +1,46 @@ + + + +Bug 1378826 : Removing last video track from recorder stream crashes. + + + + + + diff --git a/dom/media/test/crashtests/crashtests.list b/dom/media/test/crashtests/crashtests.list index 862fc08958a9..b93bc63e0e6b 100644 --- a/dom/media/test/crashtests/crashtests.list +++ b/dom/media/test/crashtests/crashtests.list @@ -85,6 +85,7 @@ load 1304948.html load 1319486.html load 1368490.html load 1291702.html +load 1378826.html load 1384248.html load disconnect-wrong-destination.html load analyser-channels-1.html From 296a4c6c273050d01f4734f587b97b19b95e067a Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Fri, 8 Sep 2017 13:57:29 -0700 Subject: [PATCH 32/42] Bug 1398417 - Fix PrioritizedEventQueue bugs with input event prioritization (r=stone) MozReview-Commit-ID: 4wk8EUv0h7C --- xpcom/threads/PrioritizedEventQueue.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xpcom/threads/PrioritizedEventQueue.cpp b/xpcom/threads/PrioritizedEventQueue.cpp index d28a542954fe..004e72b9ee5b 100644 --- a/xpcom/threads/PrioritizedEventQueue.cpp +++ b/xpcom/threads/PrioritizedEventQueue.cpp @@ -122,7 +122,7 @@ PrioritizedEventQueue::SelectQueue(bool aUpdateState, bool normalPending = !mNormalQueue->IsEmpty(aProofOfLock); size_t inputCount = mInputQueue->Count(aProofOfLock); - if (aUpdateState && mInputQueueState == STATE_ENABLED && + if (mInputQueueState == STATE_ENABLED && mInputHandlingStartTime.IsNull() && inputCount > 0) { mInputHandlingStartTime = InputEventStatistics::Get() @@ -274,7 +274,7 @@ PrioritizedEventQueue::HasReadyEvent(const MutexAutoLock& aProofOfL if (queue == EventPriority::High) { return mHighQueue->HasReadyEvent(aProofOfLock); } else if (queue == EventPriority::Input) { - return mIdleQueue->HasReadyEvent(aProofOfLock); + return mInputQueue->HasReadyEvent(aProofOfLock); } else if (queue == EventPriority::Normal) { return mNormalQueue->HasReadyEvent(aProofOfLock); } From b2c69d7dce0d5a2b3bc39e4ac7aeaec833f2f7b5 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Thu, 7 Sep 2017 15:32:25 -0700 Subject: [PATCH 33/42] Bug 1397941 - Fix DecodedStream labeling bug (r=jwwang) MozReview-Commit-ID: KL5XzLJuWgz --- dom/media/mediasink/DecodedStream.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/media/mediasink/DecodedStream.cpp b/dom/media/mediasink/DecodedStream.cpp index 5244a087c77e..7ad1c3ab713f 100644 --- a/dom/media/mediasink/DecodedStream.cpp +++ b/dom/media/mediasink/DecodedStream.cpp @@ -412,7 +412,7 @@ DecodedStream::DestroyData(UniquePtr aData) data->Forget(); nsCOMPtr r = NS_NewRunnableFunction("DecodedStream::DestroyData", [=]() { delete data; }); - mAbstractMainThread->Dispatch(r.forget()); + NS_DispatchToMainThread(r.forget()); } void From 862ed98fd3c8dbcf7eff79f2e7d0a8e7504cd9eb Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Fri, 8 Sep 2017 15:54:39 -0700 Subject: [PATCH 34/42] Bug 1398423 - Make linked list of SchedulerGroups static (r=froydnj) MozReview-Commit-ID: GgfdRlhPiHP --- xpcom/threads/LabeledEventQueue.cpp | 92 +++++++++++++++++++++++------ xpcom/threads/LabeledEventQueue.h | 12 +++- xpcom/threads/SchedulerGroup.h | 34 +++++++++++ 3 files changed, 119 insertions(+), 19 deletions(-) diff --git a/xpcom/threads/LabeledEventQueue.cpp b/xpcom/threads/LabeledEventQueue.cpp index 63f15d22a2c4..be246253eab8 100644 --- a/xpcom/threads/LabeledEventQueue.cpp +++ b/xpcom/threads/LabeledEventQueue.cpp @@ -13,8 +13,28 @@ using namespace mozilla::dom; +LinkedList* LabeledEventQueue::sSchedulerGroups; +size_t LabeledEventQueue::sLabeledEventQueueCount; +SchedulerGroup* LabeledEventQueue::sCurrentSchedulerGroup; + LabeledEventQueue::LabeledEventQueue() { + // LabeledEventQueue should only be used by one consumer since it uses a + // single static sSchedulerGroups field. It's hard to assert this, though, so + // we assert NS_IsMainThread(), which is a reasonable proxy. + MOZ_ASSERT(NS_IsMainThread()); + + if (sLabeledEventQueueCount++ == 0) { + sSchedulerGroups = new LinkedList(); + } +} + +LabeledEventQueue::~LabeledEventQueue() +{ + if (--sLabeledEventQueueCount == 0) { + delete sSchedulerGroups; + sSchedulerGroups = nullptr; + } } static SchedulerGroup* @@ -94,8 +114,14 @@ LabeledEventQueue::PutEvent(already_AddRefed&& aEvent, RunnableEpochQueue* queue = isLabeled ? mLabeled.LookupOrAdd(group) : &mUnlabeled; queue->Push(QueueEntry(event.forget(), epoch->mEpochNumber)); - if (group && !group->isInList()) { - mSchedulerGroups.insertBack(group); + if (group && group->EnqueueEvent() == SchedulerGroup::NewlyQueued) { + // This group didn't have any events before. Add it to the + // sSchedulerGroups list. + MOZ_ASSERT(!group->isInList()); + sSchedulerGroups->insertBack(group); + if (!sCurrentSchedulerGroup) { + sCurrentSchedulerGroup = group; + } } } @@ -113,6 +139,18 @@ LabeledEventQueue::PopEpoch() mNumEvents--; } +// Returns the next SchedulerGroup after |aGroup| in sSchedulerGroups. Wraps +// around to the beginning of the list when we hit the end. +/* static */ SchedulerGroup* +LabeledEventQueue::NextSchedulerGroup(SchedulerGroup* aGroup) +{ + SchedulerGroup* result = aGroup->getNext(); + if (!result) { + result = sSchedulerGroups->getFirst(); + } + return result; +} + already_AddRefed LabeledEventQueue::GetEvent(EventPriority* aPriority, const MutexAutoLock& aProofOfLock) @@ -133,13 +171,17 @@ LabeledEventQueue::GetEvent(EventPriority* aPriority, } } + if (!sCurrentSchedulerGroup) { + return nullptr; + } + // Move active tabs to the front of the queue. The mAvoidActiveTabCount field // prevents us from preferentially processing events from active tabs twice in // a row. This scheme is designed to prevent starvation. if (TabChild::HasActiveTabs() && mAvoidActiveTabCount <= 0) { for (TabChild* tabChild : TabChild::GetActiveTabs()) { SchedulerGroup* group = tabChild->TabGroup(); - if (!group->isInList() || group == mSchedulerGroups.getFirst()) { + if (!group->isInList() || group == sCurrentSchedulerGroup) { continue; } @@ -148,40 +190,56 @@ LabeledEventQueue::GetEvent(EventPriority* aPriority, // a background group) before we prioritize active tabs again. mAvoidActiveTabCount += 2; - group->removeFrom(mSchedulerGroups); - mSchedulerGroups.insertFront(group); + // We move |group| right before sCurrentSchedulerGroup and then set + // sCurrentSchedulerGroup to group. + MOZ_ASSERT(group != sCurrentSchedulerGroup); + group->removeFrom(*sSchedulerGroups); + sCurrentSchedulerGroup->setPrevious(group); + sCurrentSchedulerGroup = group; } } - // Iterate over SchedulerGroups in order. Each time we pass by a - // SchedulerGroup, we move it to the back of the list. This ensures that we - // process SchedulerGroups in a round-robin order (ignoring active tab - // prioritization). - SchedulerGroup* firstGroup = mSchedulerGroups.getFirst(); + // Iterate over each SchedulerGroup once, starting at sCurrentSchedulerGroup. + SchedulerGroup* firstGroup = sCurrentSchedulerGroup; SchedulerGroup* group = firstGroup; do { + mAvoidActiveTabCount--; + RunnableEpochQueue* queue = mLabeled.Get(group); - MOZ_ASSERT(queue); + if (!queue) { + // This can happen if |group| is in a different LabeledEventQueue than |this|. + group = NextSchedulerGroup(group); + continue; + } MOZ_ASSERT(!queue->IsEmpty()); - mAvoidActiveTabCount--; - SchedulerGroup* next = group->removeAndGetNext(); - mSchedulerGroups.insertBack(group); - QueueEntry entry = queue->FirstElement(); if (entry.mEpochNumber == epoch.mEpochNumber && IsReadyToRun(entry.mRunnable, group)) { + sCurrentSchedulerGroup = NextSchedulerGroup(group); + PopEpoch(); + if (group->DequeueEvent() == SchedulerGroup::NoLongerQueued) { + // Now we can take group out of sSchedulerGroups. + if (sCurrentSchedulerGroup == group) { + // Since we changed sCurrentSchedulerGroup above, we'll only get here + // if |group| was the only element in sSchedulerGroups. In that case + // set sCurrentSchedulerGroup to null. + MOZ_ASSERT(group->getNext() == nullptr); + MOZ_ASSERT(group->getPrevious() == nullptr); + sCurrentSchedulerGroup = nullptr; + } + group->removeFrom(*sSchedulerGroups); + } queue->Pop(); if (queue->IsEmpty()) { mLabeled.Remove(group); - group->removeFrom(mSchedulerGroups); } return entry.mRunnable.forget(); } - group = next; + group = NextSchedulerGroup(group); } while (group != firstGroup); return nullptr; diff --git a/xpcom/threads/LabeledEventQueue.h b/xpcom/threads/LabeledEventQueue.h index 89780a483dbc..1ad819162789 100644 --- a/xpcom/threads/LabeledEventQueue.h +++ b/xpcom/threads/LabeledEventQueue.h @@ -30,6 +30,7 @@ class LabeledEventQueue final : public AbstractEventQueue { public: LabeledEventQueue(); + ~LabeledEventQueue(); void PutEvent(already_AddRefed&& aEvent, EventPriority aPriority, @@ -127,13 +128,20 @@ class LabeledEventQueue final : public AbstractEventQueue }; void PopEpoch(); + static SchedulerGroup* NextSchedulerGroup(SchedulerGroup* aGroup); using RunnableEpochQueue = Queue; using LabeledMap = nsClassHashtable, RunnableEpochQueue>; using EpochQueue = Queue; - // List of SchedulerGroups that have events in the queue. - LinkedList mSchedulerGroups; + // List of SchedulerGroups that might have events. This is static, so it + // covers all LabeledEventQueues. If a SchedulerGroup is in this list, it may + // not have an event in *this* LabeledEventQueue (although it will have an + // event in *some* LabeledEventQueue). sCurrentSchedulerGroup cycles through + // the elements of sSchedulerGroups in order. + static LinkedList* sSchedulerGroups; + static size_t sLabeledEventQueueCount; + static SchedulerGroup* sCurrentSchedulerGroup; LabeledMap mLabeled; RunnableEpochQueue mUnlabeled; diff --git a/xpcom/threads/SchedulerGroup.h b/xpcom/threads/SchedulerGroup.h index 56bdb4bf5896..387ce0c18368 100644 --- a/xpcom/threads/SchedulerGroup.h +++ b/xpcom/threads/SchedulerGroup.h @@ -75,6 +75,36 @@ class SchedulerGroup : public LinkedListElement MOZ_ASSERT(IsSafeToRun()); } + enum EnqueueStatus + { + NewlyQueued, + AlreadyQueued, + }; + + // Records that this SchedulerGroup had an event enqueued in some + // queue. Returns whether the SchedulerGroup was already in a queue before + // EnqueueEvent() was called. + EnqueueStatus EnqueueEvent() + { + mEventCount++; + return mEventCount == 1 ? NewlyQueued : AlreadyQueued; + } + + enum DequeueStatus + { + StillQueued, + NoLongerQueued, + }; + + // Records that this SchedulerGroup had an event dequeued from some + // queue. Returns whether the SchedulerGroup is still in a queue after + // DequeueEvent() returns. + DequeueStatus DequeueEvent() + { + mEventCount--; + return mEventCount == 0 ? NoLongerQueued : StillQueued; + } + class Runnable final : public mozilla::Runnable , public nsIRunnablePriority , public nsILabelableRunnable @@ -167,6 +197,10 @@ class SchedulerGroup : public LinkedListElement bool mIsRunning; + // Number of events that are currently enqueued for this SchedulerGroup + // (across all queues). + size_t mEventCount = 0; + nsCOMPtr mEventTargets[size_t(TaskCategory::Count)]; RefPtr mAbstractThreads[size_t(TaskCategory::Count)]; }; From bfb4e5933a3f8858d81351572cb14c4c42cb2982 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Fri, 8 Sep 2017 15:55:06 -0700 Subject: [PATCH 35/42] Bug 1398423 - Fix LabeledEventQueue bug with unlabeled events (r=froydnj) MozReview-Commit-ID: 7ru62QTkya2 --- xpcom/threads/LabeledEventQueue.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/xpcom/threads/LabeledEventQueue.cpp b/xpcom/threads/LabeledEventQueue.cpp index be246253eab8..7d90ebd0b675 100644 --- a/xpcom/threads/LabeledEventQueue.cpp +++ b/xpcom/threads/LabeledEventQueue.cpp @@ -162,13 +162,15 @@ LabeledEventQueue::GetEvent(EventPriority* aPriority, Epoch epoch = mEpochs.FirstElement(); if (!epoch.IsLabeled()) { QueueEntry entry = mUnlabeled.FirstElement(); - if (IsReadyToRun(entry.mRunnable, nullptr)) { - PopEpoch(); - mUnlabeled.Pop(); - MOZ_ASSERT(entry.mEpochNumber == epoch.mEpochNumber); - MOZ_ASSERT(entry.mRunnable.get()); - return entry.mRunnable.forget(); + if (!IsReadyToRun(entry.mRunnable, nullptr)) { + return nullptr; } + + PopEpoch(); + mUnlabeled.Pop(); + MOZ_ASSERT(entry.mEpochNumber == epoch.mEpochNumber); + MOZ_ASSERT(entry.mRunnable.get()); + return entry.mRunnable.forget(); } if (!sCurrentSchedulerGroup) { From f0f04892fa476ceb24a9bb2f91637330787c7941 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Fri, 8 Sep 2017 21:10:22 -0700 Subject: [PATCH 36/42] Bug 1398420 - Don't use SystemGroup for CookieServiceChild (r=jdm) I noticed a bug where the following can happen. The parent sends a TrackCookiesLoad message followed by an HTTP OnStartRequest message. When these messages are received in the child, the TrackCookiesLoad message goes in the SystemGroup event queue and the OnStartRequest message goes in the event queue for the relevant tab. Unfortunately, this means that the OnStartRequest message could run first since the queues have no guaranteed ordering. We really should be putting the TrackCookiesLoad message in the same queue that the OnStartRequest message goes in. I worked on that a little bit, but it's hard to get right. For now, I would like to leave the cookie message unlabeled. Any unlabeled message/event is totally ordered with respect to all other messages/events, so this fixes the bug. MozReview-Commit-ID: KiLDAhlrbB8 --- netwerk/cookie/CookieServiceChild.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/netwerk/cookie/CookieServiceChild.cpp b/netwerk/cookie/CookieServiceChild.cpp index 9c222ae0a4dc..a3127a546386 100644 --- a/netwerk/cookie/CookieServiceChild.cpp +++ b/netwerk/cookie/CookieServiceChild.cpp @@ -75,10 +75,6 @@ CookieServiceChild::CookieServiceChild() NeckoChild::InitNeckoChild(); - gNeckoChild->SetEventTargetForActor( - this, - SystemGroup::EventTargetFor(TaskCategory::Other)); - // Create a child PCookieService actor. gNeckoChild->SendPCookieServiceConstructor(this); From 74739b7dfd40d2d1c7107cdb15ae9c8d90abc89e Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Fri, 1 Sep 2017 16:31:39 -0700 Subject: [PATCH 37/42] Bug 1398423 - Enable picking events from multiple labeled event queues (r=froydnj) MozReview-Commit-ID: AsfFUsQjMAV --- modules/libpref/init/all.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index b0e01b2a253a..f3718dc09215 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -3343,7 +3343,7 @@ pref("dom.ipc.useNativeEventProcessing.content", false); // Quantum DOM scheduling: pref("dom.ipc.scheduler", false); -pref("dom.ipc.scheduler.useMultipleQueues", false); +pref("dom.ipc.scheduler.useMultipleQueues", true); pref("dom.ipc.scheduler.preemption", false); pref("dom.ipc.scheduler.threadCount", 2); pref("dom.ipc.scheduler.chaoticScheduling", false); From 82977d1e05187f8da07009c9c1387b57070f9789 Mon Sep 17 00:00:00 2001 From: Stone Shih Date: Fri, 8 Sep 2017 10:21:09 +0800 Subject: [PATCH 38/42] Bug 1397461: Fire coalesced mousemove event when the event button or buttons changes. r=smaug. --- dom/ipc/CoalescedMouseData.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dom/ipc/CoalescedMouseData.cpp b/dom/ipc/CoalescedMouseData.cpp index a8acc8341317..7f28552143f0 100644 --- a/dom/ipc/CoalescedMouseData.cpp +++ b/dom/ipc/CoalescedMouseData.cpp @@ -26,9 +26,6 @@ CoalescedMouseData::Coalesce(const WidgetMouseEvent& aEvent, MOZ_ASSERT(mCoalescedInputEvent->mModifiers == aEvent.mModifiers); MOZ_ASSERT(mCoalescedInputEvent->mReason == aEvent.mReason); MOZ_ASSERT(mCoalescedInputEvent->inputSource == aEvent.inputSource); - - // Assuming button changes should trigger other mouse events and dispatch - // the coalesced mouse move events. MOZ_ASSERT(mCoalescedInputEvent->button == aEvent.button); MOZ_ASSERT(mCoalescedInputEvent->buttons == aEvent.buttons); mCoalescedInputEvent->mTimeStamp = aEvent.mTimeStamp; @@ -47,6 +44,8 @@ CoalescedMouseData::CanCoalesce(const WidgetMouseEvent& aEvent, (mCoalescedInputEvent->mModifiers == aEvent.mModifiers && mCoalescedInputEvent->inputSource == aEvent.inputSource && mCoalescedInputEvent->pointerId == aEvent.pointerId && + mCoalescedInputEvent->button == aEvent.button && + mCoalescedInputEvent->buttons == aEvent.buttons && mGuid == aGuid && mInputBlockId == aInputBlockId); } From 33258e191b992215b16f0767da8e5f1ce9ea32ae Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 21:18:42 -0700 Subject: [PATCH 39/42] Bug 1398642: Follow-up: Fix terrible error checking code. r=me CLOSED TREE MozReview-Commit-ID: 1xQNYwZiqsj --- .../extensions/test/mochitest/chrome_cleanup_script.js | 9 +++++++++ toolkit/modules/addons/WebRequest.jsm | 2 ++ 2 files changed, 11 insertions(+) diff --git a/toolkit/components/extensions/test/mochitest/chrome_cleanup_script.js b/toolkit/components/extensions/test/mochitest/chrome_cleanup_script.js index 8a914d5de1c0..a092dd460e32 100644 --- a/toolkit/components/extensions/test/mochitest/chrome_cleanup_script.js +++ b/toolkit/components/extensions/test/mochitest/chrome_cleanup_script.js @@ -5,6 +5,13 @@ Components.utils.import("resource://gre/modules/AppConstants.jsm"); Components.utils.import("resource://gre/modules/Services.jsm"); +let listener = msg => { + void (msg instanceof Components.interfaces.nsIConsoleMessage); + dump(`Console message: ${msg}\n`); +}; + +Services.console.registerListener(listener); + let getBrowserApp, getTabBrowser; if (AppConstants.MOZ_BUILD_APP === "mobile/android") { getBrowserApp = win => win.BrowserApp; @@ -30,6 +37,8 @@ for (let win of iterBrowserWindows()) { } addMessageListener("check-cleanup", extensionId => { + Services.console.unregisterListener(listener); + let results = { extraWindows: [], extraTabs: [], diff --git a/toolkit/modules/addons/WebRequest.jsm b/toolkit/modules/addons/WebRequest.jsm index d41f02d84052..9bc8aba74754 100644 --- a/toolkit/modules/addons/WebRequest.jsm +++ b/toolkit/modules/addons/WebRequest.jsm @@ -898,6 +898,8 @@ HttpObserverManager = { if (kind === "modify" && this.listeners.afterModify.size) { await this.runChannelListener(channel, "afterModify"); + } else { + this.errorCheck(channel); } } catch (e) { Cu.reportError(e); From 19550ec70eb7141072cf577bae8ce3e8a28907dc Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 21:26:04 -0700 Subject: [PATCH 40/42] Bug 1391707: Follow-up: Skip idle in more places that incorrectly expect strict timing. r=me CLOSED TREE MozReview-Commit-ID: BbMB5qk4F4e --- browser/components/customizableui/CustomizableUI.jsm | 2 +- devtools/server/performance/memory.js | 2 +- toolkit/components/telemetry/TelemetryController.jsm | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/browser/components/customizableui/CustomizableUI.jsm b/browser/components/customizableui/CustomizableUI.jsm index 774d49d600c8..2cc1f0b188d8 100644 --- a/browser/components/customizableui/CustomizableUI.jsm +++ b/browser/components/customizableui/CustomizableUI.jsm @@ -4353,7 +4353,7 @@ OverflowableToolbar.prototype = { _onResize(aEvent) { if (!this._lazyResizeHandler) { this._lazyResizeHandler = new DeferredTask(this._onLazyResize.bind(this), - LAZY_RESIZE_INTERVAL_MS); + LAZY_RESIZE_INTERVAL_MS, 0); } this._lazyResizeHandler.arm(); }, diff --git a/devtools/server/performance/memory.js b/devtools/server/performance/memory.js index 3469b6ac5cb2..dc4ccd047d29 100644 --- a/devtools/server/performance/memory.js +++ b/devtools/server/performance/memory.js @@ -195,7 +195,7 @@ Memory.prototype = { this._poller.disarm(); } this._poller = new DeferredTask(this._emitAllocations, - this.drainAllocationsTimeoutTimer); + this.drainAllocationsTimeoutTimer, 0); this._poller.arm(); } diff --git a/toolkit/components/telemetry/TelemetryController.jsm b/toolkit/components/telemetry/TelemetryController.jsm index 69822759ed1f..a5e68aead8aa 100644 --- a/toolkit/components/telemetry/TelemetryController.jsm +++ b/toolkit/components/telemetry/TelemetryController.jsm @@ -735,7 +735,8 @@ var Impl = { } finally { this._delayedInitTask = null; } - }, this._testMode ? TELEMETRY_TEST_DELAY : TELEMETRY_DELAY); + }, this._testMode ? TELEMETRY_TEST_DELAY : TELEMETRY_DELAY, + this._testMode ? 0 : undefined); AsyncShutdown.sendTelemetry.addBlocker("TelemetryController: shutting down", () => this.shutdown(), From bd7cddf6112eef9cd9e62de32248aa438cfb43db Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 11 Sep 2017 21:59:15 -0700 Subject: [PATCH 41/42] Bug 1398642: Bustage fix bustage fix. r=me CLOSED TREE MozReview-Commit-ID: 4jTSVg6L3is --- toolkit/modules/addons/WebRequest.jsm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/modules/addons/WebRequest.jsm b/toolkit/modules/addons/WebRequest.jsm index 9bc8aba74754..eac4ab53390d 100644 --- a/toolkit/modules/addons/WebRequest.jsm +++ b/toolkit/modules/addons/WebRequest.jsm @@ -898,7 +898,7 @@ HttpObserverManager = { if (kind === "modify" && this.listeners.afterModify.size) { await this.runChannelListener(channel, "afterModify"); - } else { + } else if (kind !== "onError") { this.errorCheck(channel); } } catch (e) { From 67b30242000cd5313cecbbaa9a28b4eaaeb9d141 Mon Sep 17 00:00:00 2001 From: Bob Owen Date: Tue, 12 Sep 2017 07:53:52 +0100 Subject: [PATCH 42/42] Bug 1395952: Enhance telemetry for failed launch of Windows sandboxed process by process type/error code key. r=jimm, data-r=rweiss Only one telemetry accumlation will occur for each key per session. --- ipc/glue/GeckoChildProcessHost.cpp | 1 + .../win/src/sandboxbroker/sandboxBroker.cpp | 23 ++++++++++++++++++- .../win/src/sandboxbroker/sandboxBroker.h | 2 ++ toolkit/components/telemetry/Histograms.json | 7 +++--- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp index c4ca5616a57e..19539daade56 100644 --- a/ipc/glue/GeckoChildProcessHost.cpp +++ b/ipc/glue/GeckoChildProcessHost.cpp @@ -1119,6 +1119,7 @@ GeckoChildProcessHost::PerformAsyncLaunchInternal(std::vector& aExt if (shouldSandboxCurrentProcess) { if (mSandboxBroker.LaunchApp(cmdLine.program().c_str(), cmdLine.command_line_string().c_str(), + mProcessType, mEnableSandboxLogging, &process)) { EnvironmentLog("MOZ_PROCESS_LOG").print( diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp index fc3d1448b09d..f77077a57ecb 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp +++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp @@ -23,6 +23,7 @@ #include "nsIProperties.h" #include "nsServiceManagerUtils.h" #include "nsString.h" +#include "nsTHashtable.h" #include "sandbox/win/src/sandbox.h" #include "sandbox/win/src/security_level.h" #include "WinUtils.h" @@ -50,6 +51,9 @@ static LazyLogModule sSandboxBrokerLog("SandboxBroker"); #define LOG_E(...) MOZ_LOG(sSandboxBrokerLog, LogLevel::Error, (__VA_ARGS__)) #define LOG_W(...) MOZ_LOG(sSandboxBrokerLog, LogLevel::Warning, (__VA_ARGS__)) +// Used to store whether we have accumulated an error combination for this session. +static UniquePtr> sLaunchErrors; + /* static */ void SandboxBroker::Initialize(sandbox::BrokerServices* aBrokerServices) @@ -135,6 +139,7 @@ SandboxBroker::SandboxBroker() bool SandboxBroker::LaunchApp(const wchar_t *aPath, const wchar_t *aArguments, + GeckoProcessType aProcessType, const bool aEnableLogging, void **aProcessHandle) { @@ -206,9 +211,25 @@ SandboxBroker::LaunchApp(const wchar_t *aPath, result = sBrokerService->SpawnTarget(aPath, aArguments, mPolicy, &last_warning, &last_error, &targetInfo); if (sandbox::SBOX_ALL_OK != result) { - Telemetry::Accumulate(Telemetry::SANDBOX_FAILED_LAUNCH, result); + nsAutoCString key; + key.AppendASCII(XRE_ChildProcessTypeToString(aProcessType)); + key.AppendLiteral("/0x"); + key.AppendInt(static_cast(last_error), 16); + + if (!sLaunchErrors) { + sLaunchErrors = MakeUnique>(); + ClearOnShutdown(&sLaunchErrors); + } + + // Only accumulate for each combination once per session. + if (!sLaunchErrors->Contains(key)) { + Telemetry::Accumulate(Telemetry::SANDBOX_FAILED_LAUNCH_KEYED, key, result); + sLaunchErrors->PutEntry(key); + } + LOG_E("Failed (ResultCode %d) to SpawnTarget with last_error=%d, last_warning=%d", result, last_error, last_warning); + return false; } else if (sandbox::SBOX_ALL_OK != last_warning) { // If there was a warning (but the result was still ok), log it and proceed. diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h index 9a224438173b..05e807c164fe 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h +++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h @@ -11,6 +11,7 @@ #include #include "base/child_privileges.h" +#include "nsXULAppAPI.h" namespace sandbox { class BrokerServices; @@ -34,6 +35,7 @@ class SandboxBroker bool LaunchApp(const wchar_t *aPath, const wchar_t *aArguments, + GeckoProcessType aProcessType, const bool aEnableLogging, void **aProcessHandle); virtual ~SandboxBroker(); diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 0d72dd159ef6..7c337e2453b5 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -12590,15 +12590,16 @@ "cpp_guard": "XP_LINUX", "description": "System calls blocked by a seccomp-bpf sandbox policy; limited to syscalls where we would crash on Nightly. The key is generally the architecture and syscall ID but in some cases we include non-personally-identifying information from the syscall arguments; see the function SubmitToTelemetry in security/sandbox/linux/reporter/SandboxReporter.cpp for details." }, - "SANDBOX_FAILED_LAUNCH": { + "SANDBOX_FAILED_LAUNCH_KEYED": { "record_in_processes": ["main"], "alert_emails": ["bowen@mozilla.com"], - "expires_in_version": "60", + "expires_in_version": "never", "kind": "enumerated", + "keyed": true, "n_values": 50, "bug_numbers": [1368600], "cpp_guard": "XP_WIN", - "description": "Error code when a Windows sandboxed process fails to launch. See https://dxr.mozilla.org/mozilla-central/search?q=ResultCode++path%3Asandbox_types.h&redirect=true for definitions of the error codes." + "description": "Error code when a Windows sandboxed process fails to launch, keyed by process type and Windows error code. See https://dxr.mozilla.org/mozilla-central/search?q=ResultCode++path%3Asandbox_types.h&redirect=true for definitions of the error codes." }, "SYNC_WORKER_OPERATION": { "record_in_processes": ["main", "content"],