Makes the OfflinePageBridge.getAllPages method asynchronous.

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.bookmarks;
 
 import android.app.Activity;
 import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
@@ skipping 28 matching lines @@
 /**
  * A class holding static util functions for bookmark.
  */
 public class BookmarkUtils {
     private static final String PREF_LAST_USED_URL = "enhanced_bookmark_last_used_url";
     private static final String PREF_LAST_USED_PARENT = "enhanced_bookmark_last_used_parent_folder";
 
     /**
      * 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.
      * @param snackbarManager The SnackbarManager used to show the snackbar.
      * @param activity Current activity.
      */
     public static void addOrEditBookmark(long idToAdd, BookmarkModel bookmarkModel,
             Tab tab, SnackbarManager snackbarManager, Activity activity) {
         // See if the Tab's contents should be saved or not.
         WebContents webContentsToSave = null;
         if (!shouldSkipSavingTabOffline(tab)) webContentsToSave = tab.getWebContents();
 
         if (idToAdd != Tab.INVALID_BOOKMARK_ID) {
             startEditActivity(activity, new BookmarkId(idToAdd, BookmarkType.NORMAL),
                     webContentsToSave);
+            bookmarkModel.destroy();
             return;
         }
 
         BookmarkId parent = getLastUsedParent(activity);
         if (parent == null || !bookmarkModel.doesBookmarkExist(parent)) {
             parent = bookmarkModel.getDefaultFolder();
         }
 
         // 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));
     }
 
     /**
      * Adds a bookmark with the given title and url to the last used parent folder. Provides
      * no visual feedback that a bookmark has been added.
      *
      * @param title The title of the bookmark.
      * @param url The URL of the new bookmark.
      */
     public static BookmarkId addBookmarkSilently(Context context,
@@ skipping 130 matching lines @@
 
             @Override
             public void onAction(Object actionData) {
                 RecordUserAction.record("EnhancedBookmarks.EditAfterCreateButtonClicked");
                 startEditActivity(activity, bookmarkId, (WebContents) actionData);
             }
         };
     }
 
     /**
-     * Gets whether bookmark manager should load offline page initially.
-     */
-    private static boolean shouldShowOfflinePageAtFirst(OfflinePageBridge bridge) {
-        if (bridge == null || bridge.getAllPages().isEmpty() || OfflinePageUtils.isConnected()) {
-            return false;
-        }
-        return true;
-    }
-
-    /**
      * Shows bookmark main UI.
+     *
+     * The initial url the bookmark manager shows depends on offline page status and some
+     * experiments we run.
      */
-    public static void showBookmarkManager(Activity activity) {
-        String url = getFirstUrlToLoad(activity);
+    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;
+        }
+
+        // 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 {
             Intent intent = new Intent(activity, BookmarkActivity.class);
             intent.setData(Uri.parse(url));
             activity.startActivity(intent);
         }
     }
 
     /**
-     * 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)}
      */
     static void setLastUsedUrl(Context context, String url) {
         PreferenceManager.getDefaultSharedPreferences(context).edit()
                 .putString(PREF_LAST_USED_URL, url).apply();
     }
 
     /**
      * Fetches url representing the user's state last time they close the bookmark manager.
@@ skipping 105 matching lines @@
     }
 
     /**
      * Indicates whether we should skip saving the given tab as an offline page.
      * A tab shouldn't be saved offline if it shows an error page or a sad tab page.
      */
     private static boolean shouldSkipSavingTabOffline(Tab tab) {
         return tab.isShowingErrorPage() || tab.isShowingSadTab();
     }
 }