Pull GetMutableValue from Perisistent to WritablePrefStore.

Pull GetMutableValue from Perisistent to WritablePrefStore.

GetMutableValue is useful for updating parts of a DictionaryValue. I want to use this for a DictionaryValue-backed preference store, and I don't want to implement the rest of the functionality of PersistentPrefStore.

BUG=372547

[erikwright (departed), 2014-06-11 18:42:09]

gab: PTAL.

[gab, 2014-06-11 19:01:32]

Please explain what motivates this change in the CL description.

I will defer to Bernhard for the API changes (as previously requested from me
when I became an owner of prefs).

[erikwright (departed), 2014-06-11 21:11:16]

Please see https://codereview.chromium.org/324493002/ for a work-in-progress CL
that demonstrates how I plan to use this.

[gab, 2014-06-12 14:55:16]

Both Mattias and Bernhard appear to be OOO.

This API change is simple and reasonable enough that I'll lgtm for now and they
can object when they come back if need be.

++CC battre and pamg for the sake of completeness if any of them know a reason
not to change this API.

Cheers,
Gab

[gab, 2014-06-12 14:56:54]

+CC pam (see above) -- are pam/pamg the same person?! :-)

[battre, 2014-06-13 09:16:00]

https://codereview.chromium.org/331433002/diff/1/base/prefs/persistent_pref_s...
File base/prefs/persistent_pref_store.h (right):

https://codereview.chromium.org/331433002/diff/1/base/prefs/persistent_pref_s...
base/prefs/persistent_pref_store.h:53: virtual void SetValueSilently(const
std::string& key, base::Value* value) = 0;
What do you think of moving this function as well? Then PersistentPrefStore
contains only logic related to persistence.

https://codereview.chromium.org/331433002/diff/1/base/prefs/writeable_pref_st...
File base/prefs/writeable_pref_store.h (right):

https://codereview.chromium.org/331433002/diff/1/base/prefs/writeable_pref_st...
base/prefs/writeable_pref_store.h:39: 
Nit: one blank line too much.

[gab, 2014-06-13 14:42:34]

Thanks for taking a look Dominic!

https://codereview.chromium.org/331433002/diff/1/base/prefs/persistent_pref_s...
File base/prefs/persistent_pref_store.h (right):

https://codereview.chromium.org/331433002/diff/1/base/prefs/persistent_pref_s...
base/prefs/persistent_pref_store.h:53: virtual void SetValueSilently(const
std::string& key, base::Value* value) = 0;
On 2014/06/13 09:16:00, battre wrote:
> What do you think of moving this function as well? Then PersistentPrefStore
> contains only logic related to persistence.

SGTM.

[Bernhard Bauer, 2014-06-16 08:05:03]

LGTM, and I support Dominic's suggestion as well.

[Pam (message me for reviews), 2014-06-16 22:42:29]

On 2014/06/16 08:05:03, Bernhard Bauer wrote:
> LGTM, and I support Dominic's suggestion as well.

pam and pamg are the same person, and I +n all the previous comments, including
the LGTM.

- Pam

[gab, 2014-06-19 19:16:44]

Looks like this was (unintentionally?) slipped into a rebase (patch set 13) in
https://codereview.chromium.org/324493002 -- I missed that in that review as I
skipped "rebase" patch sets assuming they were irrelevant...

That's too bad, I would have prefered a separate CL, but I guess it's okay since
everybody agreed here... :(

Gab

[erikwright (departed), 2014-06-23 15:38:30]

On 2014/06/19 19:16:44, gab wrote:
> Looks like this was (unintentionally?) slipped into a rebase (patch set 13) in
> https://codereview.chromium.org/324493002 -- I missed that in that review as I
> skipped "rebase" patch sets assuming they were irrelevant...
> 
> That's too bad, I would have prefered a separate CL, but I guess it's okay
since
> everybody agreed here... :(
> 
> Gab

It would appear that I did accidentally commit this change.

In the end it wasn't actually required, although I believe it is a correct
change. I will go ahead and follow-up with Dominic's suggestion today.

[gab, 2014-06-23 15:40:53]

On 2014/06/23 15:38:30, erikwright wrote:
> On 2014/06/19 19:16:44, gab wrote:
> > Looks like this was (unintentionally?) slipped into a rebase (patch set 13)
in
> > https://codereview.chromium.org/324493002 -- I missed that in that review as
I
> > skipped "rebase" patch sets assuming they were irrelevant...
> > 
> > That's too bad, I would have prefered a separate CL, but I guess it's okay
> since
> > everybody agreed here... :(
> > 
> > Gab
> 
> It would appear that I did accidentally commit this change.
> 
> In the end it wasn't actually required, although I believe it is a correct
> change. I will go ahead and follow-up with Dominic's suggestion today.

Thanks, closing this issue.

[gab, 2014-07-11 12:31:56]

On 2014/06/23 15:40:53, gab wrote:
> On 2014/06/23 15:38:30, erikwright wrote:
> > On 2014/06/19 19:16:44, gab wrote:
> > > Looks like this was (unintentionally?) slipped into a rebase (patch set
13)
> in
> > > https://codereview.chromium.org/324493002 -- I missed that in that review
as
> I
> > > skipped "rebase" patch sets assuming they were irrelevant...
> > > 
> > > That's too bad, I would have prefered a separate CL, but I guess it's okay
> > since
> > > everybody agreed here... :(
> > > 
> > > Gab
> > 
> > It would appear that I did accidentally commit this change.
> > 
> > In the end it wasn't actually required, although I believe it is a correct
> > change. I will go ahead and follow-up with Dominic's suggestion today.
> 
> Thanks, closing this issue.

ping? I didn't see the promised CL above get implemented?