1162700 - With e10s, need to initialize app-info sooner in the content process - before graphics is

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

When initializing graphics, we need to know the version we're running, so that we can interpret the blocklist correctly (see bug 1162530.)  That works in the parent process, but for the child process, app-info is initialized too late, after we've already gone through initializing graphics, so we end up making the wrong decision here:

#04: gfxPlatform::Init()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x173ee9a]
#05: gfxPlatform::GetPlatform()[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x173e3da]
#06: mozilla::layers::CompositorChild::Create(IPC::Channel*, int)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x16bac4e]
#07: mozilla::dom::ContentChild::AllocPCompositorChild(IPC::Channel*, int)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x362933f]
#08: mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&)[/Users/msreckovic/Repos/mozilla-central/obj-x86_64-apple-darwin13.4.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xdf660c]

because it happens before this in ContentParent.cpp:
    if (gAppData) {
        nsCString version(gAppData->version);
...
        // Sending all information to content process.
        unused << SendAppInfo(version, buildID, name, UAName, ID, vendor);
    }

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Heads up - will probably want to track for 39/40, if the bug 1162530 it blocks ends up being tracked for those releases.
:smaug, Conley suggested you'd be the right person to tell us if it makes sense to initialize app data earlier.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: SendAppInfo earlier, so that graphics has it. r=smaug

This fixes the problem for me, but I don't pretend to understand if app data is now initialized "too early".

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

No need for needinfo as there's by now a review request.

Comment 4

[Olli Pettay [:smaug] (away May 1 - 8)]

Comment on attachment 8602964 [details] [diff] [review]
SendAppInfo earlier, so that graphics has it. r=smaug

So, the comment says
"The CompositorChild must be created before any
 PBrowsers are created, because they rely on the Compositor
 already being around."

and SendAppInfo may actually create the child side of a PBrowser via PreloadSlowThings.
It is weird that SendAppInfo ends up doing any preloading stuff. What if
you just split that to two, and SendAppInfo could happen early, and then
the new Preload thing after aSetupOffMainThreadCompositing.
That would make the setup a tad easier to understand.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

I'll see if I can figure this out; also, I just realized we don't have to track for 39/40, as the blocking of bug 1162530 is only in the e10s case, where we're fine with the order of startup.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: WIP - split the SendAppInfo into two pieces.

Is this the right direction for the split you had in mind? If so, any detailed pointers on the naming, other things to look out for, what needs more comments, etc., if not, any suggestions as to what would work better?

Comment 7

[Olli Pettay [:smaug] (away May 1 - 8)]

Comment on attachment 8604184 [details] [diff] [review]
WIP - split the SendAppInfo into two pieces.

Yup, this looks fine to me

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bff0a16759ed

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Split the SendAppInfo into two pieces. r=smaug

Same code as the WIP f+ patch, just changed the patch comment.  Try run looks good.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b0f4250a4468

Comment 11

[Carsten Book [:Tomcat]]

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