Add BookmarkUndoService to Android build

Add BookmarkUndoService to Android build

Adding build support and JNI hooks for undoing
bookmark actions.

BUG=415411
Committed: https://crrev.com/d817896458c8d1eaea1e09b3549126ab52658e3d
Cr-Commit-Position: refs/heads/master@{#296135}

[danduong, 2014-09-20 01:05:11]

danduong@chromium.org changed reviewers:
+ cjhopman@chromium.org, ianwen@chromium.org

[danduong, 2014-09-20 01:26:45]

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

[Ian Wen, 2014-09-21 01:25:38]

https://codereview.chromium.org/586913002/diff/60001/chrome/android/java/src/...
File chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java
(right):

https://codereview.chromium.org/586913002/diff/60001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java:463:
public void undoStartGrouping() {
This name is a bit confusing as it implies we are trying to undo a grouping
action. Maybe we should rename it as startUndoGrouping(), and same below.

[danduong, 2014-09-21 20:28:06]

On 2014/09/21 01:25:38, Ian Wen wrote:
>
https://codereview.chromium.org/586913002/diff/60001/chrome/android/java/src/...
> File chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java
> (right):
> 
>
https://codereview.chromium.org/586913002/diff/60001/chrome/android/java/src/...
> chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java:463:
> public void undoStartGrouping() {
> This name is a bit confusing as it implies we are trying to undo a grouping
> action. Maybe we should rename it as startUndoGrouping(), and same below.

Good point about the naming.

What do you think about startGroupingUndos() and endGroupingUndos() to be
completely unambiguous if "undo" is used as a verb, adjective, or noun.

[Ian Wen, 2014-09-22 17:20:58]

On 2014/09/21 20:28:06, danduong wrote:
> On 2014/09/21 01:25:38, Ian Wen wrote:
> >
>
https://codereview.chromium.org/586913002/diff/60001/chrome/android/java/src/...
> > File
chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/586913002/diff/60001/chrome/android/java/src/...
> >
chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java:463:
> > public void undoStartGrouping() {
> > This name is a bit confusing as it implies we are trying to undo a grouping
> > action. Maybe we should rename it as startUndoGrouping(), and same below.
> 
> Good point about the naming.
> 
> What do you think about startGroupingUndos() and endGroupingUndos() to be
> completely unambiguous if "undo" is used as a verb, adjective, or noun.

startUndoGrouping or startGroupingUndos both look fine to me.

[Ian Wen, 2014-09-22 17:22:13]

lgtm. Please wait for Chris' or Yaron's review.

[Kibeom Kim (inactive), 2014-09-22 17:35:07]

https://codereview.chromium.org/586913002/diff/60001/chrome/browser/android/b...
File chrome/browser/android/bookmarks/bookmarks_bridge.cc (right):

https://codereview.chromium.org/586913002/diff/60001/chrome/browser/android/b...
chrome/browser/android/bookmarks/bookmarks_bridge.cc:690:
undo_manager->StartGroupingActions();
Are we supposed to call the grouping API ourselves?

It looks like BookmarkUndoService class listens to
GroupedBookmarkChangesBeginning and GroupedBookmarkChangesEnded evenets and
calls undo_manager()->StartGroupingActions(); and
undo_manager()->EndGroupingActions(); there. Probably more correct way is
exposing ScopedGroupBookmarkActions class to Java instead?

[Kibeom Kim (inactive), 2014-09-22 17:37:10]

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

[danduong, 2014-09-22 17:42:00]

danduong@chromium.org changed reviewers:
+ yfriedman@chromium.org

[cjhopman, 2014-09-22 21:05:44]

lgtm, still need an OWNER for final approval

[danduong, 2014-09-22 22:00:01]

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

[Kibeom Kim (inactive), 2014-09-22 22:37:00]

https://codereview.chromium.org/586913002/diff/120001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java
(right):

https://codereview.chromium.org/586913002/diff/120001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BookmarksBridge.java:461: *
Start grouping actions for a single undo operation
Maybe add a comment that this doesn't work for partner bookmarks?

e.g., "This only works with BookmarkModel, not partner bookmarks."

[Kibeom Kim (inactive), 2014-09-22 22:37:07]

lgtm

[Kibeom Kim (inactive), 2014-09-22 22:37:08]

lgtm

[Ted C, 2014-09-22 22:49:11]

lgtm

[sky, 2014-09-22 23:25:21]

LGTM

https://codereview.chromium.org/586913002/diff/120001/chrome/browser/android/...
File chrome/browser/android/bookmarks/bookmarks_bridge.h (right):

https://codereview.chromium.org/586913002/diff/120001/chrome/browser/android/...
chrome/browser/android/bookmarks/bookmarks_bridge.h:21: };
nit: no ;

[danduong, 2014-09-22 23:37:24]

addressed nits.

[danduong, 2014-09-22 23:40:20]

The CQ bit was checked by danduong@chromium.org

[commit-bot: I haz the power, 2014-09-22 23:41:37]

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

[danduong, 2014-09-23 00:09:37]

The CQ bit was unchecked by danduong@chromium.org

[danduong, 2014-09-23 00:13:53]

The CQ bit was checked by danduong@chromium.org

[commit-bot: I haz the power, 2014-09-23 00:14:17]

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

[commit-bot: I haz the power, 2014-09-23 02:50:07]

Committed patchset #9 (id:160001) as edfcb7e6c347f7f0aabcd09ff846ca19f148867c

[commit-bot: I haz the power, 2014-09-23 02:50:40]

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