Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2397)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java

Issue 1894703002: [Offline pages] Removing offline pages from Bookmarks UI (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Marking more methods as visible for testing Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..fb7d48fa478afaa8684a1ba2689fc4990d4f2d89 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;
@@ -21,11 +20,8 @@ import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.IntentHandler;
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;
@@ -33,7 +29,7 @@ import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.components.bookmarks.BookmarkId;
import org.chromium.components.bookmarks.BookmarkType;
-import org.chromium.content_public.browser.WebContents;
+import org.chromium.components.dom_distiller.core.DomDistillerUrlUtils;
import org.chromium.ui.base.DeviceFormFactor;
/**
@@ -49,23 +45,21 @@ public class BookmarkUtils {
*
* 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 existingBookmarkId 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.
+ * @return Bookmark ID of the bookmark. Could be <code>null</code> if bookmark didn't exist
+ * and bookmark model failed to create it.
*/
- public static void addOrEditBookmark(long idToAdd, BookmarkModel bookmarkModel,
+ public static BookmarkId addOrEditBookmark(long existingBookmarkId, 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);
+ if (existingBookmarkId != Tab.INVALID_BOOKMARK_ID) {
+ BookmarkId bookmarkId = new BookmarkId(existingBookmarkId, BookmarkType.NORMAL);
+ startEditActivity(activity, bookmarkId);
bookmarkModel.destroy();
- return;
+ return bookmarkId;
}
BookmarkId parent = getLastUsedParent(activity);
@@ -73,44 +67,14 @@ public class BookmarkUtils {
parent = bookmarkModel.getDefaultFolder();
}
- // The bookmark model will be destroyed in the created AddBookmarkCallback.
- bookmarkModel.addBookmarkAsync(parent, bookmarkModel.getChildCount(parent), tab.getTitle(),
- tab.getUrl(), webContentsToSave,
- createAddBookmarkCallback(bookmarkModel, snackbarManager, activity,
- 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,
- BookmarkModel bookmarkModel, String title, String url) {
- BookmarkId parent = getLastUsedParent(context);
- if (parent == null || !bookmarkModel.doesBookmarkExist(parent)) {
- parent = bookmarkModel.getDefaultFolder();
- }
+ String url = DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(tab.getUrl());
+ BookmarkId bookmarkId = bookmarkModel.addBookmark(
+ parent, bookmarkModel.getChildCount(parent), tab.getTitle(), url);
- return bookmarkModel.addBookmark(parent, bookmarkModel.getChildCount(parent), title, url);
- }
-
- /**
- * 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());
+ if (bookmarkId != null) {
+ Snackbar snackbar;
+ String folderName = bookmarkModel.getBookmarkTitle(
+ bookmarkModel.getBookmarkById(bookmarkId).getParentId());
SnackbarController snackbarController =
createSnackbarControllerForEditButton(activity, bookmarkId);
if (getLastUsedParent(activity) == null) {
@@ -118,91 +82,34 @@ public class BookmarkUtils {
snackbarController, Snackbar.TYPE_ACTION);
} else {
snackbar = Snackbar.make(folderName, snackbarController, Snackbar.TYPE_ACTION)
- .setTemplateText(activity.getString(R.string.bookmark_page_saved_folder));
+ .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;
+ snackbar = snackbar.setSingleLine(false).setAction(
+ activity.getString(R.string.bookmark_item_edit), null);
- 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);
}
- snackbarManager.showSnackbar(snackbar);
bookmarkModel.destroy();
+ return bookmarkId;
}
/**
- * Shows a snackbar if necessary after adding a bookmark.
+ * 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.
*
- * NOTE: This callback will call BookmarkModel#destroy() on the passed-in bookmark model.
+ * @param title The title of the bookmark.
+ * @param url The URL of the new bookmark.
*/
- private static AddBookmarkCallback createAddBookmarkCallback(
- final BookmarkModel bookmarkModel, final SnackbarManager snackbarManager,
- final Activity activity, final WebContents webContents) {
- 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;
- }
-
- // 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();
- }
+ public static BookmarkId addBookmarkSilently(
+ Context context, BookmarkModel bookmarkModel, String title, String url) {
+ BookmarkId parent = getLastUsedParent(context);
+ if (parent == null || !bookmarkModel.doesBookmarkExist(parent)) {
+ parent = bookmarkModel.getDefaultFolder();
+ }
- @Override
- protected void onPostExecute(Boolean isStorageAlmostFull) {
- showSnackbarForAddingBookmark(bookmarkModel, snackbarManager, activity,
- bookmarkId, saveResult, isStorageAlmostFull, webContents);
- }
- }.execute();
- }
- };
+ return bookmarkModel.addBookmark(parent, bookmarkModel.getChildCount(parent), title, url);
}
/**
@@ -222,20 +129,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) {
@@ -251,18 +150,9 @@ public class BookmarkUtils {
}
/**
- * The initial url the bookmark manager shows depends on offline page status and some
- * experiments we run.
+ * The initial url the bookmark manager shows depends some 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 +195,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);
@@ -327,7 +208,7 @@ public class BookmarkUtils {
}
/**
- * Opens a bookmark depending on connection status and reports UMA.
+ * Opens a bookmark and reports UMA.
* @param model Bookmarks model to manage the bookmark.
* @param activity Activity requesting to open the bookmark.
* @param bookmarkId ID of the bookmark to be opened.
@@ -338,19 +219,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;
@@ -384,12 +257,4 @@ public class BookmarkUtils {
((Activity) context).finish();
}
}
-
- /**
- * 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();
- }
}

Powered by Google App Engine
This is Rietveld 408576698