[Offline pages] Removing offline pages from Bookmarks UI

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 e5d0c6e4179d69d97b281a3de08642a25ffacb28..c10c4d7a8ca6c491d37d9de6b3d0f6cb3463518e 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
@@ -9,7 +9,6 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.net.Uri;
-import android.os.AsyncTask;
 import android.preference.PreferenceManager;
 import android.provider.Browser;
 import android.text.TextUtils;
@@ -24,8 +23,6 @@ import org.chromium.chrome.browser.UrlConstants;
 import org.chromium.chrome.browser.bookmarks.BookmarkModel.AddBookmarkCallback;
 import org.chromium.chrome.browser.document.ChromeLauncherActivity;
 import org.chromium.chrome.browser.ntp.NewTabPageUma;
-import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
-import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
 import org.chromium.chrome.browser.snackbar.Snackbar;
 import org.chromium.chrome.browser.snackbar.SnackbarManager;
 import org.chromium.chrome.browser.snackbar.SnackbarManager.SnackbarController;
@@ -57,13 +54,8 @@ public class BookmarkUtils {
      */
     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);
+            startEditActivity(activity, new BookmarkId(idToAdd, BookmarkType.NORMAL));
             bookmarkModel.destroy();
             return;
         }
@@ -73,11 +65,14 @@ public class BookmarkUtils {
             parent = bookmarkModel.getDefaultFolder();
         }
 
+        // See if the Tab's contents should be saved or not.
+        WebContents webContentsToSave = null;
+        if (!shouldSkipSavingTabOffline(tab)) webContentsToSave = tab.getWebContents();
+
         // The bookmark model will be destroyed in the created AddBookmarkCallback.
         bookmarkModel.addBookmarkAsync(parent, bookmarkModel.getChildCount(parent), tab.getTitle(),

[Ian Wen, 2016/04/18 18:33:14]

Since now bookmark and offline pages are decoupled, I suppose the cleanest
separation should be:
1. Remove addBookmarkAsync() and saveOfflinePage() in bookmark model.
2. Add one line in ChromeActivity#addOrEditBookmark() method to save the offline
page.

[fgorski, 2016/04/18 23:39:05]

Done.

                 tab.getUrl(), webContentsToSave,
-                createAddBookmarkCallback(bookmarkModel, snackbarManager, activity,
-                                               webContentsToSave));
+                createAddBookmarkCallback(bookmarkModel, snackbarManager, activity));
     }
 
     /**
@@ -98,109 +93,34 @@ public class BookmarkUtils {
     }
 
     /**
-     * Shows a snackbar after a bookmark has been added.
-     *
-     * NOTE: This method calls BookmarkModel#destroy() on the BookmarkModel that is passed to it.
-     */
-    private static void showSnackbarForAddingBookmark(final BookmarkModel bookmarkModel,
-            final SnackbarManager snackbarManager, final Activity activity,
-            final BookmarkId bookmarkId, final int saveResult, boolean isStorageAlmostFull,
-            final WebContents webContents) {
-        Snackbar snackbar;
-        OfflinePageBridge offlinePageBridge = bookmarkModel.getOfflinePageBridge();
-        if (offlinePageBridge == null) {
-            String folderName = bookmarkModel
-                    .getBookmarkTitle(bookmarkModel.getBookmarkById(bookmarkId).getParentId());
-            SnackbarController snackbarController =
-                    createSnackbarControllerForEditButton(activity, bookmarkId);
-            if (getLastUsedParent(activity) == null) {
-                snackbar = Snackbar.make(activity.getString(R.string.bookmark_page_saved),
-                        snackbarController, Snackbar.TYPE_ACTION);
-            } else {
-                snackbar = Snackbar.make(folderName, snackbarController, Snackbar.TYPE_ACTION)
-                        .setTemplateText(activity.getString(R.string.bookmark_page_saved_folder));
-            }
-            snackbar = snackbar.setSingleLine(false)
-                    .setAction(activity.getString(R.string.bookmark_item_edit), webContents);
-        } else {
-            SnackbarController snackbarController = null;
-            int messageId;
-            String suffix = null;
-            int buttonId = R.string.bookmark_item_edit;
-
-            if (saveResult == AddBookmarkCallback.SKIPPED) {
-                messageId = OfflinePageUtils.getStringId(
-                        R.string.offline_pages_as_bookmarks_page_skipped);
-            } else if (isStorageAlmostFull) {
-                messageId = OfflinePageUtils.getStringId(saveResult == AddBookmarkCallback.SAVED
-                    ? R.string.offline_pages_as_bookmarks_page_saved_storage_near_full
-                    : R.string.offline_pages_as_bookmarks_page_failed_to_save_storage_near_full);
-                // Show "Free up space" button.
-                buttonId = OfflinePageUtils.getStringId(R.string.offline_pages_free_up_space_title);
-                snackbarController = OfflinePageUtils.createSnackbarControllerForFreeUpSpaceButton(
-                        offlinePageBridge, snackbarManager, activity);
-            } else {
-                if (saveResult == AddBookmarkCallback.SAVED) {
-                    if (getLastUsedParent(activity) == null) {
-                        messageId = OfflinePageUtils.getStringId(
-                                R.string.offline_pages_as_bookmarks_page_saved);
-                    } else {
-                        messageId = OfflinePageUtils.getStringId(
-                                R.string.offline_pages_as_bookmarks_page_saved_folder);
-                        suffix = bookmarkModel.getBookmarkTitle(
-                                bookmarkModel.getBookmarkById(bookmarkId).getParentId());
-                    }
-                } else {
-                    messageId = OfflinePageUtils.getStringId(
-                            R.string.offline_pages_as_bookmarks_page_failed_to_save);
-                }
-            }
-            if (snackbarController == null) {
-                snackbarController = createSnackbarControllerForEditButton(activity, bookmarkId);
-            }
-            snackbar = Snackbar
-                    .make(activity.getString(messageId, suffix), snackbarController,
-                            Snackbar.TYPE_ACTION)
-                    .setAction(activity.getString(buttonId), webContents).setSingleLine(false);
-        }
-
-        snackbarManager.showSnackbar(snackbar);
-        bookmarkModel.destroy();
-    }
-
-    /**
      * Shows a snackbar if necessary after adding a bookmark.
      *
      * NOTE: This callback will call BookmarkModel#destroy() on the passed-in bookmark model.
      */
