Commit 4699d649 authored by Lijin Shen's avatar Lijin Shen Committed by Commit Bot

Add speculative checks and logs on bookmark adding failure

Though no idea the cause of failure, this cl tries to add some if checks
on the parent bookmark item to prevent the potential failure. It will
also log some common info for the sake of future debug.

Bug: 986978
Change-Id: I7466cb49bdf8bf04d683dac86cf4ab670f1984a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1811797Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Commit-Queue: Lijin Shen <lazzzis@google.com>
Cr-Commit-Position: refs/heads/master@{#699557}
parent 80b87ad6
......@@ -15,6 +15,7 @@ import android.text.TextUtils;
import org.chromium.base.BuildInfo;
import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.RecordHistogram;
......@@ -23,6 +24,7 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.bookmarks.BookmarkBridge.BookmarkItem;
import org.chromium.chrome.browser.document.ChromeLauncherActivity;
import org.chromium.chrome.browser.snackbar.Snackbar;
import org.chromium.chrome.browser.snackbar.SnackbarManager;
......@@ -42,6 +44,7 @@ import org.chromium.ui.base.PageTransition;
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";
private static final String TAG = "BookmarkUtils";
/**
* If the tab has already been bookmarked, start {@link BookmarkEditActivity} for the
......@@ -67,14 +70,8 @@ public class BookmarkUtils {
return bookmarkId;
}
BookmarkId parent = getLastUsedParent(activity);
if (parent == null || !bookmarkModel.doesBookmarkExist(parent)) {
parent = bookmarkModel.getDefaultFolder();
}
String url = tab.getOriginalUrl();
BookmarkId bookmarkId = bookmarkModel.addBookmark(parent,
bookmarkModel.getChildCount(parent), tab.getTitle(), url);
BookmarkId bookmarkId =
addBookmarkInternal(activity, bookmarkModel, tab.getTitle(), tab.getOriginalUrl());
Snackbar snackbar = null;
if (bookmarkId == null) {
......@@ -127,12 +124,39 @@ public class BookmarkUtils {
*/
public static BookmarkId addBookmarkSilently(
Context context, BookmarkModel bookmarkModel, String title, String url) {
return addBookmarkInternal(context, bookmarkModel, title, url);
}
/**
* An internal version of {@link #addBookmarkSilently(Context, BookmarkModel, String, String)}.
* Will reset last used parent if it fails to add a bookmark
*/
private static BookmarkId addBookmarkInternal(
Context context, BookmarkModel bookmarkModel, String title, String url) {
BookmarkId parent = getLastUsedParent(context);
if (parent == null || !bookmarkModel.doesBookmarkExist(parent)) {
BookmarkItem parentItem = null;
if (parent != null) {
parentItem = bookmarkModel.getBookmarkById(parent);
}
if (parent == null || parentItem == null || parentItem.isManaged() || !parentItem.isFolder()
|| !parentItem.isEditable()) {
parent = bookmarkModel.getDefaultFolder();
}
BookmarkId bookmarkId =
bookmarkModel.addBookmark(parent, bookmarkModel.getChildCount(parent), title, url);
return bookmarkModel.addBookmark(parent, bookmarkModel.getChildCount(parent), title, url);
// TODO(lazzzis): remove log after bookmark sync is fixed, crbug.com/986978
if (bookmarkId == null) {
Log.e(TAG,
"Failed to add bookmarks: parentTypeAndId %s, defaultFolderTypeAndId %s, "
+ "mobileFolderTypeAndId %s, parentEditable Managed isFolder %s,",
parent, bookmarkModel.getDefaultFolder(), bookmarkModel.getMobileFolderId(),
parentItem == null ? "null"
: (parentItem.isEditable() + " " + parentItem.isManaged()
+ " " + parentItem.isFolder()));
setLastUsedParent(context, bookmarkModel.getDefaultFolder());
}
return bookmarkId;
}
/**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment