Remove PrefService::ScheduleSavePersistentPrefs and SavePersistentPrefs.

Remove PrefService::ScheduleSavePersistentPrefs and change SavePersistentPrefs calls to CommitPendingWrites.

TBR=ncarter@chromium.org
BUG=99306
TEST=none


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

[Bernhard Bauer, 2011-10-11 18:32:41]

Please review.

Nick: OWNERS approval for sync
Miranda: OWNERS approval for profiles
Mattias: Rest :)

[battre, 2011-10-11 18:46:22]

http://codereview.chromium.org/8198007/diff/6001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/user_manager.cc (right):

http://codereview.chromium.org/8198007/diff/6001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/user_manager.cc:77:
local_state->CommitPendingWrite();
DictionaryPrefUpdate triggers a ScheduleSavePersistentPrefs() on *destruction*.
oauth_status_update is still on the stack at the time of calling
CommitPendingWrite. The same holds for other occurrences of this pattern.

[Miranda Callahan, 2011-10-11 18:57:13]

LGTM for profile code changes.

On 2011/10/11 18:46:22, battre wrote:
>
http://codereview.chromium.org/8198007/diff/6001/chrome/browser/chromeos/logi...
> File chrome/browser/chromeos/login/user_manager.cc (right):
> 
>
http://codereview.chromium.org/8198007/diff/6001/chrome/browser/chromeos/logi...
> chrome/browser/chromeos/login/user_manager.cc:77:
> local_state->CommitPendingWrite();
> DictionaryPrefUpdate triggers a ScheduleSavePersistentPrefs() on
*destruction*.
> oauth_status_update is still on the stack at the time of calling
> CommitPendingWrite. The same holds for other occurrences of this pattern.

[Bernhard Bauer, 2011-10-11 19:24:20]

http://codereview.chromium.org/8198007/diff/6001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/user_manager.cc (right):

http://codereview.chromium.org/8198007/diff/6001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/user_manager.cc:77:
local_state->CommitPendingWrite();
On 2011/10/11 18:46:23, battre wrote:
> DictionaryPrefUpdate triggers a ScheduleSavePersistentPrefs() on
*destruction*.
> oauth_status_update is still on the stack at the time of calling
> CommitPendingWrite. The same holds for other occurrences of this pattern.

Done.

[Mattias Nissler (ping if slow), 2011-10-12 09:23:39]

In general looks good, some minor comments.

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/chromeos/acce...
File chrome/browser/chromeos/accessibility_util.cc (right):

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/chromeos/acce...
chrome/browser/chromeos/accessibility_util.cc:98: // TODO(dmazzoni) remove once
http://crosbug.com/19491 is fixed.
That bug is closed. Remove? Probably better not to do it in this CL, but can you
ping dmazzoni?

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_impl.cc (left):

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_impl.cc:928:
prefs_->ScheduleSavePersistentPrefs();
At this point I think we actually want a CommitPendingWrite() in order to make
sure that crashes happening immediately after are detected as unclean exit.

http://codereview.chromium.org/8198007/diff/7002/chrome/common/json_pref_store.h
File chrome/common/json_pref_store.h (right):

http://codereview.chromium.org/8198007/diff/7002/chrome/common/json_pref_stor...
chrome/common/json_pref_store.h:38: const base::Value** result) const OVERRIDE;
#include "base/compiler_specific.h"

[Bernhard Bauer, 2011-10-12 11:00:10]

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_impl.cc (left):

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_impl.cc:928:
prefs_->ScheduleSavePersistentPrefs();
On 2011/10/12 09:23:39, Mattias Nissler wrote:
> At this point I think we actually want a CommitPendingWrite() in order to make
> sure that crashes happening immediately after are detected as unclean exit.

Hm, could this adversely affect startup performance, or am I being overly
paranoid here?

http://codereview.chromium.org/8198007/diff/7002/chrome/common/json_pref_store.h
File chrome/common/json_pref_store.h (right):

http://codereview.chromium.org/8198007/diff/7002/chrome/common/json_pref_stor...
chrome/common/json_pref_store.h:38: const base::Value** result) const OVERRIDE;
On 2011/10/12 09:23:39, Mattias Nissler wrote:
> #include "base/compiler_specific.h"

Done.

