[Android] Add enhanced bookmarks grid/list view experiment.

[Android] Add enhanced bookmarks grid/list view experiment

BUG=490717
Committed: https://crrev.com/5bd609076d410cbaf47ec94ec61414af26134f46
Cr-Commit-Position: refs/heads/master@{#332957}

[Kibeom Kim (inactive), 2015-05-27 19:43:01]

kkimlabs@chromium.org changed reviewers:
+ danduong@chromium.org, ianwen@chromium.org, twellington@chromium.org

[Kibeom Kim (inactive), 2015-05-27 19:43:01]

ptal

[Kibeom Kim (inactive), 2015-05-27 19:50:28]

(it's ready for review)

[Theresa, 2015-05-28 20:13:08]

https://codereview.chromium.org/1143153010/diff/60001/components/enhanced_boo...
File components/enhanced_bookmarks/enhanced_bookmark_utils.cc (right):

https://codereview.chromium.org/1143153010/diff/60001/components/enhanced_boo...
components/enhanced_bookmarks/enhanced_bookmark_utils.cc:76: // if
(default_view_mode == "Grid") return ViewMode::GRID;
If we change the default to list (or something else) later, this line will be
needed again? The default controlled by finch, yes?

https://codereview.chromium.org/1143153010/diff/60001/components/enhanced_boo...
File components/enhanced_bookmarks/enhanced_bookmark_utils.h (right):

https://codereview.chromium.org/1143153010/diff/60001/components/enhanced_boo...
components/enhanced_bookmarks/enhanced_bookmark_utils.h:70: // It doesn't return
ViewMode::DEFAULT.
Can you elaborate with what is returned; e.g. the default is controlled by the
value in the finch experiment.

[Kibeom Kim (inactive), 2015-05-29 00:11:05]

https://codereview.chromium.org/1143153010/diff/60001/components/enhanced_boo...
File components/enhanced_bookmarks/enhanced_bookmark_utils.cc (right):

https://codereview.chromium.org/1143153010/diff/60001/components/enhanced_boo...
components/enhanced_bookmarks/enhanced_bookmark_utils.cc:76: // if
(default_view_mode == "Grid") return ViewMode::GRID;
On 2015/05/28 20:13:08, Theresa Wellington wrote:
> If we change the default to list (or something else) later, this line will be
> needed again? The default controlled by finch, yes?

Sorry. The comment and the structure was confusing.. clarified it. Done.

https://codereview.chromium.org/1143153010/diff/60001/components/enhanced_boo...
File components/enhanced_bookmarks/enhanced_bookmark_utils.h (right):

https://codereview.chromium.org/1143153010/diff/60001/components/enhanced_boo...
components/enhanced_bookmarks/enhanced_bookmark_utils.h:70: // It doesn't return
ViewMode::DEFAULT.
On 2015/05/28 20:13:08, Theresa Wellington wrote:
> Can you elaborate with what is returned; e.g. the default is controlled by the
> value in the finch experiment.

Done.

[Ian Wen, 2015-05-30 00:14:01]

lgtm.

[Theresa, 2015-06-01 15:05:59]

lgtm w/ 1 nit

https://codereview.chromium.org/1143153010/diff/80001/components/enhanced_boo...
File components/enhanced_bookmarks/enhanced_bookmark_utils.h (right):

https://codereview.chromium.org/1143153010/diff/80001/components/enhanced_boo...
components/enhanced_bookmarks/enhanced_bookmark_utils.h:70: // The default is
controlled by finch experiment. If finch is not available or
nit: "by a finch experiment"

[Kibeom Kim (inactive), 2015-06-01 18:21:00]

wittman@ ptal chrome/browser/bookmarks/enhanced_bookmarks_features.cc

https://codereview.chromium.org/1143153010/diff/80001/components/enhanced_boo...
File components/enhanced_bookmarks/enhanced_bookmark_utils.h (right):

https://codereview.chromium.org/1143153010/diff/80001/components/enhanced_boo...
components/enhanced_bookmarks/enhanced_bookmark_utils.h:70: // The default is
controlled by finch experiment. If finch is not available or
On 2015/06/01 15:05:59, Theresa Wellington wrote:
> nit: "by a finch experiment"

Done.

[Kibeom Kim (inactive), 2015-06-01 18:40:51]

kkimlabs@chromium.org changed reviewers:
+ wittman@chromium.org

[Kibeom Kim (inactive), 2015-06-01 18:40:52]

+ wittman@

[Mike Wittman, 2015-06-03 00:36:45]

chrome/browser/bookmarks/enhanced_bookmarks_features.cc lgtm

[Kibeom Kim (inactive), 2015-06-04 17:44:44]

The CQ bit was checked by kkimlabs@chromium.org

[Kibeom Kim (inactive), 2015-06-04 17:44:44]

The patchset sent to the CQ was uploaded after l-g-t-m from ianwen@chromium.org,
twellington@chromium.org
Link to the patchset: https://codereview.chromium.org/1143153010/#ps100001
(title: "a finch experiment..")

[commit-bot: I haz the power, 2015-06-04 17:46:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1143153010/100001

[commit-bot: I haz the power, 2015-06-04 17:51:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-04 17:51:55]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Kibeom Kim (inactive), 2015-06-04 18:02:28]

kkimlabs@chromium.org changed reviewers:
+ asvitkine@chromium.org, tedchoc@chromium.org

[Kibeom Kim (inactive), 2015-06-04 18:02:29]

ptal:
asvitkine@ +components/variations dependency
tedchoc@ EnhancedBookmarkManager.java

[Alexei Svitkine (slow), 2015-06-04 18:08:46]

lgtm, but don't use the word "Finch" in the CL description, since it's an
internal codename. You can use "variation param", or "experiment param" instead.

https://codereview.chromium.org/1143153010/diff/120001/components/enhanced_bo...
File components/enhanced_bookmarks/enhanced_bookmark_utils.cc (right):

https://codereview.chromium.org/1143153010/diff/120001/components/enhanced_bo...
components/enhanced_bookmarks/enhanced_bookmark_utils.cc:73: if
(default_view_mode == "List") return ViewMode::LIST;
Nit: C++ style says to break line. Same below.

[Kibeom Kim (inactive), 2015-06-04 18:17:47]

https://codereview.chromium.org/1143153010/diff/120001/components/enhanced_bo...
File components/enhanced_bookmarks/enhanced_bookmark_utils.cc (right):

https://codereview.chromium.org/1143153010/diff/120001/components/enhanced_bo...
components/enhanced_bookmarks/enhanced_bookmark_utils.cc:73: if
(default_view_mode == "List") return ViewMode::LIST;
On 2015/06/04 18:08:45, Alexei Svitkine wrote:
> Nit: C++ style says to break line. Same below.

Done.

[Ted C, 2015-06-04 18:36:57]

On 2015/06/04 18:17:47, Kibeom Kim wrote:
>
https://codereview.chromium.org/1143153010/diff/120001/components/enhanced_bo...
> File components/enhanced_bookmarks/enhanced_bookmark_utils.cc (right):
> 
>
https://codereview.chromium.org/1143153010/diff/120001/components/enhanced_bo...
> components/enhanced_bookmarks/enhanced_bookmark_utils.cc:73: if
> (default_view_mode == "List") return ViewMode::LIST;
> On 2015/06/04 18:08:45, Alexei Svitkine wrote:
> > Nit: C++ style says to break line. Same below.
> 
> Done.

lgtm

[Kibeom Kim (inactive), 2015-06-04 18:37:43]

The CQ bit was checked by kkimlabs@chromium.org

[Kibeom Kim (inactive), 2015-06-04 18:37:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
wittman@chromium.org, ianwen@chromium.org, twellington@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1143153010/#ps140001
(title: "if style fix")

[commit-bot: I haz the power, 2015-06-04 18:40:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1143153010/140001

[commit-bot: I haz the power, 2015-06-04 18:52:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-04 18:52:12]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Kibeom Kim (inactive), 2015-06-04 19:21:37]

kkimlabs@chromium.org changed reviewers:
+ sky@chromium.org

[Kibeom Kim (inactive), 2015-06-04 19:21:38]

sky@ ptal chrome/chrome.gyp (one line simple renaming)

[sky, 2015-06-04 20:50:57]

LGTM

[Kibeom Kim (inactive), 2015-06-04 21:20:03]

The CQ bit was checked by kkimlabs@chromium.org

[Kibeom Kim (inactive), 2015-06-04 21:20:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
wittman@chromium.org, twellington@chromium.org, tedchoc@chromium.org,
asvitkine@chromium.org, ianwen@chromium.org
Link to the patchset: https://codereview.chromium.org/1143153010/#ps160001
(title: "gn fix")

[commit-bot: I haz the power, 2015-06-04 21:21:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1143153010/160001

[commit-bot: I haz the power, 2015-06-04 23:21:26]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-06-04 23:22:09]

Patchset 9 (id:??) landed as
https://crrev.com/5bd609076d410cbaf47ec94ec61414af26134f46
Cr-Commit-Position: refs/heads/master@{#332957}