Add a pref for a one-time wipe of gesture prefs

Add a pref for a one-time wipe of gesture prefs

Because of a bug in the gesture config UI (chrome://gesture), whenever a
user tried to reset a gesture preference to default, the preference was
set to whatever the default value of the preference was at that time,
instead of unsetting the preference. This bug is now fixed, but many
users may have unintentionally set their gesture preferences to the
default values.

A temporary preference is introduced to unset all gesture preferences
once.  This preference will be removed in a proper time in future when
we think that a good portion of users have their gesture preferences
unset.

BUG=251858
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216775

[mohsen, 2013-08-02 20:58:25]

Please take a look...

[Bernhard Bauer, 2013-08-02 22:18:40]

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:157:
prefs->ClearPref(kPrefsToObserve[i]);
Could you do this only if the pref has the default value?

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:164:
prefs->ClearPref(kFlingTouchpadPrefs[i]);
Nit: newline.

https://codereview.chromium.org/21983002/diff/1/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/21983002/diff/1/chrome/common/pref_names.cc#n...
chrome/common/pref_names.cc:2466: // A temporary pref to do a one-time wipe of
gesture preferences.
If the pref is temporary, maybe add a TODO to remove it? (Ideally with a
specific condition, i.e. on date X, or when Y% of the population have updated to
a version that runs the code).

[mohsen, 2013-08-07 05:58:04]

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:157:
prefs->ClearPref(kPrefsToObserve[i]);
On 2013/08/02 22:18:40, Bernhard Bauer wrote:
> Could you do this only if the pref has the default value?

I thought all prefs have default values. Isn't it the case?

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:164:
prefs->ClearPref(kFlingTouchpadPrefs[i]);
On 2013/08/02 22:18:40, Bernhard Bauer wrote:
> Nit: newline.

Done.

https://codereview.chromium.org/21983002/diff/1/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/21983002/diff/1/chrome/common/pref_names.cc#n...
chrome/common/pref_names.cc:2466: // A temporary pref to do a one-time wipe of
gesture preferences.
On 2013/08/02 22:18:40, Bernhard Bauer wrote:
> If the pref is temporary, maybe add a TODO to remove it? (Ideally with a
> specific condition, i.e. on date X, or when Y% of the population have updated
to
> a version that runs the code).

Done.

[Bernhard Bauer, 2013-08-07 06:01:42]

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:157:
prefs->ClearPref(kPrefsToObserve[i]);
On 2013/08/07 05:58:04, mohsen wrote:
> On 2013/08/02 22:18:40, Bernhard Bauer wrote:
> > Could you do this only if the pref has the default value?
> 
> I thought all prefs have default values. Isn't it the case?

Sorry, I meant if the current user-defined value of the pref matches the default
value. That way, the user won't even see any immediate change, just if the
default changes.

[mohsen, 2013-08-07 16:28:11]

cc'ing rbyers@ as he might have some thoughts here.

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:157:
prefs->ClearPref(kPrefsToObserve[i]);
On 2013/08/07 06:01:43, Bernhard Bauer wrote:
> On 2013/08/07 05:58:04, mohsen wrote:
> > On 2013/08/02 22:18:40, Bernhard Bauer wrote:
> > > Could you do this only if the pref has the default value?
> > 
> > I thought all prefs have default values. Isn't it the case?
> 
> Sorry, I meant if the current user-defined value of the pref matches the
default
> value. That way, the user won't even see any immediate change, just if the
> default changes.

Oh, I see. This makes sense. But, the problem is that some default values might
have changed while the original bug caused the prefs to be set instead of being
cleared. So, those prefs are meant to be default but have different values than
the current default value. And, since this chrome://gesture is an internal page
and has a few users, we wanted to take this opportunity to reset all these prefs
once - e.g. for those who might have forgotten about their custom settings.

[Bernhard Bauer, 2013-08-07 16:43:51]

LGTM

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:157:
prefs->ClearPref(kPrefsToObserve[i]);
On 2013/08/07 16:28:11, mohsen wrote:
> On 2013/08/07 06:01:43, Bernhard Bauer wrote:
> > On 2013/08/07 05:58:04, mohsen wrote:
> > > On 2013/08/02 22:18:40, Bernhard Bauer wrote:
> > > > Could you do this only if the pref has the default value?
> > > 
> > > I thought all prefs have default values. Isn't it the case?
> > 
> > Sorry, I meant if the current user-defined value of the pref matches the
> default
> > value. That way, the user won't even see any immediate change, just if the
> > default changes.
> 
> Oh, I see. This makes sense. But, the problem is that some default values
might
> have changed while the original bug caused the prefs to be set instead of
being
> cleared. So, those prefs are meant to be default but have different values
than
> the current default value. And, since this chrome://gesture is an internal
page
> and has a few users, we wanted to take this opportunity to reset all these
prefs
> once - e.g. for those who might have forgotten about their custom settings.

Hm, ok.

[mohsen, 2013-08-07 17:00:42]

pkasting@: for OWNERS in chrome/browser/ui/, please.

[Rick Byers, 2013-08-07 17:20:08]

On 2013/08/07 16:43:51, Bernhard Bauer wrote:
> LGTM
> 
>
https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
> File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):
> 
>
https://codereview.chromium.org/21983002/diff/1/chrome/browser/ui/gesture_pre...
> chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:157:
> prefs->ClearPref(kPrefsToObserve[i]);
> On 2013/08/07 16:28:11, mohsen wrote:
> > On 2013/08/07 06:01:43, Bernhard Bauer wrote:
> > > On 2013/08/07 05:58:04, mohsen wrote:
> > > > On 2013/08/02 22:18:40, Bernhard Bauer wrote:
> > > > > Could you do this only if the pref has the default value?
> > > > 
> > > > I thought all prefs have default values. Isn't it the case?
> > > 
> > > Sorry, I meant if the current user-defined value of the pref matches the
> > default
> > > value. That way, the user won't even see any immediate change, just if the
> > > default changes.
> > 
> > Oh, I see. This makes sense. But, the problem is that some default values
> might
> > have changed while the original bug caused the prefs to be set instead of
> being
> > cleared. So, those prefs are meant to be default but have different values
> than
> > the current default value. And, since this chrome://gesture is an internal
> page
> > and has a few users, we wanted to take this opportunity to reset all these
> prefs
> > once - e.g. for those who might have forgotten about their custom settings.
> 
> Hm, ok.

