Skip to content

Commit

Permalink
Revert "[bookmarks] Cleanup and unify bookmark opening logic"
Browse files Browse the repository at this point in the history
This reverts commit 82f00b7.

Reason for revert: Crashes bookmarks page on Tablets.

Original change's description:
> [bookmarks] Cleanup and unify bookmark opening logic
>
> History doesn't seem to have this problem. Using the additionalUrls
> approach and doing some cleanup along the way.
>
> Bug: 1411914, 1407633
> Change-Id: I47ba9efb65572946a2899a8d2550682a2ea6e030
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4210860
> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
> Reviewed-by: Sky Malice <skym@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: Weilun Shi <sweilun@chromium.org>
> Commit-Queue: Brandon Wylie <wylieb@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1102294}

Bug: 1411914, 1407633, 1413992
Change-Id: Ica0317ffd6ab3389b88216c637b723b609372f17
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4233502
Reviewed-by: Krishna Govind <govind@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Owners-Override: Krishna Govind <govind@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1102842}
  • Loading branch information
Sky Malice authored and Chromium LUCI CQ committed Feb 8, 2023
1 parent d4c2802 commit 9533d0b
Show file tree
Hide file tree
Showing 10 changed files with 180 additions and 529 deletions.
1 change: 0 additions & 1 deletion chrome/android/chrome_java_sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/bookmarks/BookmarkManager.java",
"java/src/org/chromium/chrome/browser/bookmarks/BookmarkModel.java",
"java/src/org/chromium/chrome/browser/bookmarks/BookmarkModelObserver.java",
"java/src/org/chromium/chrome/browser/bookmarks/BookmarkOpener.java",
"java/src/org/chromium/chrome/browser/bookmarks/BookmarkPage.java",
"java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java",
"java/src/org/chromium/chrome/browser/bookmarks/BookmarkRow.java",
Expand Down
1 change: 0 additions & 1 deletion chrome/android/chrome_test_java_sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/background_sync/PeriodicBackgroundSyncTest.java",
"javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkBridgeTest.java",
"javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkModelTest.java",
"javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkOpenerTest.java",
"javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkPersonalizedSigninPromoDismissTest.java",
"javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkPersonalizedSigninPromoTest.java",
"javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkSaveFlowTest.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
import org.chromium.chrome.browser.commerce.ShoppingServiceFactory;
import org.chromium.chrome.browser.partnerbookmarks.PartnerBookmarksReader;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.renderer_host.ChromeNavigationUIData;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.AsyncTabCreationParams;
import org.chromium.chrome.browser.tabmodel.document.TabDelegate;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.ui.native_page.BasicNativePage;
import org.chromium.components.bookmarks.BookmarkId;
Expand All @@ -40,6 +45,7 @@
import org.chromium.components.browser_ui.widget.selectable_list.SelectableListToolbar.SearchDelegate;
import org.chromium.components.browser_ui.widget.selectable_list.SelectionDelegate;
import org.chromium.components.favicon.LargeIconBridge;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.url.GURL;

import java.util.List;
Expand All @@ -59,6 +65,7 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate,
private static boolean sPreventLoadingForTesting;

private Context mContext;
private ComponentName mOpenBookmarkComponentName;
private ViewGroup mMainView;
private BookmarkModel mBookmarkModel;
private BookmarkUndoController mUndoController;
Expand Down Expand Up @@ -95,7 +102,6 @@ public synchronized BookmarkUIState pop() {
private BookmarkItemsAdapter mAdapter;
private BookmarkDragStateDelegate mDragStateDelegate;
private AdapterDataObserver mAdapterDataObserver;
private final BookmarkOpener mBookmarkOpener;

private final ObservableSupplierImpl<Boolean> mBackPressStateSupplier =
new ObservableSupplierImpl<>();
Expand Down Expand Up @@ -207,6 +213,7 @@ public void setA11yStateForTesting(boolean a11yEnabled) {
public BookmarkManager(Context context, ComponentName openBookmarkComponentName,
boolean isDialogUi, boolean isIncognito, SnackbarManager snackbarManager) {
mContext = context;
mOpenBookmarkComponentName = openBookmarkComponentName;
mIsDialogUi = isDialogUi;
mIsIncognito = isIncognito;

Expand Down Expand Up @@ -290,8 +297,6 @@ public void onChanged() {
FAVICON_MAX_CACHE_SIZE_BYTES);
mLargeIconBridge.createCache(maxSize);

mBookmarkOpener = new BookmarkOpener(mBookmarkModel, mContext, openBookmarkComponentName);

RecordUserAction.record("MobileBookmarkManagerOpen");
if (!isDialogUi) {
RecordUserAction.record("MobileBookmarkManagerPageOpen");
Expand Down Expand Up @@ -563,7 +568,10 @@ public void notifyStateChange(BookmarkUIObserver observer) {

@Override
public void openBookmark(BookmarkId bookmark) {
if (!mBookmarkOpener.openBookmarkInCurrentTab(bookmark, mIsIncognito)) return;
if (!BookmarkUtils.openBookmark(
mContext, mOpenBookmarkComponentName, mBookmarkModel, bookmark, mIsIncognito)) {
return;
}

// Close bookmark UI. Keep the reading list page open.
if (bookmark != null && bookmark.getType() != BookmarkType.READING_LIST) {
Expand All @@ -573,8 +581,21 @@ public void openBookmark(BookmarkId bookmark) {

@Override
public void openBookmarksInNewTabs(List<BookmarkId> bookmarks, boolean incognito) {
if (mBookmarkOpener.openBookmarksInNewTabs(bookmarks, incognito)) {
BookmarkUtils.finishActivityOnPhone(mContext);
TabDelegate tabDelegate = new TabDelegate(incognito);
for (BookmarkId id : bookmarks) {
if (id == null) continue;
GURL url = mBookmarkModel.getBookmarkById(id).getUrl();
LoadUrlParams params = new LoadUrlParams(url);
ChromeNavigationUIData navData = new ChromeNavigationUIData();
navData.setBookmarkId(id.getType() == BookmarkType.NORMAL ? id.getId() : -1);
params.setNavigationUIDataSupplier(navData::createUnownedNativeCopy);
AsyncTabCreationParams asyncParams =
new AsyncTabCreationParams(params, mOpenBookmarkComponentName);
tabDelegate.createNewTab(
asyncParams, TabLaunchType.FROM_LONGPRESS_BACKGROUND, Tab.INVALID_TAB_ID);
if (id.getType() == BookmarkType.READING_LIST) {
mBookmarkModel.setReadStatusForReadingList(url, true);
}
}
}

Expand Down Expand Up @@ -670,8 +691,4 @@ public RecyclerView getRecyclerViewForTests() {
public static void preventLoadingForTesting(boolean preventLoading) {
sPreventLoadingForTesting = preventLoading;
}

public BookmarkOpener getBookmarkOpenerForTesting() {
return mBookmarkOpener;
}
}

This file was deleted.

0 comments on commit 9533d0b

Please sign in to comment.