Backed out changeset f45996fe15b2 (bug 1618547) for causing failures in browser_ProcessPriorityManager.js CLOSED TREE

This commit is contained in:
Noemi Erli 2021-05-06 00:53:54 +03:00
parent 141d788226
commit b7da3f34ee
11 changed files with 115 additions and 347 deletions

View File

@ -223,11 +223,6 @@ void CanonicalBrowsingContext::ReplacedBy(
BackgroundSessionStorageManager::PropagateManager(Id(), aNewContext->Id());
}
// Transfer the ownership of the priority active status from the old context
// to the new context.
aNewContext->mPriorityActive = mPriorityActive;
mPriorityActive = false;
MOZ_ASSERT(aNewContext->mLoadingEntries.IsEmpty());
mLoadingEntries.SwapElements(aNewContext->mLoadingEntries);
MOZ_ASSERT(!aNewContext->mActiveEntry);

View File

@ -302,17 +302,6 @@ class CanonicalBrowsingContext final : public BrowsingContext {
bool AllowedInBFCache(const Maybe<uint64_t>& aChannelId);
// Methods for getting and setting the active state for top level
// browsing contexts, for the process priority manager.
bool IsPriorityActive() const {
MOZ_RELEASE_ASSERT(IsTop());
return mPriorityActive;
}
void SetPriorityActive(bool aIsActive) {
MOZ_RELEASE_ASSERT(IsTop());
mPriorityActive = aIsActive;
}
protected:
// Called when the browsing context is being discarded.
void CanonicalDiscard();
@ -454,10 +443,6 @@ class CanonicalBrowsingContext final : public BrowsingContext {
RefPtr<FeaturePolicy> mContainerFeaturePolicy;
RefPtr<RestoreState> mRestoreState;
// If this is a top level context, this is true if our browser ID is marked as
// active in the process priority manager.
bool mPriorityActive = false;
};
} // namespace dom

View File

@ -10,7 +10,6 @@
#include "mozilla/dom/CancelContentJSOptionsBinding.h"
#include "mozilla/dom/ContentParent.h"
#include "mozilla/dom/WindowGlobalParent.h"
#include "mozilla/ProcessPriorityManager.h"
#include "nsIObserverService.h"
@ -117,8 +116,6 @@ BrowserHost::SetRenderLayers(bool aRenderLayers) {
if (!mRoot) {
return NS_OK;
}
ProcessPriorityManager::ActivityChanged(GetBrowsingContext()->Canonical(),
aRenderLayers);
mRoot->SetRenderLayers(aRenderLayers);
return NS_OK;
}
@ -152,8 +149,8 @@ BrowserHost::Deprioritize(void) {
if (!mRoot) {
return NS_OK;
}
ProcessPriorityManager::ActivityChanged(GetBrowsingContext()->Canonical(),
/* aIsActive = */ false);
VisitAll(
[](BrowserParent* aBrowserParent) { aBrowserParent->Deprioritize(); });
return NS_OK;
}

View File

