Add an about flag to toggle the visibility of "All bookmarks"

Add an about flag to toggle the visibility of "All bookmarks"

There will be experiments about whether "All bookmarks" section should
be shown in bookmark manager. This CL makes it possible by adding an
about flag in Chrome.

BUG=605614
Committed: https://crrev.com/d37c3af126cb22a7831032b22a7c24bb2628c4bc
Cr-Commit-Position: refs/heads/master@{#392209}

[Ian Wen, 2016-05-05 21:39:50]

ianwen@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Ian Wen, 2016-05-05 21:39:51]

Sorry for the hassle, Dan. PTAL at my second try. :)

[gone, 2016-05-05 21:50:15]

https://codereview.chromium.org/1951273003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUIState.java
(right):

https://codereview.chromium.org/1951273003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUIState.java:79:
return createAllBookmarksState(bookmarkModel);
Why does this differ from the one above?  Should the state.mState =
STATE_ALL_BOOKMARKS above do this too?

https://codereview.chromium.org/1951273003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1951273003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1143: {"enable-all-bookmarks-view",
IDS_FLAGS_ENABLE_ALL_BOOKMARKS_VIEW_NAME,
Look up FeatureEntry and how that works instead of adding a flag like this.

https://codereview.chromium.org/1951273003/diff/1/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1951273003/diff/1/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:208: const char kDisableAllBookmarksView[] =
"disable-all-bookmarks-view";
You'll run afoul of the AboutFlagsHistogramTest because you're adding new
command line flags that aren't used on ChromeOS but they still test against it
anyway.

[Ian Wen, 2016-05-05 23:27:07]

ianwen@chromium.org changed reviewers:
+ rkaplow@chromium.org

[Ian Wen, 2016-05-05 23:27:08]

PTAL, thanks!

https://codereview.chromium.org/1951273003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUIState.java
(right):

https://codereview.chromium.org/1951273003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUIState.java:79:
return createAllBookmarksState(bookmarkModel);
On 2016/05/05 21:50:14, dfalcantara wrote:
> Why does this differ from the one above?  Should the state.mState =
> STATE_ALL_BOOKMARKS above do this too?

Will talk offline to explain this.

https://codereview.chromium.org/1951273003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1951273003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1143: {"enable-all-bookmarks-view",
IDS_FLAGS_ENABLE_ALL_BOOKMARKS_VIEW_NAME,
On 2016/05/05 21:50:14, dfalcantara wrote:
> Look up FeatureEntry and how that works instead of adding a flag like this.

Done.

https://codereview.chromium.org/1951273003/diff/1/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1951273003/diff/1/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:208: const char kDisableAllBookmarksView[] =
"disable-all-bookmarks-view";
On 2016/05/05 21:50:14, dfalcantara wrote:
> You'll run afoul of the AboutFlagsHistogramTest because you're adding new
> command line flags that aren't used on ChromeOS but they still test against it
> anyway.

Done.

[rkaplow, 2016-05-06 20:19:40]

can you also add the associated action to actions.xml see comment in
about_flags.cc

[rkaplow, 2016-05-06 20:50:32]

lgtm

[Ian Wen, 2016-05-06 21:54:03]

PTAL

[gone, 2016-05-06 22:02:00]

lgtm

[Ian Wen, 2016-05-06 22:02:13]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-05-06 22:02:13]

The patchset sent to the CQ was uploaded after l-g-t-m from rkaplow@chromium.org
Link to the patchset: https://codereview.chromium.org/1951273003/#ps40001
(title: "simplify uistate")

[commit-bot: I haz the power, 2016-05-06 22:02:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1951273003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1951273003/40001

[commit-bot: I haz the power, 2016-05-06 22:05:46]

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

[commit-bot: I haz the power, 2016-05-06 22:05:47]

Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  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_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[Ian Wen, 2016-05-06 22:23:57]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-05-06 22:23:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1951273003/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-06 22:24:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1951273003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1951273003/60001

[commit-bot: I haz the power, 2016-05-07 00:00:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-07 00:02:54]

Description was changed from

==========
Add an about flag to toggle the visibility of "All bookmarks"

There will be experiments about whether "All bookmarks" section should
be shown in bookmark manager. This CL makes it possible by adding an
about flag in Chrome.

BUG=605614
==========

to

==========
Add an about flag to toggle the visibility of "All bookmarks"

There will be experiments about whether "All bookmarks" section should
be shown in bookmark manager. This CL makes it possible by adding an
about flag in Chrome.

BUG=605614

Committed: https://crrev.com/d37c3af126cb22a7831032b22a7c24bb2628c4bc
Cr-Commit-Position: refs/heads/master@{#392209}
==========

[commit-bot: I haz the power, 2016-05-07 00:02:55]

Patchset 4 (id:??) landed as
https://crrev.com/d37c3af126cb22a7831032b22a7c24bb2628c4bc
Cr-Commit-Position: refs/heads/master@{#392209}