Skip to content

Commit

Permalink
Add bridge for price tracking utils and use the new setter
Browse files Browse the repository at this point in the history
This patch adds a bridge for java to use the price_tracking_utils in
the commerce component and has existing code use the now-available
SetPriceTrackingStateForBookmark. This allows for the removal of
a large amount of existing code since the utility in components
handles all the edge cases of setting the state in the bookmarks
system and in subscriptions for free.

The bridge lives in chrome/browser since we don't have a java
BookmarkModel in components. Rather than passing each of the
ShoppingService and BookmarkModel, the profile is passed and each is
acquired in native through the respective factories.

Most significantly, these changes switch the android implementation
of price tracking to primarily use the native implementation of the
subscriptions manager.

Bug: 1351830, 1268976
Change-Id: I0291901ad19d1e06cf34d3aaeb768996da0b25b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3978289
Reviewed-by: Zhiyuan Cai <zhiyuancai@chromium.org>
Reviewed-by: Brandon Wylie <wylieb@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1067535}
  • Loading branch information
iotitan authored and Chromium LUCI CQ committed Nov 4, 2022
1 parent bda41dc commit c69516e
Show file tree
Hide file tree
Showing 12 changed files with 156 additions and 198 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
import org.chromium.chrome.browser.bookmarks.BookmarkModel;
import org.chromium.chrome.browser.bookmarks.PowerBookmarkUtils;
import org.chromium.chrome.browser.bookmarks.TabBookmarker;
import org.chromium.chrome.browser.commerce.ShoppingFeatures;
import org.chromium.chrome.browser.compositor.CompositorViewHolder;
import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.compositor.layouts.LayoutManagerImpl;
Expand Down Expand Up @@ -154,8 +153,6 @@
import org.chromium.chrome.browser.share.ShareDelegateImpl;
import org.chromium.chrome.browser.share.ShareDelegateSupplier;
import org.chromium.chrome.browser.stylus_handwriting.StylusWritingCoordinator;
import org.chromium.chrome.browser.subscriptions.CommerceSubscriptionsServiceFactory;
import org.chromium.chrome.browser.subscriptions.SubscriptionsManager;
import org.chromium.chrome.browser.sync.SyncService;
import org.chromium.chrome.browser.tab.AccessibilityVisibilityHandler;
import org.chromium.chrome.browser.tab.RequestDesktopUtils;
Expand Down Expand Up @@ -197,7 +194,6 @@
import org.chromium.chrome.browser.ui.system.StatusBarColorController;
import org.chromium.chrome.browser.vr.ArDelegateProvider;
import org.chromium.chrome.browser.vr.VrModuleProvider;
import org.chromium.components.bookmarks.BookmarkId;
import org.chromium.components.browser_ui.accessibility.FontSizePrefs;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.modaldialog.AppModalPresenter;
Expand Down Expand Up @@ -2530,19 +2526,9 @@ public boolean onMenuOrKeyboardAction(int id, boolean fromMenu) {
}