-    private static AddBookmarkCallback createAddBookmarkCallback(
-            final BookmarkModel bookmarkModel, final SnackbarManager snackbarManager,
-            final Activity activity, final WebContents webContents) {
+    private static AddBookmarkCallback createAddBookmarkCallback(final BookmarkModel bookmarkModel,
+            final SnackbarManager snackbarManager, final Activity activity) {
         return new AddBookmarkCallback() {
             @Override
             public void onBookmarkAdded(final BookmarkId bookmarkId, final int saveResult) {
-                // Shows the snackbar right away when offline pages feature is not enabled since
-                // there is no need to wait to get the storage info.
-                if (bookmarkModel.getOfflinePageBridge() == null) {
-                    showSnackbarForAddingBookmark(bookmarkModel, snackbarManager, activity,
-                            bookmarkId, saveResult, false, webContents);
-                    return;
+                // TODO(ianwen): Handle the case where the saveResult == ERROR.
+                Snackbar snackbar;
+                String folderName = bookmarkModel.getBookmarkTitle(
+                        bookmarkModel.getBookmarkById(bookmarkId).getParentId());
+                SnackbarController snackbarController =
+                        createSnackbarControllerForEditButton(activity, bookmarkId);
+                if (getLastUsedParent(activity) == null) {
+                    snackbar = Snackbar.make(activity.getString(R.string.bookmark_page_saved),
+                            snackbarController, Snackbar.TYPE_ACTION);
+                } else {
+                    snackbar = Snackbar.make(folderName, snackbarController, Snackbar.TYPE_ACTION)
+                                       .setTemplateText(activity.getString(
+                                               R.string.bookmark_page_saved_folder));
                 }
+                snackbar = snackbar.setSingleLine(false).setAction(
+                        activity.getString(R.string.bookmark_item_edit), null);
 
-                // Gets the storage info asynchronously which is needed to produce the message for
-                // the snackbar.
-                new AsyncTask<Void, Void, Boolean>() {
-                    @Override
-                    protected Boolean doInBackground(Void... params) {
-                        return OfflinePageUtils.isStorageAlmostFull();
-                    }
-
-                    @Override
-                    protected void onPostExecute(Boolean isStorageAlmostFull) {
-                        showSnackbarForAddingBookmark(bookmarkModel, snackbarManager, activity,
-                                bookmarkId, saveResult, isStorageAlmostFull, webContents);
-                    }
-                }.execute();
+                snackbarManager.showSnackbar(snackbar);
+                bookmarkModel.destroy();
             }
         };
     }
@@ -222,20 +142,12 @@ public class BookmarkUtils {
             @Override
             public void onAction(Object actionData) {
                 RecordUserAction.record("EnhancedBookmarks.EditAfterCreateButtonClicked");
-                startEditActivity(activity, bookmarkId, (WebContents) actionData);
+                startEditActivity(activity, bookmarkId);
             }
         };
     }
 
     /**
-     * Gets whether bookmark manager should load offline page initially.
-     */
-    private static boolean shouldShowOfflinePageAtFirst(OfflinePageBridge bridge) {
-        return !OfflinePageUtils.isConnected() && bridge != null
-                && bridge.maybeHasPages(OfflinePageBridge.BOOKMARK_NAMESPACE);
-    }
-
-    /**
      * Shows bookmark main UI.
      */
     public static void showBookmarkManager(Activity activity) {
@@ -255,14 +167,6 @@ public class BookmarkUtils {
      * experiments we run.
      */
     private static String getFirstUrlToLoad(Activity activity) {
-        BookmarkModel model = new BookmarkModel();
-        OfflinePageBridge bridge = model.getOfflinePageBridge();
-        model.destroy();
-
-        if (shouldShowOfflinePageAtFirst(bridge)) {
-            return BookmarkUIState.createFilterUrl(BookmarkFilter.OFFLINE_PAGES, false).toString();
-        }
-
         String lastUsedUrl = getLastUsedUrl(activity);
         return TextUtils.isEmpty(lastUsedUrl) ? UrlConstants.BOOKMARKS_URL : lastUsedUrl;
     }
@@ -305,19 +209,10 @@ public class BookmarkUtils {
                 preferences.getString(PREF_LAST_USED_PARENT, null));
     }
 
-    /**
-     * Starts an {@link BookmarkEditActivity} for the given {@link BookmarkId}.
-     * If the given {@link WebContents} is null, an option to visit the page is shown
-     * as opposed to showing an option to directly save the page
-     * (only if offline pages are enabled).
-     */
-    public static void startEditActivity(
-            Context context, BookmarkId bookmarkId, WebContents webContents) {
+    /** Starts an {@link BookmarkEditActivity} for the given {@link BookmarkId}. */
+    public static void startEditActivity(Context context, BookmarkId bookmarkId) {
         Intent intent = new Intent(context, BookmarkEditActivity.class);
         intent.putExtra(BookmarkEditActivity.INTENT_BOOKMARK_ID, bookmarkId.toString());
-        if (webContents != null) {
-            intent.putExtra(BookmarkEditActivity.INTENT_WEB_CONTENTS, webContents);
-        }
         if (context instanceof BookmarkActivity) {
             ((BookmarkActivity) context).startActivityForResult(
                     intent, BookmarkActivity.EDIT_BOOKMARK_REQUEST_CODE);
@@ -338,19 +233,11 @@ public class BookmarkUtils {
             BookmarkId bookmarkId, int launchLocation) {
         if (model.getBookmarkById(bookmarkId) == null) return false;
 
-        String url = model.getLaunchUrlAndMarkAccessed(bookmarkId);
-
-        // TODO(jianli): Notify the user about the failure.
-        if (TextUtils.isEmpty(url)) return false;
+        String url = model.getBookmarkById(bookmarkId).getUrl();
 
         NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_BOOKMARK);
-        if (url.startsWith("file:")) {
-            RecordHistogram.recordEnumeratedHistogram(
-                    "OfflinePages.LaunchLocation", launchLocation, BookmarkLaunchLocation.COUNT);
-        } else {
-            RecordHistogram.recordEnumeratedHistogram(
-                    "Stars.LaunchLocation", launchLocation, BookmarkLaunchLocation.COUNT);
-        }
+        RecordHistogram.recordEnumeratedHistogram(
+                "Stars.LaunchLocation", launchLocation, BookmarkLaunchLocation.COUNT);
 
         openUrl(activity, url);
         return true;