Yep, the legitimate scenarios for using a pref at all are small (just when we're
field testing different alternatives), so doing a one-time reset to default is
probably the better choice.

> pkasting@: for OWNERS in chrome/browser/ui/, please.

Peter!  Funny that after raising concerns with this feature on Francois's post
about it, you're now reviewing it.  Now's your chance <grin>!

[Peter Kasting, 2013-08-09 19:03:54]

LGTM

https://codereview.chromium.org/21983002/diff/8001/chrome/browser/ui/gesture_...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/21983002/diff/8001/chrome/browser/ui/gesture_...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:154: // Do a one-time
wipe of all gesture preferences.
Nit: Say why, as well as when this will be removed and how we'll make that
determination.

[mohsen, 2013-08-09 20:12:34]

https://codereview.chromium.org/21983002/diff/8001/chrome/browser/ui/gesture_...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/21983002/diff/8001/chrome/browser/ui/gesture_...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:154: // Do a one-time
wipe of all gesture preferences.
On 2013/08/09 19:03:54, Peter Kasting wrote:
> Nit: Say why, as well as when this will be removed and how we'll make that
> determination.

Done.

[commit-bot: I haz the power, 2013-08-09 20:15:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mohsen@chromium.org/21983002/20001

[commit-bot: I haz the power, 2013-08-09 21:33:32]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-08-09 21:46:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mohsen@chromium.org/21983002/20001

[commit-bot: I haz the power, 2013-08-09 23:23:13]

Change committed as 216775