Remove gesture configuration from about://gesture

Remove gesture configuration from about://gesture.

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

[tdresser, 2014-06-02 20:39:56]

Charlie, would this negatively impact any of your work?

[charliemooney, 2014-06-02 21:14:31]

On 2014/06/02 20:39:56, tdresser wrote:
> Charlie, would this negatively impact any of your work?

Nope, this should be fine to reduce the number of parameters.  Thanks for
patching this!

[charliemooney, 2014-06-02 21:16:09]

On 2014/06/02 21:14:31, charliemooney wrote:
> On 2014/06/02 20:39:56, tdresser wrote:
> > Charlie, would this negatively impact any of your work?
> 
> Nope, this should be fine to reduce the number of parameters.  Thanks for
> patching this!

LGTM

[tdresser, 2014-06-02 21:21:41]

Sadrul, can you make sure this is reasonable?

Rick and I agree that about://gesture isn't being used significantly for gesture
detection configuration, and the way we're hosting multiple gesture detectors
with different configurations with the unified GR would make keeping this
functionality kinda tricky.

[sadrul, 2014-06-03 10:25:47]

Add BUG= to the CL description.

https://codereview.chromium.org/313473002/diff/20001/chrome/browser/ui/gestur...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/313473002/diff/20001/chrome/browser/ui/gestur...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:181:
prefs->ClearPref(kShowPressDelayInMS);
1. Maybe you need to FindPreference() first for each of these before doing
ClearPref (because ClearPref() has a NOTREACHED() for unregistered prefs, and a
reboot after clearing the prefs will probably trigger that?)

2. Why not a kFlagsToRemove array of prefs, instead of calling each by name?

https://codereview.chromium.org/313473002/diff/20001/chrome/browser/ui/gestur...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:345:
registry->RegisterDoublePref(
Why register these anymore?

[tdresser, 2014-06-03 12:13:04]

https://codereview.chromium.org/313473002/diff/20001/chrome/browser/ui/gestur...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/313473002/diff/20001/chrome/browser/ui/gestur...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:181:
prefs->ClearPref(kShowPressDelayInMS);
On 2014/06/03 10:25:47, sadrul wrote:
> 1. Maybe you need to FindPreference() first for each of these before doing
> ClearPref (because ClearPref() has a NOTREACHED() for unregistered prefs, and
a
> reboot after clearing the prefs will probably trigger that?)
> 
> 2. Why not a kFlagsToRemove array of prefs, instead of calling each by name?

Registering the prefs to be removed stops the NOTREACHED() from being hit, but
using FindPreference is clearly better, thanks.

Done.

https://codereview.chromium.org/313473002/diff/20001/chrome/browser/ui/gestur...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:345:
registry->RegisterDoublePref(
On 2014/06/03 10:25:47, sadrul wrote:
> Why register these anymore?

Hmmm, I was using
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gesture_pre...
as a model (originally by rjkroege).

There are two options, one is to only call ClearPref if FindPreference returns
something, the other option is to always register all the prefs. 

I definitely prefer the way you're proposing.
Done.

[sadrul, 2014-06-03 16:33:29]

lgtm

[tdresser, 2014-06-03 16:38:01]

xiyuan@, can you take a look at
- chrome/browser/resources/gesture_config.js
- chrome/browser/ui/webui/chromeos/salsa_ui.cc

msw@, can you take a look at
- chrome/browser/ui/gesture_prefs_observer_factory_aura.cc

[xiyuan, 2014-06-03 16:42:50]

lgtm

[msw, 2014-06-03 17:05:55]

gesture_prefs_observer_factory_aura.cc lgtm with a nit.

https://codereview.chromium.org/313473002/diff/40001/chrome/browser/ui/gestur...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/313473002/diff/40001/chrome/browser/ui/gestur...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:187:
prefs->ClearPref(kPrefsToRemove[i]);
nit: indentation

[tdresser, 2014-06-03 18:02:32]

https://codereview.chromium.org/313473002/diff/40001/chrome/browser/ui/gestur...
File chrome/browser/ui/gesture_prefs_observer_factory_aura.cc (right):

https://codereview.chromium.org/313473002/diff/40001/chrome/browser/ui/gestur...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:187:
prefs->ClearPref(kPrefsToRemove[i]);
On 2014/06/03 17:05:55, msw wrote:
> nit: indentation

Done.

[tdresser, 2014-06-03 18:02:44]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2014-06-03 18:06:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdresser@chromium.org/313473002/60001

[tdresser, 2014-06-03 19:01:57]

The CQ bit was unchecked by tdresser@chromium.org

[tdresser, 2014-06-03 19:02:02]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2014-06-03 19:02:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdresser@chromium.org/313473002/60001

[tdresser, 2014-06-03 21:36:27]

The CQ bit was unchecked by tdresser@chromium.org

[tdresser, 2014-06-03 21:36:27]

The CQ bit was checked by tdresser@chromium.org

[commit-bot: I haz the power, 2014-06-03 21:36:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdresser@chromium.org/313473002/60001

[commit-bot: I haz the power, 2014-06-04 09:12:22]

Change committed as 274739