Make the workspace cycler animation parameters editable via chrome://gesture

Make the workspace cycler animation parameters editable via chrome://gesture

Bug=174123
Test=Manual

Repro steps:
1) Run chrome with --ash-enable-workspace-scrubbing
2) Go to chrome://gesture
3) Modify the value of "Duration of the animations to start cycling"
4) Ensure that you have several maximized workspaces
5) Move fingers vertically on track pad. Notice difference in the duration of the animations to start cycling through workspaces
Repeat 3 - 5 as many times as you see fit

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

[pkotwicz, 2013-02-06 20:17:37]

Sadrul, can you please take a look?

[pkotwicz, 2013-02-08 19:48:02]

Ping!

[sadrul, 2013-02-11 16:41:19]

Looks mostly good. Some comments:

https://codereview.chromium.org/12212040/diff/1/ash/ash.gyp
File ash/ash.gyp (right):

https://codereview.chromium.org/12212040/diff/1/ash/ash.gyp#newcode434
ash/ash.gyp:434: 'wm/workspace/workspace_cycler_configuration.h',
sort

https://codereview.chromium.org/12212040/diff/1/ash/wm/workspace/workspace_cy...
File ash/wm/workspace/workspace_cycler_configuration.h (right):

https://codereview.chromium.org/12212040/diff/1/ash/wm/workspace/workspace_cy...
ash/wm/workspace/workspace_cycler_configuration.h:75: const base::ListValue*
list_value);
Use constref instead.

https://codereview.chromium.org/12212040/diff/1/ash/wm/workspace/workspace_cy...
ash/wm/workspace/workspace_cycler_configuration.h:81: static const
base::ListValue* GetListValue(Property property);
ditto

https://codereview.chromium.org/12212040/diff/1/chrome/browser/resources/gest...
File chrome/browser/resources/gesture_config.js (right):

https://codereview.chromium.org/12212040/diff/1/chrome/browser/resources/gest...
chrome/browser/resources/gesture_config.js:403: 
This isn't necessary. Just add the config here. This isn't meant for end user,
so it can be a bit crude. (We do not do this for over scroll configs either)

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

https://codereview.chromium.org/12212040/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:37:
CR_DEFINE_STATIC_LOCAL(std::vector<OverscrollPref>, overscroll_prefs, ());
Whats the reason for this change?

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

https://codereview.chromium.org/12212040/diff/1/chrome/common/pref_names.h#ne...
chrome/common/pref_names.h:785: extern const char
kWorkspaceCyclerDeeperThanSelectedYOffsets[];
#if defined ash?

[pkotwicz, 2013-02-12 00:00:25]

Sadrul, can you please take another look?

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

https://codereview.chromium.org/12212040/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:37:
CR_DEFINE_STATIC_LOCAL(std::vector<OverscrollPref>, overscroll_prefs, ());
The reason for this change is consistency with how I implemented the
WorkspaceCyclerPrefs.
In particular, I did not want to have WorkspaceCyclerConfiguration::NONE.
Other than making me happy, there is no reason for this change.

[sadrul, 2013-02-12 15:59:58]

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

https://codereview.chromium.org/12212040/diff/1/chrome/browser/ui/gesture_pre...
chrome/browser/ui/gesture_prefs_observer_factory_aura.cc:37:
CR_DEFINE_STATIC_LOCAL(std::vector<OverscrollPref>, overscroll_prefs, ());
On 2013/02/12 00:00:26, pkotwicz wrote:
> The reason for this change is consistency with how I implemented the
> WorkspaceCyclerPrefs.
> In particular, I did not want to have WorkspaceCyclerConfiguration::NONE.

Why would you need ::NONE?

> Other than making me happy, there is no reason for this change.

Unless the change adds value (better follow style guide, performance
improvement, more readable etc.), it's better to not add code churn.

https://codereview.chromium.org/12212040/diff/17/ash/wm/workspace/workspace_c...
File ash/wm/workspace/workspace_cycler_configuration.cc (right):

https://codereview.chromium.org/12212040/diff/17/ash/wm/workspace/workspace_c...
ash/wm/workspace/workspace_cycler_configuration.cc:156: return 0;
NOTREACHED here too?

[sadrul, 2013-02-12 16:11:25]

> > In particular, I did not want to have WorkspaceCyclerConfiguration::NONE.
> 
> Why would you need ::NONE?

I see what you mean. This is fine. LGTM.

> > Other than making me happy, there is no reason for this change.
> 
> Unless the change adds value (better follow style guide, performance
> improvement, more readable etc.), it's better to not add code churn.
> 
>
https://codereview.chromium.org/12212040/diff/17/ash/wm/workspace/workspace_c...
> File ash/wm/workspace/workspace_cycler_configuration.cc (right):
> 
>
https://codereview.chromium.org/12212040/diff/17/ash/wm/workspace/workspace_c...
> ash/wm/workspace/workspace_cycler_configuration.cc:156: return 0;
> NOTREACHED here too?

[pkotwicz, 2013-02-12 16:52:28]

Scott, can you please take a look?

[sky, 2013-02-12 18:30:56]

As long as this is temporary, LGTM

https://codereview.chromium.org/12212040/diff/12002/ash/wm/workspace/workspac...
File ash/wm/workspace/workspace_cycler_configuration.h (right):

https://codereview.chromium.org/12212040/diff/12002/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_cycler_configuration.h:67: // Returns true if the
cycler is enabled.
Classes like this are a pain for testing. Primarily because there is no 'reset
to default state'. I assume this is all temporary, right?

https://codereview.chromium.org/12212040/diff/12002/ash/wm/workspace/workspac...
ash/wm/workspace/workspace_cycler_configuration.h:85: };
private: DISALLOW_IMPLICIT_CONSTRUCTORS(...)

[commit-bot: I haz the power, 2013-02-12 23:44:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/12212040/13001

[commit-bot: I haz the power, 2013-02-13 00:15:58]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-02-13 01:57:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/12212040/9006

[commit-bot: I haz the power, 2013-02-13 04:00:03]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-02-13 14:28:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/12212040/16001

[commit-bot: I haz the power, 2013-02-13 16:39:31]

Change committed as 182243