Add a preference to enable/disable ID persistence in bookmark model and...

Add a preference to enable/disable ID persistence in bookmark model and
add methods in bookmark model to get/set this preference.


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

[sky, 2009-05-27 14:44:24]

http://codereview.chromium.org/113882/diff/1/2
File chrome/browser/bookmarks/bookmark_model.cc (right):

http://codereview.chromium.org/113882/diff/1/2#newcode59
Line 59: const wchar_t kPrefPersistIDs[] = L"bookmarks.persist_ids";
nit: don't indent this.

http://codereview.chromium.org/113882/diff/1/2#newcode386
Line 386: PrefService* pref_service = profile_->GetPrefs();
profile_ may be NULL during testing.

http://codereview.chromium.org/113882/diff/1/2#newcode391
Line 391: PrefService* pref_service = profile_->GetPrefs();
same thing here.

http://codereview.chromium.org/113882/diff/1/2#newcode709
Line 709: PrefService* pref_service = profile_->GetPrefs();
And here.

http://codereview.chromium.org/113882/diff/1/3
File chrome/browser/bookmarks/bookmark_model.h (right):

http://codereview.chromium.org/113882/diff/1/3#newcode329
Line 329: void SetShouldPersistIDs(bool value);
Nuke the should, just SetPersistIDs, and PersistIDs.

[Munjal (Google), 2009-05-27 19:19:33]

http://codereview.chromium.org/113882/diff/1/2
File chrome/browser/bookmarks/bookmark_model.cc (right):

http://codereview.chromium.org/113882/diff/1/2#newcode59
Line 59: const wchar_t kPrefPersistIDs[] = L"bookmarks.persist_ids";
On 2009/05/27 14:44:24, sky wrote:
> nit: don't indent this.

Done. VS sometimes automatically indents.

http://codereview.chromium.org/113882/diff/1/2#newcode386
Line 386: PrefService* pref_service = profile_->GetPrefs();
On 2009/05/27 14:44:24, sky wrote:
> profile_ may be NULL during testing.

Done.

http://codereview.chromium.org/113882/diff/1/2#newcode391
Line 391: PrefService* pref_service = profile_->GetPrefs();
On 2009/05/27 14:44:24, sky wrote:
> same thing here.

Done.

http://codereview.chromium.org/113882/diff/1/2#newcode709
Line 709: PrefService* pref_service = profile_->GetPrefs();
On 2009/05/27 14:44:24, sky wrote:
> And here.

Done.

http://codereview.chromium.org/113882/diff/1/3
File chrome/browser/bookmarks/bookmark_model.h (right):

http://codereview.chromium.org/113882/diff/1/3#newcode329
Line 329: void SetShouldPersistIDs(bool value);
On 2009/05/27 14:44:24, sky wrote:
> Nuke the should, just SetPersistIDs, and PersistIDs.

Done.

[sky, 2009-05-27 19:34:51]

LGTM with the following change.

http://codereview.chromium.org/113882/diff/1002/12
File chrome/browser/bookmarks/bookmark_model.cc (right):

http://codereview.chromium.org/113882/diff/1002/12#newcode388
Line 388: return persist_ids_;
You may as well put this in the and inline it now that it's trivial.

[Munjal (Google), 2009-05-27 20:00:05]

http://codereview.chromium.org/113882/diff/1002/12
File chrome/browser/bookmarks/bookmark_model.cc (right):

http://codereview.chromium.org/113882/diff/1002/12#newcode388
Line 388: return persist_ids_;
On 2009/05/27 19:34:51, sky wrote:
> You may as well put this in the and inline it now that it's trivial.

Done.