Closed Bug 1174857 Opened 9 years ago Closed 9 years ago

[e10s] Safe mode isn't sent to content process

Categories

(Core :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
e10s m8+ ---
firefox41 --- affected

People

(Reporter: BenWa, Assigned: BenWa)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Right now any content process side check of safe mode will always return true. AFAIK no one has done an audit of the consequence of these safe mode checks being incorrect.

We should fix it or audit and understand the implication of this.
Requesting e10s triage since safe-mode is barely tested but it is an important troubleshooting tool.
tracking-e10s: --- → ?
mconley says a good starting point is to see if we can use bug 1162838 to send the data to the content process.
Blocks: 1172491
How about I propagate the safemode attribute from InitProcessAttributes?

https://dxr.mozilla.org/mozilla-central/source/dom/ipc/ContentChild.cpp?from=ContentChild.cpp&case=true#681

In the parent process safemode is setup pretty early so ideally we can set it up early in the child process. InitProcessAttributes feels like a good place for that.
Flags: needinfo?(wmccloskey)
Attached patch patch (obsolete) — Splinter Review
How about this? This makes it such that part 1 in bug 1171682 works in e10s.
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Flags: needinfo?(wmccloskey)
Attachment #8623090 - Flags: review?(wmccloskey)
Comment on attachment 8623090 [details] [diff] [review]
patch

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

::: dom/ipc/ContentChild.cpp
@@ +678,5 @@
>                                    XRE_GetProcessType());
>  #endif
>  
> +    bool isSafeMode = false;
> +    SendGetProcessAttributes(&mID, &mIsForApp, &mIsForBrowser, &isSafeMode);

I think isSafeMode makes more sense as part of SendGetXPCOMProcessAttributes. I don't know why we have both GetXPCOMProcessAttributes and GetProcessAttributes; they seem to do the same thing at basically the same time. But the XPCOM one sends more random stuff like this, so it seems like a better fit.

::: dom/ipc/ContentParent.cpp
@@ +3196,5 @@
>  {
>      *aCpId = mChildID;
>      *aIsForApp = IsForApp();
>      *aIsForBrowser = mIsForBrowser;
> +    *aIsSafeMode = gfxPlatform::InSafeMode();

I'd rather not make this depend on some random gfx code. Can you add GetSafeMode to nsAppRunner.h just like SetSafeMode? That seems a lot more direct.
Attachment #8623090 - Flags: review?(wmccloskey) → review+
Attached patch patch, r=billmSplinter Review
Attachment #8623090 - Attachment is obsolete: true
Attachment #8623289 - Flags: review+
Flags: needinfo?(bgirard)
Looks like the failure are coming from my qparent:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=25e99bc12482

Landing this now.
Flags: needinfo?(bgirard)
Backout: 
https://hg.mozilla.org/releases/mozilla-beta/rev/f069ca975718

I didn't notice that gfxPlatform::InSafeMode() was missing from the beta branch.
Flags: needinfo?(bgirard)
https://hg.mozilla.org/mozilla-central/rev/ce7146d40eb9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Looks like it didn't match the follow-up with the backout.
You need to log in before you can comment on or make changes to this bug.