Places should not be initialized before first paint

RESOLVED FIXED in Firefox 56

Status

()

Toolkit
Places
RESOLVED FIXED
3 months ago
22 days ago

People

(Reporter: florian, Assigned: florian)

Tracking

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 months ago
The code in nsBrowserGlue.js causing places to be initialized before we create the first browser window seems to be doing it by accident.
(Assignee)

Comment 1

3 months ago
Created attachment 8876185 [details] [diff] [review]
Patch
(Assignee)

Updated

3 months ago
Assignee: nobody → florian
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 months ago
Created attachment 8876312 [details] [diff] [review]
Patch v2

Try was green-ish for attachment 8876185 [details] [diff] [review] except for the startup test in which my blacklist was a bit too agressive:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1594de7529d88e9cccf58ae3525e7c6357b1c7b

Here is a try push of the new version with the fixed blacklist:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=85838b5ba04bd3f959ef07de7a41a6ed6de5ffa5

I think this is now ready for review.
Attachment #8876312 - Flags: review?(mconley)
(Assignee)

Updated

3 months ago
Attachment #8876185 - Attachment is obsolete: true
Comment on attachment 8876312 [details] [diff] [review]
Patch v2

Review of attachment 8876312 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8876312 - Flags: review?(mconley) → review+
I'll be eager to see what, if anything, this does to our ts_paint numbers. I'm 60% confident that we'll see an improvement.
(Assignee)

Comment 5

2 months ago
Created attachment 8876861 [details] [diff] [review]
Patch without test

I'm failing to make the tests pass on Windows, and giving up, given that Marco is actively working on bug 1371677 which should help a lot.
My current investigations show that on try on Windows some C++ code is starting places when we load a page from a JAR file (I suspect about:newtab).
(Assignee)

Updated

2 months ago
Attachment #8876312 - Attachment is obsolete: true

Comment 6

2 months ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d3d1741941f
Places should not be initialized before first paint, r=mconley.

Comment 7

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2d3d1741941f
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Assignee)

Updated

2 months ago
Depends on: 1373387

Updated

22 days ago
Blocks: 1386445
You need to log in before you can comment on or make changes to this bug.