[Bookmarks] Remove side drawer and introduce root folder view

[Bookmarks] Remove side drawer and introduce root folder view

The bookmarks side drawer is being removed to support Chrome Home.
A new "Bookmarks" root view containing the "Mobile bookmarks",
"Bookmarks bar", and "Other bookmarks" top-level folders has been
introduced to allow navigation between top-level folders.

Also updates the tablet style to match the phone style by removing
divider lines and adding background 9-patches to the folder rows.

BUG=682808
Review-Url: https://codereview.chromium.org/2837643002
Cr-Commit-Position: refs/heads/master@{#467095}
Committed: https://chromium.googlesource.com/chromium/src/+/7eb3a44514aa1db6ab8fad4bc74d822ca2b87ae4

[Theresa, 2017-04-21 17:13:36]

The CQ bit was checked by twellington@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 17:14:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Theresa, 2017-04-21 17:14:51]

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

[Theresa, 2017-04-21 17:14:52]

ptal - screenshots are in the bug

[commit-bot: I haz the power, 2017-04-21 17:25:08]

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

[commit-bot: I haz the power, 2017-04-21 17:25:09]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Theresa, 2017-04-21 17:28:42]

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

https://codereview.chromium.org/2837643002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkRow.java:81:
if (isSelectable()) mMoreIcon.setClickable(
Presubmit is complaining about this line requiring curly braces.

[gone, 2017-04-21 17:29:50]

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

https://codereview.chromium.org/2837643002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkRow.java:81:
if (isSelectable()) mMoreIcon.setClickable(
On 2017/04/21 17:28:42, Theresa wrote:
> Presubmit is complaining about this line requiring curly braces.

Yeah... it needs them.

[gone, 2017-04-22 00:32:17]

lgtm

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

https://codereview.chromium.org/2837643002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkItemsAdapter.java:51:
private List<BookmarkId> mTopLevelFolders = new ArrayList<>();
should these all be finals?

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

https://codereview.chromium.org/2837643002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkManager.java:442:

@VisibleForTesting?

https://codereview.chromium.org/2837643002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkTest.java
(right):

https://codereview.chromium.org/2837643002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkTest.java:190:
// Open the "Mobile bookmarks" folder
nit: add a period

[Theresa, 2017-04-24 18:25:00]

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

https://codereview.chromium.org/2837643002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkItemsAdapter.java:51:
private List<BookmarkId> mTopLevelFolders = new ArrayList<>();
On 2017/04/22 00:32:15, slow (dfalcantara) wrote:
> should these all be finals?

Done.

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

https://codereview.chromium.org/2837643002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkManager.java:442:

On 2017/04/22 00:32:15, slow (dfalcantara) wrote:
> @VisibleForTesting?

Done.

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

https://codereview.chromium.org/2837643002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkRow.java:81:
if (isSelectable()) mMoreIcon.setClickable(
On 2017/04/21 17:29:50, slow (dfalcantara) wrote:
> On 2017/04/21 17:28:42, Theresa wrote:
> > Presubmit is complaining about this line requiring curly braces.
> 
> Yeah... it needs them.

Done. I removed isSelectable() because it's always true now.

https://codereview.chromium.org/2837643002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkTest.java
(right):

https://codereview.chromium.org/2837643002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkTest.java:190:
// Open the "Mobile bookmarks" folder
On 2017/04/22 00:32:15, slow (dfalcantara) wrote:
> nit: add a period

Done.

[Theresa, 2017-04-24 18:25:36]

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

[Theresa, 2017-04-24 18:25:37]

+sky@ ptal at components/bookmarks/managed/managed_bookmark_service.cc

[Theresa, 2017-04-24 18:26:18]

The CQ bit was checked by twellington@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-24 18:27:07]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-24 19:09:21]

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

[commit-bot: I haz the power, 2017-04-24 19:09:22]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sky, 2017-04-24 19:22:25]

https://codereview.chromium.org/2837643002/diff/20001/components/bookmarks/ma...
File components/bookmarks/managed/managed_bookmark_service.cc (right):

https://codereview.chromium.org/2837643002/diff/20001/components/bookmarks/ma...
components/bookmarks/managed/managed_bookmark_service.cc:151:
!IsDescendantOf(node, supervised_node_) && !node->is_root();
How about using !is_permanent_node? (on the model)

[Theresa, 2017-04-24 20:19:05]

The CQ bit was checked by twellington@chromium.org to run a CQ dry run

[Theresa, 2017-04-24 20:19:10]

https://codereview.chromium.org/2837643002/diff/20001/components/bookmarks/ma...
File components/bookmarks/managed/managed_bookmark_service.cc (right):

https://codereview.chromium.org/2837643002/diff/20001/components/bookmarks/ma...
components/bookmarks/managed/managed_bookmark_service.cc:151:
!IsDescendantOf(node, supervised_node_) && !node->is_root();
On 2017/04/24 19:22:25, sky wrote:
> How about using !is_permanent_node? (on the model)

Done.

