Closed Bug 568271 Opened 12 years ago Closed 12 years ago

e10s: remote nsIPrefBranch::GetComplexValue()

Categories

(Core :: Preferences: Backend, defect)

x86
Windows NT
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dwitte, Assigned: dwitte)

References

Details

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #506269 +++

I'm not sure OTOH who would need this in the content process, but it seems pretty important. (In the tree, its most common usage is to get localized strings.)
Attached patch remote itSplinter Review
This remotes GetComplexValue() for the case of nsIPrefLocalizedString & nsISupportsString, and asserts & fails for nsILocalFile & nsIRelativeFilePref. (If the content process is asking for those, something's wrong, and the consumer code needs to be remoted.)
Assignee: nobody → dwitte
Status: NEW → ASSIGNED
Attachment #447578 - Flags: review?(josh)
Bonus patch: we should be immediately bailing if the content process tries to modify anything.
Attachment #447579 - Flags: review?(josh)
Comment on attachment 447579 [details] [diff] [review]
assert & fail for nsIPrefBranch::Set* methods

Why not NS_ERROR instead of asserting false?

Either way, r=me.
Attachment #447579 - Flags: review?(josh) → review+
Comment on attachment 447578 [details] [diff] [review]
remote it

Same comment about NS_ERROR.  I like the removal of the crufty ADDREF stuff, as well as the un-indent, even if it makes the patch more complicated.

r=me
Attachment #447578 - Flags: review?(josh) → review+
http://hg.mozilla.org/projects/electrolysis/rev/ee0c3deb3ac4
http://hg.mozilla.org/projects/electrolysis/rev/0227403e8a15
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
This landing was mismerged in the following changeset:

http://hg.mozilla.org/mozilla-central/rev/155d97b3f8c9

What's needed to fix this?
Neil, could you be more specific about the problem?
Bug 564048 changed nsPrefBranch::GetComplexValue on trunk while this patch changed it in e10s. The merge did not take this into account and resulted in presumably incorrect code.
Depends on: 635665
I filed bug 635665 on the mismerge.
You need to log in before you can comment on or make changes to this bug.