Skip to content

Commit

Permalink
Backed out changeset 7b3a02a659ef (bug 1767346) for causing geckoview…
Browse files Browse the repository at this point in the history
… failures. CLOSED TREE
  • Loading branch information
Marian-Vasile Laza committed May 20, 2022
1 parent 4530958 commit 9adb720
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 85 deletions.
31 changes: 13 additions & 18 deletions docshell/base/BrowsingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#endif
#include "mozilla/AppShutdown.h"
#include "mozilla/dom/CanonicalBrowsingContext.h"
#include "mozilla/dom/BrowserHost.h"
#include "mozilla/dom/BrowserChild.h"
#include "mozilla/dom/BrowserParent.h"
#include "mozilla/dom/BrowsingContextGroup.h"
Expand Down Expand Up @@ -2642,29 +2641,25 @@ void BrowsingContext::DidSet(FieldIndex<IDX_ExplicitActive>,
if (IsTop()) {
Group()->UpdateToplevelsSuspendedIfNeeded();

if (XRE_IsParentProcess()) {
auto* bc = Canonical();
if (BrowserParent* bp = bc->GetBrowserParent()) {
bp->RecomputeProcessPriority();
#if defined(XP_WIN) && defined(ACCESSIBILITY)
if (a11y::Compatibility::IsDolphin()) {
// update active accessible documents on windows
if (a11y::DocAccessibleParent* tabDoc =
bp->GetTopLevelDocAccessible()) {
HWND window = tabDoc->GetEmulatedWindowHandle();
MOZ_ASSERT(window);
if (window) {
if (isActive) {
a11y::nsWinUtils::ShowNativeWindow(window);
} else {
a11y::nsWinUtils::HideNativeWindow(window);
}
if (XRE_IsParentProcess() && a11y::Compatibility::IsDolphin()) {
// update active accessible documents on windows
if (BrowserParent* bp = Canonical()->GetBrowserParent()) {
if (a11y::DocAccessibleParent* tabDoc =
bp->GetTopLevelDocAccessible()) {
HWND window = tabDoc->GetEmulatedWindowHandle();
MOZ_ASSERT(window);
if (window) {
if (isActive) {
a11y::nsWinUtils::ShowNativeWindow(window);
} else {
a11y::nsWinUtils::HideNativeWindow(window);
}
}
}
#endif
}
}
#endif
}

PreOrderWalk([&](BrowsingContext* aContext) {
Expand Down
30 changes: 23 additions & 7 deletions dom/ipc/BrowserHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,22 @@ BrowserHost::SetRenderLayers(bool aRenderLayers) {
return NS_OK;
}

bool priorityHint;
GetPriorityHint(&priorityHint);
ProcessPriorityManager::BrowserPriorityChanged(
GetBrowsingContext()->Canonical(), priorityHint || aRenderLayers);
mRoot->SetRenderLayers(aRenderLayers);
return NS_OK;
}

/* readonly attribute boolean hasLayers; */
NS_IMETHODIMP
BrowserHost::GetHasLayers(bool* aHasLayers) {
*aHasLayers = mRoot && mRoot->GetHasLayers();
if (!mRoot) {
*aHasLayers = false;
return NS_OK;
}
*aHasLayers = mRoot->GetHasLayers();
return NS_OK;
}

Expand All @@ -137,19 +145,27 @@ BrowserHost::SetPriorityHint(bool aPriorityHint) {
if (!mRoot) {
return NS_OK;
}
bool renderLayers;
GetRenderLayers(&renderLayers);
ProcessPriorityManager::BrowserPriorityChanged(
GetBrowsingContext()->Canonical(), aPriorityHint || renderLayers);
mRoot->SetPriorityHint(aPriorityHint);
return NS_OK;
}

NS_IMETHODIMP
BrowserHost::GetPriorityHint(bool* aPriorityHint) {
*aPriorityHint = mRoot && mRoot->GetPriorityHint();
if (!mRoot) {
*aPriorityHint = false;
return NS_OK;
}
*aPriorityHint = mRoot->GetPriorityHint();
return NS_OK;
}

/* void resolutionChanged (); */
NS_IMETHODIMP
BrowserHost::NotifyResolutionChanged() {
BrowserHost::NotifyResolutionChanged(void) {
if (!mRoot) {
return NS_OK;
}
Expand All @@ -161,13 +177,13 @@ BrowserHost::NotifyResolutionChanged() {

/* void deprioritize (); */
NS_IMETHODIMP
BrowserHost::Deprioritize() {
BrowserHost::Deprioritize(void) {
if (!mRoot) {
return NS_OK;
}
auto* bc = GetBrowsingContext()->Canonical();
ProcessPriorityManager::BrowserPriorityChanged(bc,
/* aPriority = */ false);
ProcessPriorityManager::BrowserPriorityChanged(
GetBrowsingContext()->Canonical(),
/* aPriority = */ false);
return NS_OK;
}

Expand Down
18 changes: 2 additions & 16 deletions dom/ipc/BrowserParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ using namespace mozilla::widget;
using namespace mozilla::gfx;

using mozilla::LazyLogModule;
using mozilla::Unused;

extern mozilla::LazyLogModule gSHIPBFCacheLog;

Expand Down Expand Up @@ -242,18 +243,10 @@ BrowserParent::BrowserParent(ContentParent* aManager, const TabId& aTabId,
// that some input events are dispatched before PBrowserConstructor.
mIsReadyToHandleInputEvents = !ContentParent::IsInputEventQueueSupported();

// Make sure to compute our process priority if needed before the block of
// code below. This makes sure the block below prioritizes our process if
// needed.
if (aBrowsingContext->IsTop()) {
RecomputeProcessPriority();
}

// 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, and also that the process is correctly prioritized if we got
// created for a browsing context which was already active.
// active tab.
if (aBrowsingContext->Top()->IsPriorityActive()) {
ProcessPriorityManager::BrowserPriorityChanged(this, true);
}
Expand Down Expand Up @@ -3484,13 +3477,6 @@ bool BrowserParent::GetPriorityHint() { return mPriorityHint; }

void BrowserParent::SetPriorityHint(bool aPriorityHint) {
mPriorityHint = aPriorityHint;
RecomputeProcessPriority();
}

void BrowserParent::RecomputeProcessPriority() {
auto* bc = GetBrowsingContext();
ProcessPriorityManager::BrowserPriorityChanged(
bc, bc->IsActive() || mPriorityHint);
}

void BrowserParent::PreserveLayers(bool aPreserveLayers) {
Expand Down
2 changes: 0 additions & 2 deletions dom/ipc/BrowserParent.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ class BrowserParent final : public PBrowserParent,

CanonicalBrowsingContext* GetBrowsingContext() { return mBrowsingContext; }

void RecomputeProcessPriority();

already_AddRefed<nsILoadContext> GetLoadContext();

Element* GetOwnerElement() const { return mFrameElement; }
Expand Down
21 changes: 3 additions & 18 deletions dom/ipc/ProcessPriorityManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -509,20 +509,10 @@ void ProcessPriorityManagerImpl::NotifyProcessPriorityChanged(
}
}

static nsCString BCToString(dom::CanonicalBrowsingContext* aBC) {
nsCOMPtr<nsIURI> uri = aBC->GetCurrentURI();
return nsPrintfCString("id=%" PRIu64 " uri=%s active=%d pactive=%d",
aBC->Id(),
uri ? uri->GetSpecOrDefault().get() : nullptr,
aBC->IsActive(), aBC->IsPriorityActive());
}

void ProcessPriorityManagerImpl::BrowserPriorityChanged(
dom::CanonicalBrowsingContext* aBC, bool aPriority) {
MOZ_ASSERT(aBC->IsTop());

LOG("BrowserPriorityChanged(%s, %d)\n", BCToString(aBC).get(), aPriority);

bool alreadyActive = aBC->IsPriorityActive();
if (alreadyActive == aPriority) {
return;
Expand All @@ -535,8 +525,6 @@ void ProcessPriorityManagerImpl::BrowserPriorityChanged(

aBC->PreOrderWalk([&](BrowsingContext* aContext) {
CanonicalBrowsingContext* canonical = aContext->Canonical();
LOG("PreOrderWalk for %p: %p -> %p, %p\n", aBC, canonical,
canonical->GetContentParent(), canonical->GetBrowserParent());
if (ContentParent* cp = canonical->GetContentParent()) {
if (RefPtr pppm = GetParticularProcessPriorityManager(cp)) {
if (auto* bp = canonical->GetBrowserParent()) {
Expand All @@ -549,8 +537,6 @@ void ProcessPriorityManagerImpl::BrowserPriorityChanged(

void ProcessPriorityManagerImpl::BrowserPriorityChanged(
BrowserParent* aBrowserParent, bool aPriority) {
LOG("BrowserPriorityChanged(bp=%p, %d)\n", aBrowserParent, aPriority);

if (RefPtr pppm =
GetParticularProcessPriorityManager(aBrowserParent->Manager())) {
Telemetry::ScalarAdd(
Expand Down Expand Up @@ -780,14 +766,13 @@ void ParticularProcessPriorityManager::SetPriorityNow(
return;
}

LOGP("Changing priority from %s to %s (cp=%p).",
ProcessPriorityToString(mPriority), ProcessPriorityToString(aPriority),
mContentParent);

if (!mContentParent || mPriority == aPriority) {
return;
}

LOGP("Changing priority from %s to %s.", ProcessPriorityToString(mPriority),
ProcessPriorityToString(aPriority));

PROFILER_MARKER(
"Subprocess Priority", OTHER,
MarkerOptions(MarkerThreadId::MainThread(), MarkerStack::Capture()),
Expand Down
16 changes: 0 additions & 16 deletions dom/ipc/tests/browser_ProcessPriorityManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,19 +318,6 @@ add_task(async function test_normal_background_tab() {
"PriorityHint of the original tab should be false on default"
);

// Changing renderLayers doesn't change priority of the background tab.
originalTab.linkedBrowser.preserveLayers(true);
originalTab.linkedBrowser.renderLayers = true;
await new Promise(resolve =>
// eslint-disable-next-line mozilla/no-arbitrary-setTimeout
setTimeout(resolve, WAIT_FOR_CHANGE_TIME_MS)
);
Assert.equal(
gTabPriorityWatcher.currentPriority(origtabID),
PROCESS_PRIORITY_BACKGROUND,
"Tab didn't get prioritized only due to renderLayers"
);

// Test when priorityHint is true, the original tab priority
// becomes PROCESS_PRIORITY_FOREGROUND.
originalTab.linkedBrowser.frameLoader.remoteTab.priorityHint = true;
Expand Down Expand Up @@ -377,9 +364,6 @@ add_task(async function test_normal_background_tab() {
PROCESS_PRIORITY_FOREGROUND,
"Setting priorityHint to false should maintain the new tab priority as foreground"
);

originalTab.linkedBrowser.preserveLayers(false);
originalTab.linkedBrowser.renderLayers = false;
}
);
});
Expand Down
2 changes: 0 additions & 2 deletions hal/android/AndroidProcessPriority.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
#include "mozilla/java/GeckoProcessTypeWrappers.h"
#include "mozilla/java/ServiceAllocatorWrappers.h"

using namespace mozilla::hal;

/**
* Bucket the Gecko HAL process priority level into one of the three Android
* priority levels.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,12 @@ class GeckoViewTest : BaseSessionTest() {
val lowPids = low.map { sessionRule.getSessionPid(it) }.toSet()

UiThreadUtils.waitForCondition({
val shouldBeHighPri = getContentProcessesOomScore(highPids)
val shouldBeLowPri = getContentProcessesOomScore(lowPids)
// Note that higher oom score means less priority
shouldBeHighPri.count { it > 100 } == 0
&& shouldBeLowPri.count { it < 300 } == 0
getContentProcessesPriority(highPids).count { it > 100 } == 0
&& getContentProcessesPriority(lowPids).count { it < 300 } == 0
}, env.defaultTimeoutMillis)
}

fun getContentProcessesOomScore(pids: Collection<Int>) : List<Int> {
fun getContentProcessesPriority(pids: Collection<Int>) : List<Int> {
return pids.map { pid ->
File("/proc/$pid/oom_score").readText(Charsets.UTF_8).trim().toInt()
}
Expand Down Expand Up @@ -165,6 +162,29 @@ class GeckoViewTest : BaseSessionTest() {
}
}

@Test
@NullDelegate(Autofill.Delegate::class)
fun extensionCurrentTabRaisesPriority() {
// Bug 1767346
assumeThat(false, equalTo(true))

val otherSession = setupPriorityTest()

// Setting priorityHint to PRIORITY_HIGH raises priority
mainSession.setPriorityHint(GeckoSession.PRIORITY_HIGH)

waitUntilContentProcessPriority(
high = listOf(mainSession, otherSession), low = listOf()
)

// Setting the priorityHint to default should lower priority
mainSession.setPriorityHint(GeckoSession.PRIORITY_DEFAULT)

waitUntilContentProcessPriority(
high = listOf(mainSession), low = listOf(otherSession)
)
}

@Test
@NullDelegate(Autofill.Delegate::class)
fun processPriorityTest() {
Expand Down Expand Up @@ -201,6 +221,9 @@ class GeckoViewTest : BaseSessionTest() {
@Test
@NullDelegate(Autofill.Delegate::class)
fun setPriorityHint() {
// Bug 1767346
assumeThat(false, equalTo(true))

val otherSession = setupPriorityTest()

// Setting priorityHint to PRIORITY_HIGH raises priority
Expand Down

0 comments on commit 9adb720

Please sign in to comment.