[commit-bot: I haz the power, 2017-04-24 20:20:14]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sky, 2017-04-24 20:34:10]

LGTM

[commit-bot: I haz the power, 2017-04-24 20:55:45]

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

[commit-bot: I haz the power, 2017-04-24 20:55:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Theresa, 2017-04-24 23:43:03]

The CQ bit was checked by twellington@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-24 23:43:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-25 00:48:27]

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

[commit-bot: I haz the power, 2017-04-25 00:48:28]

Dry run: This issue passed the CQ dry run.

[Theresa, 2017-04-25 15:29:45]

dfalcantara@ - I had to change something in bookmark_bridge.cc; will you please
take another quick look?

https://codereview.chromium.org/2837643002/diff/40001/components/bookmarks/ma...
File components/bookmarks/managed/managed_bookmark_service.cc (right):

https://codereview.chromium.org/2837643002/diff/40001/components/bookmarks/ma...
components/bookmarks/managed/managed_bookmark_service.cc:152:
!bookmark_model_->is_permanent_node(node);
sky@ - I moved this check to the Android specific bookmarks_bridge.cc method
that checks if a bookmark is editable. Numerous places in our code rely on
ManagedBookmarkService::CanBeEditedByUser returning true for the root node, so
changing this method caused the following tests to fail:

BookmarkContextMenuControllerTest.BookmarkBar
BookmarkContextMenuControllerTest.EmptyNodes
BookmarkContextMenuControllerTest.SingleURL
BookmarkContextMenuTest.SingleURL
BookmarkContextMenuTest.EmptyNodes
BookmarkContextMenuControllerTest.SingleFolder
BookmarkContextMenuTest.MultipleURLs
BookmarkEditorViewTest.EditKeepsScheme
BookmarkEditorViewTest.ModelsMatch
BookmarkEditorViewTest.EditTitleKeepsPosition
BookmarkContextMenuTest.SingleFolder
BookmarkEditorViewTest.NewFolder
ProfileWriterTest.CheckBookmarksWithMultiProfile
BookmarkContextMenuControllerTest.MultipleURLs
BookmarkEditorViewTest.MoveFolder
BookmarkEditorViewTest.EditURLKeepsPosition
BookmarkBubbleViewTest.SyncPromoSignedIn
BookmarkContextMenuTest.MultipleEmptyFolders
BookmarkEditorViewTest.NewURL
BookmarkBubbleViewTest.SyncPromoNotSignedIn
ManagedBookmarkServiceTest.RemoveAllDoesntRemoveManaged
BookmarkEditorViewTest.ChangeParent
BookmarkContextMenuControllerTest.MultipleEmptyFolders
BrowserCommandsTest.BookmarkCurrentPage
BookmarkEditorViewTest.ChangeParentAndURL
BookmarkEditorViewTest.MoveToNewParent
BookmarkContextMenuTest.MultipleFoldersWithURLs
BookmarkEditorViewTest.NewFolderTitleUpdatedOnCommit
BookmarkContextMenuControllerTest.MultipleFoldersWithURLs

[gone, 2017-04-25 20:18:01]

lgtm

[Theresa, 2017-04-25 20:19:15]

The CQ bit was checked by twellington@chromium.org

[Theresa, 2017-04-25 20:19:15]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2837643002/#ps60001
(title: "Move is_permanent_node_ check to bookmark_bridge.cc")

[commit-bot: I haz the power, 2017-04-25 20:19:51]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-25 20:28:01]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493151555069780,
"parent_rev": "b60799e04934ed8a06d47beafffd1d28b995915f", "commit_rev":
"7eb3a44514aa1db6ab8fad4bc74d822ca2b87ae4"}

[commit-bot: I haz the power, 2017-04-25 20:28:37]

Description was changed from

==========
[Bookmarks] Remove side drawer and introduce root folder view

The bookmarks side drawer is being removed to support Chrome Home.
A new "Bookmarks" root view containing the "Mobile bookmarks",
"Bookmarks bar", and "Other bookmarks" top-level folders has been
introduced to allow navigation between top-level folders.

Also updates the tablet style to match the phone style by removing
divider lines and adding background 9-patches to the folder rows.

BUG=682808
==========

to

==========
[Bookmarks] Remove side drawer and introduce root folder view

The bookmarks side drawer is being removed to support Chrome Home.
A new "Bookmarks" root view containing the "Mobile bookmarks",
"Bookmarks bar", and "Other bookmarks" top-level folders has been
introduced to allow navigation between top-level folders.

Also updates the tablet style to match the phone style by removing
divider lines and adding background 9-patches to the folder rows.

BUG=682808

Review-Url: https://codereview.chromium.org/2837643002
Cr-Commit-Position: refs/heads/master@{#467095}
Committed:
https://chromium.googlesource.com/chromium/src/+/7eb3a44514aa1db6ab8fad4bc74d...
==========

[commit-bot: I haz the power, 2017-04-25 20:28:38]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/7eb3a44514aa1db6ab8fad4bc74d...