@ -224,6 +224,7 @@ BrowserParent::BrowserParent(ContentParent* aManager, const TabId& aTabId,
mRemoteTargetSetsCursor(false),
mPreserveLayers(false),
mRenderLayers(true),
mActiveInPriorityManager(false),
mHasLayers(false),
mHasPresented(false),
mIsReadyToHandleInputEvents(false),
@ -233,14 +234,6 @@ BrowserParent::BrowserParent(ContentParent* aManager, const TabId& aTabId,
// When the input event queue is disabled, we don't need to handle the case
// that some input events are dispatched before PBrowserConstructor.
mIsReadyToHandleInputEvents = !ContentParent::IsInputEventQueueSupported();
// If we're in a BC tree that is active with respect to the priority manager,
// ensure that this new BrowserParent is marked as active. This ensures that
// the process will be prioritized in a cross-site iframe navigation in an
// active tab.
if (aBrowsingContext->Top()->IsPriorityActive()) {
ProcessPriorityManager::ActivityChanged(this, true);
}
}
BrowserParent::~BrowserParent() = default;
@ -3384,6 +3377,13 @@ bool BrowserParent::GetHasLayers() { return mHasLayers; }
bool BrowserParent::GetRenderLayers() { return mRenderLayers; }
void BrowserParent::SetRenderLayers(bool aEnabled) {
if (mActiveInPriorityManager != aEnabled) {
mActiveInPriorityManager = aEnabled;
// Let's inform the priority manager. This operation can end up with the
// changing of the process priority.
ProcessPriorityManager::ActivityChanged(this, aEnabled);
}
if (aEnabled == mRenderLayers) {
if (aEnabled && mHasLayers && mPreserveLayers) {
// RenderLayers might be called when we've been preserving layers,
@ -3446,6 +3446,13 @@ void BrowserParent::NotifyResolutionChanged() {
}
}
void BrowserParent::Deprioritize() {
if (mActiveInPriorityManager) {
ProcessPriorityManager::ActivityChanged(this, false);
mActiveInPriorityManager = false;
}
}
bool BrowserParent::StartApzAutoscroll(float aAnchorX, float aAnchorY,
nsViewID aScrollId,
uint32_t aPresShellId) {

View File

@ -702,6 +702,8 @@ class BrowserParent final : public PBrowserParent,
void PreserveLayers(bool aPreserveLayers);
void NotifyResolutionChanged();
void Deprioritize();
bool StartApzAutoscroll(float aAnchorX, float aAnchorY, nsViewID aScrollId,
uint32_t aPresShellId);
void StopApzAutoscroll(nsViewID aScrollId, uint32_t aPresShellId);
@ -967,6 +969,9 @@ class BrowserParent final : public PBrowserParent,
// and have uploaded - for that, use mHasLayers.
bool mRenderLayers : 1;
// Whether this is active for the ProcessPriorityManager or not.
bool mActiveInPriorityManager : 1;
// True if the compositor has reported that the BrowserChild has uploaded
// layers.
bool mHasLayers : 1;

View File

@ -6,7 +6,6 @@
#include "ProcessPriorityManager.h"
#include "mozilla/ClearOnShutdown.h"
#include "mozilla/dom/CanonicalBrowsingContext.h"
#include "mozilla/dom/ContentParent.h"
#include "mozilla/dom/Element.h"
#include "mozilla/dom/BrowserHost.h"
@ -143,7 +142,6 @@ class ProcessPriorityManagerImpl final : public nsIObserver,
ParticularProcessPriorityManager* aParticularManager,
hal::ProcessPriority aOldPriority);
void ActivityChanged(CanonicalBrowsingContext* aBC, bool aIsActive);
void ActivityChanged(BrowserParent* aBrowserParent, bool aIsActive);
void ResetPriority(ContentParent* aContentParent);
@ -446,31 +444,6 @@ void ProcessPriorityManagerImpl::NotifyProcessPriorityChanged(
}
}
void ProcessPriorityManagerImpl::ActivityChanged(
dom::CanonicalBrowsingContext* aBC, bool aIsActive) {
MOZ_ASSERT(aBC->IsTop());
bool alreadyActive = aBC->IsPriorityActive();
if (alreadyActive == aIsActive) {
return;
}
Telemetry::ScalarAdd(
Telemetry::ScalarID::DOM_CONTENTPROCESS_OS_PRIORITY_CHANGE_CONSIDERED, 1);
aBC->SetPriorityActive(aIsActive);
aBC->PreOrderWalk([&](BrowsingContext* aContext) {
CanonicalBrowsingContext* canonical = aContext->Canonical();
if (ContentParent* cp = canonical->GetContentParent()) {
if (RefPtr<ParticularProcessPriorityManager> pppm =
GetParticularProcessPriorityManager(cp)) {
pppm->ActivityChanged(canonical->GetBrowserParent(), aIsActive);
}
}
});
}
void ProcessPriorityManagerImpl::ActivityChanged(BrowserParent* aBrowserParent,
bool aIsActive) {
RefPtr<ParticularProcessPriorityManager> pppm =
@ -884,14 +857,6 @@ bool ProcessPriorityManager::CurrentProcessIsForeground() {
return ProcessPriorityManagerChild::Singleton()->CurrentProcessIsForeground();
}
/* static */
void ProcessPriorityManager::ActivityChanged(CanonicalBrowsingContext* aBC,
bool aIsActive) {
if (auto* singleton = ProcessPriorityManagerImpl::GetSingleton()) {
singleton->ActivityChanged(aBC, aIsActive);
}
}
/* static */
void ProcessPriorityManager::ActivityChanged(BrowserParent* aBrowserParent,
bool aIsActive) {

View File

@ -14,7 +14,6 @@ class nsFrameLoader;
namespace mozilla {
namespace dom {
class BrowserParent;
class CanonicalBrowsingContext;
class ContentParent;
} // namespace dom
@ -71,8 +70,6 @@ class ProcessPriorityManager final {
*/
static bool CurrentProcessIsForeground();
static void ActivityChanged(dom::CanonicalBrowsingContext* aBC,
bool aIsActive);
static void ActivityChanged(dom::BrowserParent* aBrowserParent,
bool aIsActive);

View File

@ -11,11 +11,6 @@ skip-if = !crashreporter
apple_silicon # crash
[browser_ProcessPriorityManager.js]
skip-if = os != "win" # The Process Priority Manager is only enabled for Windows so far. Bug 1522879.
support-files =
file_cross_frame.html
file_dummy.html
../../tests/browser/file_coop_coep.html
../../tests/browser/file_coop_coep.html^headers^
[browser_crash_oopiframe.js]
skip-if = !fission || !crashreporter || verify
apple_silicon # crash

View File

@ -15,11 +15,6 @@ const PROCESS_PRIORITY_BACKGROUND = "BACKGROUND";
// change before we assume that it's just not happening.
const WAIT_FOR_CHANGE_TIME_MS = 2000;
// A convenience function for getting the child ID from a browsing context.
function browsingContextChildID(bc) {
return bc.currentWindowGlobal?.domProcess.childID;
}
/**
* This class is responsible for watching process priority changes, and
* mapping them to tabs in a single window.
@ -44,14 +39,12 @@ class TabPriorityWatcher {
"with a single tab to start."
);
// This maps from childIDs to process priorities.
this.priorityMap = new Map();
// The keys in this map are childIDs we're not expecting to change.
// Each value is either null (if no change has been seen) or the
// priority that the process changed to.
this.noChangeChildIDs = new Map();
this.priorityMap = new WeakMap();
this.priorityMap.set(
this.tabbrowser.selectedBrowser,
PROCESS_PRIORITY_FOREGROUND
);
this.noChangeBrowsers = new WeakMap();
Services.obs.addObserver(this, PRIORITY_SET_TOPIC);
}
@ -62,15 +55,16 @@ class TabPriorityWatcher {
*/
destroy() {
Services.obs.removeObserver(this, PRIORITY_SET_TOPIC);
this.window = null;
}
/**
* This returns a Promise that resolves when the process with
* the given childID reaches the given priority.
* This will eventually time out if that priority is never reached.
* Returns a Promise that resolves when a particular <browser>
* has its content process reach a particular priority. Will
* eventually time out if that priority is never reached.
*
* @param childID
* The childID of the process to wait on.
* @param browser (<browser>)
* The <browser> that we expect to change priority.
* @param expectedPriority (String)
* One of the PROCESS_PRIORITY_ constants defined at the
* top of this file.
@ -78,91 +72,74 @@ class TabPriorityWatcher {
* @resolves undefined
* Once the browser reaches the expected priority.
*/
async waitForPriorityChange(childID, expectedPriority) {
await TestUtils.waitForCondition(() => {
let currentPriority = this.priorityMap.get(childID);
async waitForPriorityChange(browser, expectedPriority) {
return TestUtils.waitForCondition(() => {
let currentPriority = this.priorityMap.get(browser);
if (currentPriority == expectedPriority) {
Assert.ok(
true,
`Process with child ID ${childID} reached expected ` +
`Browser at ${browser.currentURI.spec} reached expected ` +
`priority: ${currentPriority}`
);
return true;
}
return false;
}, `Waiting for process with child ID ${childID} to reach priority ${expectedPriority}`);
}, `Waiting for browser at ${browser.currentURI.spec} to reach priority ` + expectedPriority);
}
/**
* Returns a Promise that resolves after a duration of
* WAIT_FOR_CHANGE_TIME_MS. During that time, if the process
* with the passed in child ID changes priority, a test
* failure will be registered.
* WAIT_FOR_CHANGE_TIME_MS. During that time, if the passed browser
* changes priority, a test failure will be registered.
*
* @param childID
* The childID of the process that we expect to not change priority.
* @param browser (<browser>)
* The <browser> that we expect to change priority.
* @return Promise
* @resolves undefined
* Once the WAIT_FOR_CHANGE_TIME_MS duration has passed.
*/
async ensureNoPriorityChange(childID) {
this.noChangeChildIDs.set(childID, null);
async ensureNoPriorityChange(browser) {
this.noChangeBrowsers.set(browser, null);
// eslint-disable-next-line mozilla/no-arbitrary-setTimeout
await new Promise(resolve => setTimeout(resolve, WAIT_FOR_CHANGE_TIME_MS));
let priority = this.noChangeChildIDs.get(childID);
let priority = this.noChangeBrowsers.get(browser);
Assert.equal(
priority,
null,
`Should have seen no process priority change for child ID ${childID}`
`Should have seen no process priority change for a browser at ${browser.currentURI.spec}`
);
this.noChangeChildIDs.delete(childID);
this.noChangeBrowsers.delete(browser);
}
/**
* This returns a Promise that resolves when all of the processes
* of the browsing contexts in the browsing context tree
* of a particular <browser> have reached a particular priority.
* This will eventually time out if that priority is never reached.
* Makes sure that a particular foreground browser has been
* registered in the priority map. This is needed because browsers are
* only registered when their priorities change - and if a browser's
* priority never changes during a test, then they wouldn't be registered.
*
* @param browser (<browser>)
* The <browser> to get the BC tree from.
* @param expectedPriority (String)
* One of the PROCESS_PRIORITY_ constants defined at the
* top of this file.
* @return Promise
* @resolves undefined
* Once the browser reaches the expected priority.
* The passed browser must be a foreground browser, since it's assumed that
* the associated content process is running with foreground priority.
*
* @param browser (browser)
* A _foreground_ browser.
*/
async waitForBrowserTreePriority(browser, expectedPriority) {
let childIDs = new Set(
browser.browsingContext
.getAllBrowsingContextsInSubtree()
.map(browsingContextChildID)
);
let promises = [];
for (let childID of childIDs) {
let currentPriority = this.priorityMap.get(childID);
promises.push(
currentPriority == expectedPriority
? this.ensureNoPriorityChange(childID)
: this.waitForPriorityChange(childID, expectedPriority)
);
ensureForegroundRegistered(browser) {
if (!this.priorityMap.has(browser)) {
this.priorityMap.set(browser, PROCESS_PRIORITY_FOREGROUND);
}
await Promise.all(promises);
}
/**
* Synchronously returns the priority of a particular child ID.
* Synchronously returns the priority of a particular browser's
* content process.
*
* @param childID
* The childID to get the content process priority for.
* @param browser (browser)
* The browser to get the content process priority for.
* @return String
* The priority of the child ID's process.
* The priority that the browser's content process is at.
*/
currentPriority(childID) {
return this.priorityMap.get(childID);
currentPriority(browser) {
return this.priorityMap.get(browser);
}
/**
@ -198,10 +175,17 @@ class TabPriorityWatcher {
}
let { childID, priority } = this.parsePPMData(data);
if (this.noChangeChildIDs.has(childID)) {
this.noChangeChildIDs.set(childID, priority);
for (let browser of this.tabbrowser.browsers) {
if (browser.frameLoader.childID == childID) {
info(
`Browser at: ${browser.currentURI.spec} transitioning to ${priority}`
);
if (this.noChangeBrowsers.has(browser)) {
this.noChangeBrowsers.set(browser, priority);
}
this.priorityMap.set(browser, priority);
}
}
this.priorityMap.set(childID, priority);
}
}
@ -271,14 +255,32 @@ async function assertPriorityChangeOnBackground({
"Tabs should be running in separate processes."
);
let fromPromise = gTabPriorityWatcher.waitForBrowserTreePriority(
fromBrowser,
fromTabExpectedPriority
);
let toPromise = gTabPriorityWatcher.waitForBrowserTreePriority(
toBrowser,
gTabPriorityWatcher.ensureForegroundRegistered(fromBrowser);
let fromPromise;
if (
gTabPriorityWatcher.currentPriority(fromBrowser) == fromTabExpectedPriority
) {
fromPromise = gTabPriorityWatcher.ensureNoPriorityChange(fromBrowser);
} else {
fromPromise = gTabPriorityWatcher.waitForPriorityChange(
fromBrowser,
fromTabExpectedPriority
);
}
let toPromise;
if (
gTabPriorityWatcher.currentPriority(toBrowser) ==
PROCESS_PRIORITY_FOREGROUND
);
) {
toPromise = gTabPriorityWatcher.ensureNoPriorityChange(toBrowser);
} else {
toPromise = gTabPriorityWatcher.waitForPriorityChange(
toBrowser,
PROCESS_PRIORITY_FOREGROUND
);
}
await BrowserTestUtils.switchTab(gBrowser, toTab);
await Promise.all([fromPromise, toPromise]);
@ -292,185 +294,20 @@ async function assertPriorityChangeOnBackground({
add_task(async function test_normal_background_tab() {
let originalTab = gBrowser.selectedTab;
await BrowserTestUtils.withNewTab(
"http://example.com/browser/dom/ipc/tests/file_cross_frame.html",
async browser => {
let tab = gBrowser.getTabForBrowser(browser);
await assertPriorityChangeOnBackground({
fromTab: tab,
toTab: originalTab,
fromTabExpectedPriority: PROCESS_PRIORITY_BACKGROUND,
});
await BrowserTestUtils.withNewTab("http://example.com", async browser => {
let tab = gBrowser.getTabForBrowser(browser);
await assertPriorityChangeOnBackground({
fromTab: tab,
toTab: originalTab,
fromTabExpectedPriority: PROCESS_PRIORITY_BACKGROUND,
});
await assertPriorityChangeOnBackground({
fromTab: originalTab,
toTab: tab,
fromTabExpectedPriority: PROCESS_PRIORITY_BACKGROUND,
});
}
);
});
/**
* If an iframe in a foreground tab is navigated to a new page for
* a different site, then the process of the new iframe page should
* have priority PROCESS_PRIORITY_FOREGROUND. Additionally, if Fission
* is enabled, then the old iframe page's process's priority should be
* lowered to PROCESS_PRIORITY_BACKGROUND.
*/
add_task(async function test_iframe_navigate() {
// The URI we're going to navigate the iframe to.
let iframeURI2 =
"http://mochi.test:8888/browser/dom/ipc/tests/file_dummy.html";
// Load this as a top level tab so that when we later load it as an iframe we
// won't be creating a new process, so that we're testing the "load a new page"
// part of prioritization and not the "initial process load" part.
let newIFrameTab = await BrowserTestUtils.openNewForegroundTab(
gBrowser,
iframeURI2
);
let newIFrameTabChildID = browsingContextChildID(
gBrowser.selectedBrowser.browsingContext
);
Assert.equal(
gTabPriorityWatcher.currentPriority(newIFrameTabChildID),
PROCESS_PRIORITY_FOREGROUND,
"Loading a new tab should make it prioritized"
);
await BrowserTestUtils.withNewTab(
"http://example.com/browser/dom/ipc/tests/file_cross_frame.html",
async browser => {
Assert.equal(
gTabPriorityWatcher.currentPriority(newIFrameTabChildID),
PROCESS_PRIORITY_BACKGROUND,
"Switching to a new tab should deprioritize the old one"
);
let iframe = browser.browsingContext.children[0];
let iframeChildID1 = browsingContextChildID(iframe);
// Do a cross-origin navigation in the iframe in the foreground tab.
let loaded = BrowserTestUtils.browserLoaded(browser, true, iframeURI2);
await SpecialPowers.spawn(iframe, [iframeURI2], async function(
_iframeURI2
) {
content.location = _iframeURI2;
});
await loaded;
let iframeChildID2 = browsingContextChildID(iframe);
let iframePriority1 = gTabPriorityWatcher.currentPriority(iframeChildID1);
let iframePriority2 = gTabPriorityWatcher.currentPriority(iframeChildID2);
if (SpecialPowers.useRemoteSubframes) {
Assert.equal(
newIFrameTabChildID,
iframeChildID2,
"The same site should get loaded into the same process"
);
Assert.notEqual(
iframeChildID1,
iframeChildID2,
"Navigation should have switched processes"
);
Assert.equal(
iframePriority1,
PROCESS_PRIORITY_BACKGROUND,
"The old iframe process should have been deprioritized"
);
} else {
Assert.equal(
iframeChildID1,
iframeChildID2,
"Navigation should not have switched processes"
);
}
Assert.equal(
iframePriority2,
PROCESS_PRIORITY_FOREGROUND,
"The new iframe process should be prioritized"
);
}
);
await BrowserTestUtils.removeTab(newIFrameTab);
});
/**
* Test that a cross-group navigation properly preserves the process priority.
* The goal of this test is to check that the code related to mPriorityActive in
* CanonicalBrowsingContext::ReplacedBy works correctly, but in practice the
* prioritization code in SetRenderLayers will also make this test pass, though
* that prioritization happens slightly later.
*/
add_task(async function test_cross_group_navigate() {
// This page is same-site with the page we're going to cross-group navigate to.
let coopPage =
"https://example.com/browser/dom/tests/browser/file_coop_coep.html";
// Load it as a top level tab so that we don't accidentally get the initial
// load prioritization.
let backgroundTab = await BrowserTestUtils.openNewForegroundTab(
gBrowser,
coopPage
);
let backgroundTabChildID = browsingContextChildID(
gBrowser.selectedBrowser.browsingContext
);
Assert.equal(
gTabPriorityWatcher.currentPriority(backgroundTabChildID),
PROCESS_PRIORITY_FOREGROUND,
"Loading a new tab should make it prioritized"
);
await BrowserTestUtils.withNewTab(
"http://example.org/browser/dom/ipc/tests/file_cross_frame.html",
async browser => {
Assert.equal(
gTabPriorityWatcher.currentPriority(backgroundTabChildID),
PROCESS_PRIORITY_BACKGROUND,
"Switching to a new tab should deprioritize the old one"
);
let dotOrgChildID = browsingContextChildID(browser.browsingContext);
// Do a cross-group navigation.
BrowserTestUtils.loadURI(browser, coopPage);
await BrowserTestUtils.browserLoaded(browser);
let coopChildID = browsingContextChildID(browser.browsingContext);
let coopPriority = gTabPriorityWatcher.currentPriority(coopChildID);
let dotOrgPriority = gTabPriorityWatcher.currentPriority(dotOrgChildID);
Assert.equal(
backgroundTabChildID,
coopChildID,
"The same site should get loaded into the same process"
);
Assert.notEqual(
dotOrgChildID,
coopChildID,
"Navigation should have switched processes"
);
Assert.equal(
dotOrgPriority,
PROCESS_PRIORITY_BACKGROUND,
"The old page process should have been deprioritized"
);
Assert.equal(
coopPriority,
PROCESS_PRIORITY_FOREGROUND,
"The new page process should be prioritized"
);
}
);
await BrowserTestUtils.removeTab(backgroundTab);
await assertPriorityChangeOnBackground({
fromTab: originalTab,
toTab: tab,
fromTabExpectedPriority: PROCESS_PRIORITY_BACKGROUND,
});
});
});
/**

View File

@ -1,12 +0,0 @@
<!DOCTYPE HTML>
<html>
<head>
<meta charset="utf-8">
<title>Different-origin iframe</title>
</head>
<body>
<iframe id="testIFrame" src="http://example.org/browser/dom/ipc/tests/file_dummy.html"></iframe>
<i>I am a web page</i>
</div>
</body>
</html>

View File

@ -1,7 +1,4 @@
<!doctype html>
<html>
<head><meta charset="utf-8"></head>
<body>
<h1>This is a dummy file</h1>
</body>
</html>