1489762 - WebProgressChild.jsm forces creation of initial about:blank and thus slows down page loading

Status: RESOLVED FIXED

(Firefox :: General, defect, P3)

Description

[Olli Pettay [:smaug][[e-mail]]]

The issue seems to be in _setupObjects.

https://perfht.ml/2McpCG1

That profile is from otherwise horrible page bug https://bugzilla.mozilla.org/show_bug.cgi?id=1488164, but the issue is there anyhow.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Gijs is looking into this across several bugs, so CC'ing him to see if he has noticed this one yet

Comment 2

[Alex Thayer [:alexical] (she/her)]

Giving this a p2 for now, assuming this isn't already accounted for in Gijs's todo list.

Comment 3

[:Gijs (away/busy until Mar 10)]

At a glance, this seems to have been regressed by bug 1479312. We used to go to significant lengths in https://hg.mozilla.org/mozreview/gecko/rev/d9c4348fdca4#l2.58 to avoid creating the about:blank viewer.

Kris, the mozreview comments are gone, was there a particular reason you didn't keep those parts of setupObjects ?

Comment 4

[:Gijs (away/busy until Mar 10)]

Would be nice if we could get a fix in for 63.

Comment 5

[Kris Maglione [:kmag]]

(In reply to :Gijs (he/him) from comment #3)
> At a glance, this seems to have been regressed by bug 1479312. We used to go
> to significant lengths in
> https://hg.mozilla.org/mozreview/gecko/rev/d9c4348fdca4#l2.58 to avoid
> creating the about:blank viewer.
> 
> Kris, the mozreview comments are gone, was there a particular reason you
> didn't keep those parts of setupObjects ?

Not sure what you mean. As far as I can tell, I didn't change the logic there.

Comment 6

[Kris Maglione [:kmag]]

We definitely do force the creation of a content viewer in content.js:

https://searchfox.org/mozilla-central/rev/a0333927deabfe980094a14d0549b589f34cbe49/browser/base/content/content.js#53-54

But that is also definitely not new. That hack is there to preserve the previous behavior after I removed or lazified other code which eagerly touched `content`.

Comment 7

[:Gijs (away/busy until Mar 10)]

(In reply to Kris Maglione [:kmag] from comment #5)
> (In reply to :Gijs (he/him) from comment #3)
> > At a glance, this seems to have been regressed by bug 1479312. We used to go
> > to significant lengths in
> > https://hg.mozilla.org/mozreview/gecko/rev/d9c4348fdca4#l2.58 to avoid
> > creating the about:blank viewer.
> > 
> > Kris, the mozreview comments are gone, was there a particular reason you
> > didn't keep those parts of setupObjects ?
> 
> Not sure what you mean. As far as I can tell, I didn't change the logic
> there.

Yeah, sorry, I clearly misread this. setupJSON had guards (and still does) against this type of thing, but setupObjects doesn't.

(In reply to Kris Maglione [:kmag] from comment #6)
> We definitely do force the creation of a content viewer in content.js:
> 
> https://searchfox.org/mozilla-central/rev/
> a0333927deabfe980094a14d0549b589f34cbe49/browser/base/content/content.js#53-
> 54

Right, I'm aware of bug 1471327. Based on my experience so far fixing tests for bug 1362774, there's going to be a bunch of whack-a-mole to make a change there to avoid the about:blank creation. :-(

Comment 8

[Kris Maglione [:kmag]]

This code has been deleted.