1362891 - Add a XRE_IsE10sParentProcess function

Status: RESOLVED FIXED

(Core :: General, enhancement)

Description

[Jonathan Watt [:jwatt]]

XRE_IsParentProcess returns true for the e10s parent process, but also for the main process when e10s is disabled. In some cases, such as in bug 1362120, we want to evaluate code only in the e10s parent process. To do that code checks both XRE_IsParentProcess() && BrowserTabsRemoteAutostart(). It would make code clearer to add an XRE_IsE10sParentProcess function.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: part 1 - Add a XRE_IsE10sParentProcess function

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: part 2 - Make use of XRE_IsE10sParentProcess in code that could use it

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8865265 [details] [diff] [review]
part 1 - Add a XRE_IsE10sParentProcess function

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

r=me with some minor fixes below.

::: toolkit/xre/nsAppRunner.cpp
@@ +4928,5 @@
>    return XRE_GetProcessType() == GeckoProcessType_Default;
>  }
>  
>  bool
> +XRE_IsE10sParentProcess()

This needs to be declared in xpcom/build/nsXULAppAPI.h (I know, I know, why is it declared there and defined here...).  The comment added above should be in that file as well, and if you could please add a small blurb about this function, that would be good too.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8865266 [details] [diff] [review]
part 2 - Make use of XRE_IsE10sParentProcess in code that could use it

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

We should probably call XRE_IsParentProcess something like XRE_IsMainProcess now; file a followup for that?

Comment 5

[Jonathan Watt [:jwatt]]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> We should probably call XRE_IsParentProcess something like XRE_IsMainProcess
> now; file a followup for that?

Can do. But there are 504 instances of the string "XRE_IsParentProcess" in the source right now, and given that our plan is to switch fully to e10s I was wondering if this is worth doing.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4c76d6c5e72
part 1 - Add an XRE_IsE10sParentProcess function. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e02553dcf62
part 2 - Make use of XRE_IsE10sParentProcess in code that could use it. r=froydnj

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e4c76d6c5e72
https://hg.mozilla.org/mozilla-central/rev/1e02553dcf62