1582042 - BrowserChild should use DocShell::SetIsActive rather than PresShell::SetIsActive

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Emilio Cobos Álvarez [:emilio]]

The later is not supposed to be called from anywhere else that isn't presShell / docshell.

Comment 1

[Timothy Nikkel (:tnikkel)]

I noticed the same thing in bug 1577084. Looks like splitting the docshell and presshell notion of Active happened in https://hg.mozilla.org/mozilla-central/rev/b0c38584a789 from bug 1397426. And then that has morphed into what we have today I guess?

Comment 2

[Emilio Cobos Álvarez [:emilio]]

I have a patch that on top of bug 1578379 is ~green. Causes some autoplay failures probably related to bug 1572798...

Comment 3

[Emilio Cobos Álvarez [:emilio]]

The autoplay failures were because we were calling this from tab warming, which turns our document "visible", and thus we unblock media playback. Was a fun one to dig.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1582042 - Use DocShell::SetIsActive from BrowserChild::MakeVisible for non top-level browsers.

That is, for fission iframes. For top level stuff we rely on the tab switcher
going through SetDocShellIsActive and such.

See the expanded comment as for why. Ideally we could simplify this further by
not making RecvRenderLayers update the visibility (which spins the refresh
driver and such).

It's a bit suspect because it's very easy to get to an inconsistent state if the
browser chrome does something wrong.

I'll try to do that, but for now this should improve the fission situation
anyway.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1582042 - Remove useless threading of aForceRepaint around various IPC messages.

aForceRepaint wasn't doing what it claimed to do at all, as we've recently
learned. In current mozilla-central:

• All those arguments ended up in a RecvRenderLayers with aForceRepaint = true
  (so far so good, that's expected).

• But it was ignored (so that aForceRepaint is always true to calls to
  MakeVisible) from UpdateVisibility:

https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/dom/ipc/BrowserChild.cpp#2523

• Plus that argument only does anything useful on current central if we get to
  the end of MakeVisible(true). And MakeVisible() early returns if already
  visible.

So all in all this seems somewhat useless, and nobody has complained about such
a thing in a long time.

It seemed to do what it promised when it was introduced in
https://hg.mozilla.org/mozilla-central/rev/27f6f789b194, but it seems the
refactoring in https://hg.mozilla.org/mozilla-central/rev/4df5fa6fa785 broke it.

I think the new setup is somewhat easier to reason about, and nobody seems to be
missing that.

I'll try to remove the forceRepaint() call itself on a follow-up.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1582042 - Remove nsIRemoteTab.forceRepaint().

It's useless if the tab is already visible (i.e., has renderLayers=true), per
the previous patches, and that's the only point at which it gets called.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Try runs look green:

• https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a0bde451b1ae77300b282e8dfaf4dff95d2b33f
• https://treeherder.mozilla.org/#/jobs?repo=try&revision=91c8c85093d1be64701e35c7ebfc8cdf7c0f0678
• https://treeherder.mozilla.org/#/jobs?repo=try&revision=123c27ccd0db481723979ed2d88a619a0eeddcb7

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08088ff7a8be
Use DocShell::SetIsActive from BrowserChild::MakeVisible for non top-level browsers. r=mconley,rhunt

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f2441f2f26e
Remove useless threading of aForceRepaint around various IPC messages. r=mconley

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f12d9b4c6b5
Remove nsIRemoteTab.forceRepaint(). r=mconley

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/08088ff7a8be
https://hg.mozilla.org/mozilla-central/rev/7f2441f2f26e
https://hg.mozilla.org/mozilla-central/rev/9f12d9b4c6b5