if (id == R.id.disable_price_tracking_menu_id) {
// TODO(crbug.com/1268976): Extract this code into a one-liner.
List<BookmarkId> bookmarkIds =
PowerBookmarkUtils.getBookmarkIdsWithSharedClusterIdForTab(
currentTab, mBookmarkModelSupplier.get());
SubscriptionsManager subscriptionsManager = null;
if (ShoppingFeatures.isShoppingListEnabled()) {
subscriptionsManager = new CommerceSubscriptionsServiceFactory()
.getForLastUsedProfile()
.getSubscriptionsManager();
}
PowerBookmarkUtils.setPriceTrackingEnabledWithSnackbars(subscriptionsManager,
mBookmarkModelSupplier.get(), bookmarkIds,
/*enabled=*/false, mSnackbarManager, getResources());
PowerBookmarkUtils.setPriceTrackingEnabledWithSnackbars(mBookmarkModelSupplier.get(),
mBookmarkModelSupplier.get().getUserBookmarkIdForTab(currentTab),
/*enabled=*/false, mSnackbarManager, getResources(), (success) -> {});
RecordUserAction.record("MobileMenuDisablePriceTracking");
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@
import org.chromium.chrome.R;
import org.chromium.chrome.browser.bookmarks.BookmarkListEntry.ViewType;
import org.chromium.chrome.browser.bookmarks.BookmarkRow.Location;
import org.chromium.chrome.browser.commerce.ShoppingFeatures;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.subscriptions.CommerceSubscriptionsServiceFactory;
import org.chromium.chrome.browser.subscriptions.SubscriptionsManager;
import org.chromium.chrome.browser.sync.SyncService;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.ui.signin.PersonalizedSigninPromoView;
Expand Down Expand Up @@ -75,8 +73,6 @@ public class BookmarkItemsAdapter extends DragReorderableListAdapter<BookmarkLis
// Keep track of the currently highlighted bookmark - used for "show in folder" action.
private BookmarkId mHighlightedBookmark;

private SubscriptionsManager mSubscriptionsManager;

private BookmarkModelObserver mBookmarkModelObserver = new BookmarkModelObserver() {
@Override
public void bookmarkNodeChanged(BookmarkItem node) {
Expand Down Expand Up @@ -134,11 +130,6 @@ public void bookmarkModelChanged() {
GlobalDiscardableReferencePool.getReferencePool());
mCommerceSubscriptionsServiceFactory = new CommerceSubscriptionsServiceFactory();
mSnackbarManager = snackbarManager;

if (ShoppingFeatures.isShoppingListEnabled()) {
mSubscriptionsManager = mCommerceSubscriptionsServiceFactory.getForLastUsedProfile()
.getSubscriptionsManager();
}
}

/**
Expand Down Expand Up @@ -254,8 +245,7 @@ public ViewHolder onCreateViewHolder(ViewGroup parent, @ViewType int viewType) {
if (BookmarkFeatures.isBookmarksVisualRefreshEnabled()) {
vh = createViewHolderHelper(parent, R.layout.power_bookmark_shopping_item_row);
((PowerBookmarkShoppingItemRow) vh.itemView)
.init(mImageFetcher, mDelegate.getModel(), mSubscriptionsManager,
mSnackbarManager);
.init(mImageFetcher, mDelegate.getModel(), mSnackbarManager);
} else {
vh = createViewHolderHelper(parent, R.layout.bookmark_item_row);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.bookmarks.PowerBookmarkMetrics.PriceTrackingState;
import org.chromium.chrome.browser.commerce.PriceTrackingUtils;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.subscriptions.CommerceSubscription;
Expand All @@ -41,7 +42,7 @@ public class BookmarkSaveFlowMediator
private boolean mWasBookmarkMoved;
private SubscriptionsManager mSubscriptionsManager;
private CommerceSubscription mSubscription;
private Callback<Integer> mSubscriptionsManagerCallback;
private Callback<Boolean> mSubscriptionsManagerCallback;
private String mFolderName;

/**
Expand Down Expand Up @@ -149,16 +150,16 @@ private void bindPowerBookmarkProperties(

void handleNotificationSwitchToggle(CompoundButton view, boolean toggled) {
if (mSubscriptionsManagerCallback == null) {
mSubscriptionsManagerCallback = mCallbackController.makeCancelable((Integer status) -> {
setPriceTrackingToggleVisualsOnly(
status == SubscriptionsManager.StatusCode.OK && view.isChecked());
setPriceTrackingNotificationUiEnabled(status == SubscriptionsManager.StatusCode.OK);
});
mSubscriptionsManagerCallback =
mCallbackController.makeCancelable((Boolean success) -> {
setPriceTrackingToggleVisualsOnly(success && view.isChecked());
setPriceTrackingNotificationUiEnabled(success);
});
}

setPriceTrackingIconForEnabledState(toggled);
PowerBookmarkUtils.setPriceTrackingEnabled(mSubscriptionsManager, mBookmarkModel,
mBookmarkId, toggled, mSubscriptionsManagerCallback);
PriceTrackingUtils.setPriceTrackingStateForBookmark(Profile.getLastUsedRegularProfile(),
mBookmarkId.getId(), toggled, mSubscriptionsManagerCallback);
PowerBookmarkMetrics.reportBookmarkSaveFlowPriceTrackingState(toggled
? PriceTrackingState.PRICE_TRACKING_ENABLED
: PriceTrackingState.PRICE_TRACKING_DISABLED);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.chromium.chrome.R;
import org.chromium.chrome.browser.bookmarks.PowerBookmarkMetrics.PriceTrackingState;
import org.chromium.chrome.browser.subscriptions.CommerceSubscription;
import org.chromium.chrome.browser.subscriptions.SubscriptionsManager;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.components.bookmarks.BookmarkId;
import org.chromium.components.bookmarks.BookmarkItem;
Expand All @@ -40,7 +39,6 @@ public class PowerBookmarkShoppingItemRow extends BookmarkItemRow {

private ImageFetcher mImageFetcher;
private BookmarkModel mBookmarkModel;
private SubscriptionsManager mSubscriptionsManager;

private boolean mIsPriceTrackingEnabled;
private CurrencyFormatter mCurrencyFormatter;
Expand All @@ -59,13 +57,11 @@ public PowerBookmarkShoppingItemRow(Context context, AttributeSet attrs) {
* Initialize properties for the item row.
* @param imageFetcher {@link ImageFetcher} used to fetch shopping images.
* @param bookmarkModel The {@link BookmarkModel} used to query power bookmark metadata.
* @param subscriptionsManager Used to manage the price-tracking subscriptions.
*/
void init(ImageFetcher imageFetcher, BookmarkModel bookmarkModel,
SubscriptionsManager subscriptionsManager, SnackbarManager snackbarManager) {
SnackbarManager snackbarManager) {
mImageFetcher = imageFetcher;
mBookmarkModel = bookmarkModel;
mSubscriptionsManager = subscriptionsManager;
mSnackbarManager = snackbarManager;
}

Expand Down Expand Up @@ -175,10 +171,10 @@ private void setPriceTrackingButton(boolean priceTrackingEnabled) {
: R.string.enable_price_tracking_menu_item));
mEndStartButtonView.setVisibility(View.VISIBLE);
updatePriceTrackingImageForCurrentState();
Callback<Integer> subscriptionCallback = (status) -> {
Callback<Boolean> subscriptionCallback = (success) -> {
mSubscriptionChangeInProgress = false;
// TODO(crbug.com/1243383): Handle the failure edge case.
if (status != SubscriptionsManager.StatusCode.OK) return;
if (!success) return;
mIsPriceTrackingEnabled = !mIsPriceTrackingEnabled;
updatePriceTrackingImageForCurrentState();
};
Expand All @@ -189,9 +185,9 @@ private void setPriceTrackingButton(boolean priceTrackingEnabled) {
PowerBookmarkMetrics.reportBookmarkShoppingItemRowPriceTrackingState(
!mIsPriceTrackingEnabled ? PriceTrackingState.PRICE_TRACKING_ENABLED
: PriceTrackingState.PRICE_TRACKING_DISABLED);
PowerBookmarkUtils.setPriceTrackingEnabledWithSnackbars(mSubscriptionsManager,
mBookmarkModel, mBookmarkId, !mIsPriceTrackingEnabled, mSnackbarManager,
getContext().getResources(), subscriptionCallback);
PowerBookmarkUtils.setPriceTrackingEnabledWithSnackbars(mBookmarkModel, mBookmarkId,
!mIsPriceTrackingEnabled, mSnackbarManager, getContext().getResources(),
subscriptionCallback);
});
}

Expand Down

0 comments on commit c69516e

Please sign in to comment.