1520791 - "docShellIsActive" is no longer set to "true" for the initial tab after opening the browser

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Since the patch on bug 1506608 landed the value of docShellIsActive is no longer true for the initial tab.

Steps:

1. Start Firefox
2. Open the Scratch pad
3. Switch to Browser environment
4. Run: alert(window.gBrowser.selectedTab.linkedBrowser.docShellIsActive)

Expected result:

The flag should report that the docshell is active

Current result:

The flag tells that the docshell is not active

Once a new tab gets opened the value is correctly initialized.

I don't know which implications that generally has and how important it is to get fixed. As such I will mark it as tracking firefox 65.

Comment 1

[:Gijs (he/him)]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #0)

I don't know which implications that generally has and how important it is to get fixed. As such I will mark it as tracking firefox 65.

Me neither. The content docshell does know that it's active, from the looks of it (when checking with the browser content toolbox . But I expect this isn't good for the tabswitcher code...

Comment 2

[Ryan VanderMeulen [:RyanVM]]

tracking65+, but it would be really good to understand what the implications of this are given that we're building the final beta of the 65 cycle today and the RC next week.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Before bug 1506608 landed, we were flipping the remoteness of the initial tab, which caused us to hit this part of the codebase: (1).

shouldActivateDocShell would return true, and bam, the docShellIsActive property would be set to true.

Because we don't flip remoteness for the initial tab (which is a good thing), we never hit this code path, so the property is never touched.

What's particularly troubling is that the TabParent and the underlying DocShell are out of sync - TabParent's mDocShellIsActive member defaults to false (2), but DocShell's mIsActive member defaults to true (3). That's what Gijs was seeing in comment 1.

Given how close we are to the uplift, here's what I suggest:

1. We paper over the issue the safe way by setting the docShellIsActive property on the TabParent for the initial tab manually.
2. File a follow-up bug to reconcile the initial active states between TabParent and nsDocShell.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1520791 - ensure initial browser's docshell is marked as active in the parent, r?mconley

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de634684229e
ensure initial browser's docshell is marked as active in the parent, r=mconley

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/de634684229e

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 9037269 [details]
Bug 1520791 - ensure initial browser's docshell is marked as active in the parent, r?mconley

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1506608

User impact if declined: Unknown. Possibly nothing. This patch is addressing an inconsistency that we discovered where something in the parent process is out of sync with the underlying representation in its content process. This has been on Nightly and in Beta for a while, and we haven't heard about any major problems, but we'd rather be safe than sorry.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce:

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Not risky, since we're effectively reverting the logic back to what we did before bug 1506608 landed, where we manually set the docShellIsActive state in the parent on the initial tab to true, to match the underlying DocShell isActive.

String changes made/needed: None.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9037269 [details]
Bug 1520791 - ensure initial browser's docshell is marked as active in the parent, r?mconley

[Triage Comment]
Gets us back to a known-safer state. Patch is low-risk. Approved for 65.0b12.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9e5bf16f07c4

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks for the quick fix.