Closed Bug 1257937 Opened 6 years ago Closed 6 years ago

Make _delayedStartup run after both the outer window and the web content in the initial browser has painted

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mconley, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 obsolete file)

Right now, _delayedStartup in browser.js is fired once MozAfterPaint is fired in the parent process. This is fine in single-process mode, but in multi-process mode, this causes the parent to enter _delayedStartup at a rather inconvenient time. For example, _delayedStartup will focus the browser, which sends a message down to content to switch focus, which causes IME to send sync messages to the parent (which is still running delayedStartup stuff), all before content has had a chance to paint.

We should optimize for painting the web content so that we can show something to the user ASAP.
It looks like this patch would make the running of delayedStartup dependent on whatever web page(s) we load in the initial tabs (the initial about:blank load being explicitly excluded). If I'm on a rubbish internet connection, this is just going to delay the running of delayedStartup by the limit of 1000ms, right? That... that doesn't sound like a great idea? Some of that code is actually reasonably important (ie some UI won't work (very well) when that code hasn't run).
Flags: needinfo?(mconley)
(In reply to :Gijs Kruitbosch from comment #2)
> It looks like this patch would make the running of delayedStartup dependent
> on whatever web page(s) we load in the initial tabs (the initial about:blank
> load being explicitly excluded). If I'm on a rubbish internet connection,
> this is just going to delay the running of delayedStartup by the limit of
> 1000ms, right? That... that doesn't sound like a great idea? Some of that
> code is actually reasonably important (ie some UI won't work (very well)
> when that code hasn't run).

Yeah, that's a good point. I just talked to BenWa about this, and here's my understanding of how things work:

When we load a page, we start downloading the bytes over the network and assembling the page on our side. We suppress painting until one of two things happen: 1) We receive enough styling information to try painting something (this is based on a heuristic, and the point is to avoid showing a flash of unstyled content), or 2) We hit some timeout.

So you're right. For a bad connection, we might not paint for a while. Perhaps we should drop this timeout down from 1000ms (which I picked arbitrarily) to something like 200ms? Or 100ms?
Flags: needinfo?(mconley)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #3)
> flash of unstyled content), or 2) We hit some timeout.
> 

To be clear, BenWa suggested that there's a timeout somewhere in either our styling or painting stack that will cause the paint to occur if we haven't received enough styling information. I don't exactly know where that timeout lives.
Having talked to avih and jmaher about this, I think this optimizes for the wrong thing in tpaint. This patch will optimize for painting content and the browser UI, but does so by sacrificing making the browser usable. According to avih and jmaher in bug 1174770, tpaint is supposed to capture (even though it doesn't do this perfectly) how long it takes to present a usable browser to the user - which _includes_ delayedStartup. So this bug would be cheating.

So I don't think we should do this anymore.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Attachment #8732346 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.