Handle ADDBOOKMARK intent with enhanced bookmarks UI.

Handle ADDBOOKMARK intent with enhanced bookmarks UI.

The ManageBookmarkActivity, which currently handles the ADDBOOKMARK
intent, will soon be deleted along with the rest of the old bookmarks
UI. However, we still need to handle the ADDBOOKMARK intent for
compatibility with old Android versions. This CL adds a new activity,
EnhancedBookmarkAddActivity, to handle the intent.

BUG=581961
Committed: https://crrev.com/26c1683c7f1a85ce0276e5dec97ca792f76bb8d4
Cr-Commit-Position: refs/heads/master@{#372263}

[newt (away), 2016-01-28 04:16:57]

newt@chromium.org changed reviewers:
+ ianwen@chromium.org, mpearson@chromium.org, yusufo@chromium.org

[newt (away), 2016-01-28 04:16:57]

ianwen: PTAL at everything
yusufo: PTAL at use of AsyncInitializationActivity
mpearson: PTAL at actions.xml

Thanks!

[newt (away), 2016-01-28 04:20:16]

https://codereview.chromium.org/1641983002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java
(right):

https://codereview.chromium.org/1641983002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java:33:
public void onStartWithNative() {
Yusuf: Is this the best method to override if I want to run code one time once
the native library is loaded? Currently, I need a little hack to make sure the
code below only runs once -- I check whether mModel is null.
"onCreateWithNative()" is really what I'd like to override, but it's final in
the superclass.

[Mark P, 2016-01-28 18:22:35]

actions.xml lgtm

For my information, will this intent cause an emit to
MobileIntent.PageLoadDueToExternalApp
?

--mark

[newt (away), 2016-01-28 18:23:39]

On 2016/01/28 18:22:35, Mark P wrote:
> actions.xml lgtm
> 
> For my information, will this intent cause an emit to
> MobileIntent.PageLoadDueToExternalApp
> ?

No. The intent just triggers a dialog to save a bookmark. It doesn't load the
page open the main Chrome activity.

Thanks :)

[Yusuf, 2016-01-28 19:47:43]

https://codereview.chromium.org/1641983002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java
(right):

https://codereview.chromium.org/1641983002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java:33:
public void onStartWithNative() {
On 2016/01/28 04:20:15, newt wrote:
> Yusuf: Is this the best method to override if I want to run code one time once
> the native library is loaded? Currently, I need a little hack to make sure the
> code below only runs once -- I check whether mModel is null.
> "onCreateWithNative()" is really what I'd like to override, but it's final in
> the superclass.

Once is once per activity, right?

onCreateWithNative calls handlePostNativeStartup in ChromeBrowserInitializer and
start the native dependent task queue. Anything there (initializeState,
initializeCompositor, finishNativeInitialization) is a good option for running
this only once. I would probably go with onFinishNativeInitialization

[newt (away), 2016-01-28 21:27:25]

https://codereview.chromium.org/1641983002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java
(right):

https://codereview.chromium.org/1641983002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java:33:
public void onStartWithNative() {
On 2016/01/28 19:47:43, Yusuf wrote:
> On 2016/01/28 04:20:15, newt wrote:
> > Yusuf: Is this the best method to override if I want to run code one time
once
> > the native library is loaded? Currently, I need a little hack to make sure
the
> > code below only runs once -- I check whether mModel is null.
> > "onCreateWithNative()" is really what I'd like to override, but it's final
in
> > the superclass.
> 
> Once is once per activity, right?
> 
> onCreateWithNative calls handlePostNativeStartup in ChromeBrowserInitializer
and
> start the native dependent task queue. Anything there (initializeState,
> initializeCompositor, finishNativeInitialization) is a good option for running
> this only once. I would probably go with onFinishNativeInitialization

finishNativeInitialization sgtm. Thanks!

[Ian Wen, 2016-01-28 22:29:35]

https://codereview.chromium.org/1641983002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java
(right):

https://codereview.chromium.org/1641983002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java:56:
});
Add onDestroy() and put mModel.destroy() there?

https://codereview.chromium.org/1641983002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUtils.java
(right):

https://codereview.chromium.org/1641983002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUtils.java:89:
public static BookmarkId addBookmark(Context context, EnhancedBookmarksModel
bookmarkModel,
Rename addBookmark to addBookmarkNoUI, or addBookmarkSilently or
addBookmarkToLastLocation()? This method is only used by EBAddActivity, but the
name is a bit generic... WDYT?

[newt (away), 2016-01-29 01:48:35]

https://codereview.chromium.org/1641983002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java
(right):

https://codereview.chromium.org/1641983002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkAddActivity.java:56:
});
On 2016/01/28 22:29:35, Ian Wen wrote:
> Add onDestroy() and put mModel.destroy() there?

Whoops. Good catch.

https://codereview.chromium.org/1641983002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUtils.java
(right):

https://codereview.chromium.org/1641983002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUtils.java:89:
public static BookmarkId addBookmark(Context context, EnhancedBookmarksModel
bookmarkModel,
On 2016/01/28 22:29:35, Ian Wen wrote:
> Rename addBookmark to addBookmarkNoUI, or addBookmarkSilently or
> addBookmarkToLastLocation()? This method is only used by EBAddActivity, but
the
> name is a bit generic... WDYT?

Changed to addBookmarkSilently().

[Ian Wen, 2016-01-29 02:04:30]

lgtm

[newt (away), 2016-01-29 02:05:41]

The CQ bit was checked by newt@chromium.org

[newt (away), 2016-01-29 02:05:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/1641983002/#ps40001
(title: "Ian's comments")

[commit-bot: I haz the power, 2016-01-29 02:06:54]

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

[commit-bot: I haz the power, 2016-01-29 02:46:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-29 02:47:02]

Description was changed from

==========
Handle ADDBOOKMARK intent with enhanced bookmarks UI.

The ManageBookmarkActivity, which currently handles the ADDBOOKMARK
intent, will soon be deleted along with the rest of the old bookmarks
UI. However, we still need to handle the ADDBOOKMARK intent for
compatibility with old Android versions. This CL adds a new activity,
EnhancedBookmarkAddActivity, to handle the intent.

BUG=581961
==========

to

==========
Handle ADDBOOKMARK intent with enhanced bookmarks UI.

The ManageBookmarkActivity, which currently handles the ADDBOOKMARK
intent, will soon be deleted along with the rest of the old bookmarks
UI. However, we still need to handle the ADDBOOKMARK intent for
compatibility with old Android versions. This CL adds a new activity,
EnhancedBookmarkAddActivity, to handle the intent.

BUG=581961

Committed: https://crrev.com/26c1683c7f1a85ce0276e5dec97ca792f76bb8d4
Cr-Commit-Position: refs/heads/master@{#372263}
==========

[commit-bot: I haz the power, 2016-01-29 02:47:03]

Patchset 3 (id:??) landed as
https://crrev.com/26c1683c7f1a85ce0276e5dec97ca792f76bb8d4
Cr-Commit-Position: refs/heads/master@{#372263}