Skip migrating for default shown sections prefs.

Skip migrating for default shown sections prefs.

BUG=chromium-os:9737
TEST=Verify fix for chromium-os:9737

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

[xiyuan, 2010-12-01 19:52:33]

MigrateUserPrefs is also called for a brand new user profile and clears out the
minimized section masks, which we want to keep for ChromeOS.

This fix skips the migration if default value is used for shown sections prefs.

Another possible fix would be keep the minimized section bits during migration.

Let me know which way is better.
Thanks.

[Aaron Boodman, 2010-12-01 21:57:48]

I think this will result in the current version never getting set until the user
changes the expanded sections. It might be better to move your defaulting logic
for Chrome OS here, so it is all in one place.

[xiyuan, 2010-12-01 22:10:34]

On 2010/12/01 21:57:48, Aaron Boodman wrote:
> I think this will result in the current version never getting set until the
user
> changes the expanded sections. It might be better to move your defaulting
logic
> for Chrome OS here, so it is all in one place.

If user never changes anything on the NTP, should we show them the default
layout always?

The old code will set an explicit shown sections value. Thus user would stick to
this layout regardless of the default values again. Is this the desired
behavior?

[Aaron Boodman, 2010-12-01 23:01:12]

On 2010/12/01 22:10:34, xiyuan wrote:
> On 2010/12/01 21:57:48, Aaron Boodman wrote:
> > I think this will result in the current version never getting set until the
> user
> > changes the expanded sections. It might be better to move your defaulting
> logic
> > for Chrome OS here, so it is all in one place.
> 
> If user never changes anything on the NTP, should we show them the default
> layout always?
> 
> The old code will set an explicit shown sections value. Thus user would stick
to
> this layout regardless of the default values again. Is this the desired
> behavior?

Nevermind, I misread the code.

I'm a little worried that this will have unintended consequences in the future
if we add more migration code for future versions, and the current value at that
point just happens to match the default value.

But I don't have a better suggestion, so LGTM

[xiyuan, 2010-12-01 23:11:48]

On 2010/12/01 23:01:12, Aaron Boodman wrote:
> On 2010/12/01 22:10:34, xiyuan wrote:
> > On 2010/12/01 21:57:48, Aaron Boodman wrote:
> > > I think this will result in the current version never getting set until
the
> > user
> > > changes the expanded sections. It might be better to move your defaulting
> > logic
> > > for Chrome OS here, so it is all in one place.
> > 
> > If user never changes anything on the NTP, should we show them the default
> > layout always?
> > 
> > The old code will set an explicit shown sections value. Thus user would
stick
> to
> > this layout regardless of the default values again. Is this the desired
> > behavior?
> 
> Nevermind, I misread the code.
> 
> I'm a little worried that this will have unintended consequences in the future
> if we add more migration code for future versions, and the current value at
that
> point just happens to match the default value.
> 
> But I don't have a better suggestion, so LGTM

PrefService::Preference::IsDefaultValue does not depend on the prefs value. It
checks if the value is from default prefs value store or not. I hope this would
make you feel better. :)

As long as we keep the default value of kNTPShownSections consistent with the
migration logic (i.e. no invalid default kNTPShownSections values), we should be
fine.