Makes the OfflinePageBridge.getAllPages method asynchronous.

chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java

Index: chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java
index 5a6b260011939de8f8464e4d8a6d22da70d3acf5..bfde06491a4570dcf28632e5e8f7652ff7cf0b75 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java
@@ -46,6 +46,9 @@ public class BookmarkUtils {
     /**
      * If the tab has already been bookmarked, start {@link BookmarkEditActivity} for the
      * bookmark. If not, add the bookmark to bookmarkmodel, and show a snackbar notifying the user.
+     *
+     * Note: Takes ownership of bookmarkModel, and will call |destroy| on it when finished.
+     *
      * @param idToAdd The bookmark ID if the tab has already been bookmarked.
      * @param bookmarkModel The bookmark model.
      * @param tab The tab to add or edit a bookmark.
@@ -61,6 +64,7 @@ public class BookmarkUtils {
         if (idToAdd != Tab.INVALID_BOOKMARK_ID) {
             startEditActivity(activity, new BookmarkId(idToAdd, BookmarkType.NORMAL),
                     webContentsToSave);
+            bookmarkModel.destroy();
             return;
         }
 
@@ -72,8 +76,9 @@ public class BookmarkUtils {
         // The bookmark model will be destroyed in the created AddBookmarkCallback.
         bookmarkModel.addBookmarkAsync(parent, bookmarkModel.getChildCount(parent), tab.getTitle(),
                 tab.getUrl(), webContentsToSave,
+                // bookmarkModel will be destroyed in the callback.
                 createAddBookmarkCallback(bookmarkModel, snackbarManager, activity,
-                        webContentsToSave));
+                                               webContentsToSave));
     }
 
     /**
@@ -224,21 +229,42 @@ public class BookmarkUtils {
     }
 
     /**
-     * Gets whether bookmark manager should load offline page initially.
+     * Shows bookmark main UI.
+     *
+     * The initial url the bookmark manager shows depends on offline page status and some
+     * experiments we run.
      */
-    private static boolean shouldShowOfflinePageAtFirst(OfflinePageBridge bridge) {
-        if (bridge == null || bridge.getAllPages().isEmpty() || OfflinePageUtils.isConnected()) {
-            return false;
+    public static void showBookmarkManager(final Activity activity) {
+        String lastUsedUrl = getLastUsedUrl(activity);
+        final String defaultUrl =
+                TextUtils.isEmpty(lastUsedUrl) ? UrlConstants.BOOKMARKS_URL : lastUsedUrl;
+
+        BookmarkModel bookmarkModel = new BookmarkModel();
+        OfflinePageBridge bridge = bookmarkModel.getOfflinePageBridge();

[Ian Wen, 2016/04/08 00:07:45]

I am a bit wary here. Is it possible that we retain the previous way as much as
possible? getFirstUrlToLoad() is a handy function as the logic differes for
phone/tablet and for offline pages.

[dewittj, 2016/04/08 18:06:58]

I reorganized the patch so that the code is laid out in a similar way as before,
wdyt?

+        bookmarkModel.destroy();
+
+        // If we are connected show the default URL.
+        if (bridge == null || OfflinePageUtils.isConnected()) {
+            openUrlOrStartActivityWithUrl(activity, defaultUrl);
+            return;
         }
-        return true;
-    }
 
-    /**
-     * Shows bookmark main UI.
-     */
-    public static void showBookmarkManager(Activity activity) {
-        String url = getFirstUrlToLoad(activity);
+        // Otherwise we want to show offline pages.  If there aren't any, just default to the
+        // regular list.
+        bridge.hasPages(new OfflinePageBridge.HasPagesCallback() {
+            @Override
+            public void onResult(boolean hasPages) {
+                String url = defaultUrl;
+                if (hasPages) {
+                    url = BookmarkUIState.createFilterUrl(BookmarkFilter.OFFLINE_PAGES, false)
+                                  .toString();
+                }
+                openUrlOrStartActivityWithUrl(activity, url);
+            }
+        });
+    }
 
+    private static void openUrlOrStartActivityWithUrl(Activity activity, String url) {
         if (DeviceFormFactor.isTablet(activity)) {
             openUrl(activity, url);
         } else {
@@ -249,26 +275,6 @@ public class BookmarkUtils {
     }
 
     /**
-     * The initial url the bookmark manager shows depends on offline page status and some
-     * experiments we run.
-     */
-    private static String getFirstUrlToLoad(Activity activity) {
-        BookmarkModel model = new BookmarkModel();
-        OfflinePageBridge bridge = model.getOfflinePageBridge();
-        try {
-            if (shouldShowOfflinePageAtFirst(bridge)) {
-                return BookmarkUIState.createFilterUrl(BookmarkFilter.OFFLINE_PAGES,
-                        false).toString();
-            }
-            String lastUsedUrl = getLastUsedUrl(activity);
-            if (!TextUtils.isEmpty(lastUsedUrl)) return lastUsedUrl;
-            return UrlConstants.BOOKMARKS_URL;
-        } finally {
-            model.destroy();
-        }
-    }
-
-    /**
      * Saves the last used url to preference. The saved url will be later queried by
      * {@link #getLastUsedUrl(Context)}
      */