568271 - e10s: remote nsIPrefBranch::GetComplexValue()

Status: RESOLVED FIXED

(Core :: Preferences: Backend, defect)

Description

[[e-mail]]

+++ 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.)

Comment 1

[[e-mail]]

Attachment: remote it

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.)

Comment 2

[[e-mail]]

Attachment: assert & fail for nsIPrefBranch::Set* methods

Bonus patch: we should be immediately bailing if the content process tries to modify anything.

Comment 3

[Josh Matthews [:jdm]]

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.

Comment 4

[Josh Matthews [:jdm]]

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

Comment 5

[[e-mail]]

http://hg.mozilla.org/projects/electrolysis/rev/ee0c3deb3ac4
http://hg.mozilla.org/projects/electrolysis/rev/0227403e8a15

Comment 6

[[e-mail]]

This landing was mismerged in the following changeset:

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

What's needed to fix this?

Comment 7

[Josh Matthews [:jdm]]

Neil, could you be more specific about the problem?

Comment 8

[[e-mail]]

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.

Comment 9

[[e-mail]]

I filed bug 635665 on the mismerge.