Closed Bug 1362891 Opened 3 years ago Closed 3 years ago

Add a XRE_IsE10sParentProcess function

Categories

(Core :: General, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(2 files)

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.
Assignee: nobody → jwatt
Attachment #8865265 - Flags: review?(nfroyd)
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.
Attachment #8865265 - Flags: review?(nfroyd) → review+
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?
Attachment #8865266 - Flags: review?(nfroyd) → review+
(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.
Pushed by jwatt@jwatt.org:
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
https://hg.mozilla.org/mozilla-central/rev/e4c76d6c5e72
https://hg.mozilla.org/mozilla-central/rev/1e02553dcf62
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.