Rewrite DefaultPrefStore for performance

Rewrite DefaultPrefStore for performance

Derive directly from PrefStore, and ignore observers
since they can't usefully observe anything. Also remove now redundant members
from ValueMapPrefStore.

BUG=158330
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=166469

[Bernhard Bauer, 2012-11-06 18:25:02]

lgtm

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store.cc
File base/prefs/default_pref_store.cc (right):

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store...
base/prefs/default_pref_store.cc:14: const const_iterator entry =
prefs_.find(key);
Nit: I think a const const_iterator is overkill ;-)

[Mattias Nissler (ping if slow), 2012-11-07 08:24:02]

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store.cc
File base/prefs/default_pref_store.cc (right):

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store...
base/prefs/default_pref_store.cc:14: const const_iterator entry =
prefs_.find(key);
On 2012/11/06 18:25:02, Bernhard Bauer wrote:
> Nit: I think a const const_iterator is overkill ;-)

It's correct w.r.t. the style guide though.

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store.h
File base/prefs/default_pref_store.h (right):

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store...
base/prefs/default_pref_store.h:45: std::hash_map<std::string, base::Value*>
prefs_;
This should be a PrefValueMap.

[Anthony Berent, 2012-11-07 11:22:54]

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store.cc
File base/prefs/default_pref_store.cc (right):

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store...
base/prefs/default_pref_store.cc:14: const const_iterator entry =
prefs_.find(key);
On 2012/11/07 08:24:02, Mattias Nissler wrote:
> On 2012/11/06 18:25:02, Bernhard Bauer wrote:
> > Nit: I think a const const_iterator is overkill ;-)
> 
> It's correct w.r.t. the style guide though.

This code actually goes away as a result of Mattlas's other comment.

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store.h
File base/prefs/default_pref_store.h (right):

https://codereview.chromium.org/11358123/diff/1/base/prefs/default_pref_store...
base/prefs/default_pref_store.h:45: std::hash_map<std::string, base::Value*>
prefs_;
On 2012/11/07 08:24:02, Mattias Nissler wrote:
> This should be a PrefValueMap.

Done. Will be in next patch.

[Anthony Berent, 2012-11-07 11:33:37]

Please review again - changed to use PrefValueMap

[battre, 2012-11-07 11:50:49]

lgtm

https://codereview.chromium.org/11358123/diff/5001/base/prefs/default_pref_st...
File base/prefs/default_pref_store.cc (right):

https://codereview.chromium.org/11358123/diff/5001/base/prefs/default_pref_st...
base/prefs/default_pref_store.cc:6: #include "base/logging.h"
not necessary

https://codereview.chromium.org/11358123/diff/5001/base/prefs/default_pref_st...
File base/prefs/default_pref_store.h (right):

https://codereview.chromium.org/11358123/diff/5001/base/prefs/default_pref_st...
base/prefs/default_pref_store.h:22: 
nit: // Implementation of PrefStore.

[Mattias Nissler (ping if slow), 2012-11-07 12:56:19]

LGTM

[commit-bot: I haz the power, 2012-11-07 15:39:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aberent@chromium.org/11358123/5001

[commit-bot: I haz the power, 2012-11-07 18:06:03]

Change committed as 166469