Closed
Bug 1174857
Opened 9 years ago
Closed 9 years ago
[e10s] Safe mode isn't sent to content process
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: BenWa, Assigned: BenWa)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
7.78 KB,
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
Requesting e10s triage since safe-mode is barely tested but it is an important troubleshooting tool.
tracking-e10s:
--- → ?
Assignee | ||
Comment 2•9 years ago
|
||
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
Assignee | ||
Comment 3•9 years ago
|
||
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)
Assignee | ||
Comment 4•9 years ago
|
||
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)
Updated•9 years ago
|
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+
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8623090 -
Attachment is obsolete: true
Attachment #8623289 -
Flags: review+
Assignee | ||
Comment 7•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7864171d7d12
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(bgirard)
Assignee | ||
Comment 8•9 years ago
|
||
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)
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8601df335c1
Comment 11•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/b4060e527bd2
Comment 12•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/545afafa3df4
Assignee | ||
Comment 13•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2a19a8f46a2
Flags: needinfo?(bgirard)
Assignee | ||
Comment 14•9 years ago
|
||
Backout: https://hg.mozilla.org/releases/mozilla-beta/rev/f069ca975718 I didn't notice that gfxPlatform::InSafeMode() was missing from the beta branch.
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(bgirard)
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ce7146d40eb9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Assignee | ||
Comment 16•9 years ago
|
||
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.
Description
•