[Bernhard Bauer, 2011-10-12 11:02:05]

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/chromeos/acce...
File chrome/browser/chromeos/accessibility_util.cc (right):

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/chromeos/acce...
chrome/browser/chromeos/accessibility_util.cc:98: // TODO(dmazzoni) remove once
http://crosbug.com/19491 is fixed.
On 2011/10/12 09:23:39, Mattias Nissler wrote:
> That bug is closed. Remove? Probably better not to do it in this CL, but can
you
> ping dmazzoni?

OK. Dominic, can you take a look? :-D

[Mattias Nissler (ping if slow), 2011-10-12 11:02:26]

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_impl.cc (left):

http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_impl.cc:928:
prefs_->ScheduleSavePersistentPrefs();
On 2011/10/12 11:00:11, Bernhard Bauer wrote:
> On 2011/10/12 09:23:39, Mattias Nissler wrote:
> > At this point I think we actually want a CommitPendingWrite() in order to
make
> > sure that crashes happening immediately after are detected as unclean exit.
> 
> Hm, could this adversely affect startup performance, or am I being overly
> paranoid here?

Ah, the old fast vs. correct tradeoff :) I'm OK either way.

[Mattias Nissler (ping if slow), 2011-10-13 12:49:49]

On 2011/10/12 11:02:26, Mattias Nissler wrote:
>
http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
> File chrome/browser/profiles/profile_impl.cc (left):
> 
>
http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
> chrome/browser/profiles/profile_impl.cc:928:
> prefs_->ScheduleSavePersistentPrefs();
> On 2011/10/12 11:00:11, Bernhard Bauer wrote:
> > On 2011/10/12 09:23:39, Mattias Nissler wrote:
> > > At this point I think we actually want a CommitPendingWrite() in order to
> make
> > > sure that crashes happening immediately after are detected as unclean
exit.
> > 
> > Hm, could this adversely affect startup performance, or am I being overly
> > paranoid here?
> 
> Ah, the old fast vs. correct tradeoff :) I'm OK either way.

OK as in LGTM, in case I wasn't clear.

[Bernhard Bauer, 2011-10-13 14:16:37]

On 2011/10/13 12:49:49, Mattias Nissler wrote:
> On 2011/10/12 11:02:26, Mattias Nissler wrote:
> >
>
http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
> > File chrome/browser/profiles/profile_impl.cc (left):
> > 
> >
>
http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
> > chrome/browser/profiles/profile_impl.cc:928:
> > prefs_->ScheduleSavePersistentPrefs();
> > On 2011/10/12 11:00:11, Bernhard Bauer wrote:
> > > On 2011/10/12 09:23:39, Mattias Nissler wrote:
> > > > At this point I think we actually want a CommitPendingWrite() in order
to
> > make
> > > > sure that crashes happening immediately after are detected as unclean
> exit.
> > > 
> > > Hm, could this adversely affect startup performance, or am I being overly
> > > paranoid here?
> > 
> > Ah, the old fast vs. correct tradeoff :) I'm OK either way.
> 
> OK as in LGTM, in case I wasn't clear.

Nick, can you take a look at sync/?

[Bernhard Bauer, 2011-10-17 15:36:17]

On 2011/10/13 14:16:37, Bernhard Bauer wrote:
> On 2011/10/13 12:49:49, Mattias Nissler wrote:
> > On 2011/10/12 11:02:26, Mattias Nissler wrote:
> > >
> >
>
http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
> > > File chrome/browser/profiles/profile_impl.cc (left):
> > > 
> > >
> >
>
http://codereview.chromium.org/8198007/diff/7002/chrome/browser/profiles/prof...
> > > chrome/browser/profiles/profile_impl.cc:928:
> > > prefs_->ScheduleSavePersistentPrefs();
> > > On 2011/10/12 11:00:11, Bernhard Bauer wrote:
> > > > On 2011/10/12 09:23:39, Mattias Nissler wrote:
> > > > > At this point I think we actually want a CommitPendingWrite() in order
> to
> > > make
> > > > > sure that crashes happening immediately after are detected as unclean
> > exit.
> > > > 
> > > > Hm, could this adversely affect startup performance, or am I being
overly
> > > > paranoid here?
> > > 
> > > Ah, the old fast vs. correct tradeoff :) I'm OK either way.
> > 
> > OK as in LGTM, in case I wasn't clear.
> 
> Nick, can you take a look at sync/?

Ping? :)

[commit-bot: I haz the power, 2011-12-27 15:19:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/8198007/41055

[commit-bot: I haz the power, 2011-12-27 16:41:52]

Change committed as 115829