Skip to content

Commit

Permalink
Remove getLastUsedRegularProfile from AppMenuPropertiesDelegateImpl
Browse files Browse the repository at this point in the history
This introduced a method in ShoppingFeatures that took an explicit
Profile. Upon that, this fixes the easier usage patterns of the
utility method where the profile was readily available.

Bug: 1410601
Change-Id: I482c76118361159d01d729c063767b3ccdd3289d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4939750
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1209773}
  • Loading branch information
Ted Choc authored and Chromium LUCI CQ committed Oct 14, 2023
1 parent 3f0a610 commit 6dc07c4
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ private void preparePageMenu(
menu.findItem(R.id.get_image_descriptions_id).setVisible(true);

int titleId = R.string.menu_stop_image_descriptions;
Profile profile = Profile.getLastUsedRegularProfile();
Profile profile = Profile.fromWebContents(currentTab.getWebContents());
// If image descriptions are not enabled, then we want the menu item to be "Get".
if (!ImageDescriptionsController.getInstance().imageDescriptionsEnabled(profile)) {
titleId = R.string.menu_get_image_descriptions;
Expand Down Expand Up @@ -1086,15 +1086,23 @@ protected void updateBookmarkMenuItemRow(
*/
protected void updatePriceTrackingMenuItemRow(@NonNull MenuItem startPriceTrackingMenuItem,
@NonNull MenuItem stopPriceTrackingMenuItem, @Nullable Tab currentTab) {
ShoppingService service =
ShoppingServiceFactory.getForProfile(Profile.getLastUsedRegularProfile());
if (currentTab == null || currentTab.getWebContents() == null) {
startPriceTrackingMenuItem.setVisible(false);
stopPriceTrackingMenuItem.setVisible(false);
return;
}

Profile profile = Profile.fromWebContents(currentTab.getWebContents());
assert profile != null;

ShoppingService service = ShoppingServiceFactory.getForProfile(profile);
ShoppingService.ProductInfo info = null;
if (service != null && currentTab != null) {
if (service != null) {
info = service.getAvailableProductInfoForUrl(currentTab.getUrl());
}

// If price tracking isn't enabled or the page isn't eligible, then hide both items.
if (!ShoppingFeatures.isShoppingListEligible()
if (!ShoppingFeatures.isShoppingListEligible(profile)
|| !PowerBookmarkUtils.isPriceTrackingEligible(currentTab)
|| !mBookmarkModelSupplier.hasValue()) {
startPriceTrackingMenuItem.setVisible(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public BookmarkManagerCoordinator(Context context, ComponentName openBookmarkCom
mMainView = (ViewGroup) LayoutInflater.from(context).inflate(R.layout.bookmark_main, null);
mBookmarkModel = BookmarkModel.getForProfile(profile);
mBookmarkOpener = new BookmarkOpener(mBookmarkModel, context, openBookmarkComponentName);
if (ShoppingFeatures.isShoppingListEligible()) {
if (ShoppingFeatures.isShoppingListEligible(profile)) {
ShoppingServiceFactory.getForProfile(profile).scheduleSavedProductUpdate();
}
mBookmarkUiPrefs = bookmarkUiPrefs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,7 @@ private void assertIsAndroidImprovedBookmarksEnabled() {
// properly.
@VisibleForTesting
void updateShoppingFilterVisible() {
boolean eligible = ShoppingFeatures.isShoppingListEligible();
boolean eligible = ShoppingFeatures.isShoppingListEligible(mProfile);
if (!eligible) {
updateFilterAvailability(false);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ void show(BookmarkId bookmarkId, boolean fromExplicitTrackUi, boolean wasBookmar
setupAutodismiss();
}

if (ShoppingFeatures.isShoppingListEligible()) {
if (ShoppingFeatures.isShoppingListEligible(mProfile)) {
PriceTrackingUtils.isBookmarkPriceTracked(mProfile, bookmarkId.getId(), (isTracked) -> {
if (isTracked) return;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,13 @@ public static boolean isShoppingListItem(PowerBookmarkMeta meta) {
* @return Whether the given tab is eligible for price-tracking.
*/
public static boolean isPriceTrackingEligible(@Nullable Tab tab) {
if (tab == null) return false;
if (tab == null || tab.getWebContents() == null) return false;
if (sPriceTrackingEligibleForTesting != null) return sPriceTrackingEligibleForTesting;

ShoppingService service =
ShoppingServiceFactory.getForProfile(Profile.getLastUsedRegularProfile());
Profile profile = Profile.fromWebContents(tab.getWebContents());
assert profile != null;

ShoppingService service = ShoppingServiceFactory.getForProfile(profile);
if (service == null) return false;

ShoppingService.ProductInfo info = service.getAvailableProductInfoForUrl(tab.getUrl());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public void onCreatePreferences(@Nullable Bundle savedInstanceState, String root
}

mPriceNotificationSection = findPreference(PREF_PRICE_NOTIFICATION_SECTION);
if (ShoppingFeatures.isShoppingListEligible()) {
if (ShoppingFeatures.isShoppingListEligible(getProfile())) {
mPriceNotificationSection.setVisible(true);
} else {
removePreference(getPreferenceScreen(), mPriceNotificationSection);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ public void destroy() {
* @param tab The tab currently being displayed to the user.
*/
private void showPriceTrackingIPH(Tab tab) {
if (!ShoppingFeatures.isShoppingListEligible()
if (tab == null || tab.getWebContents() == null) return;

if (!ShoppingFeatures.isShoppingListEligible(Profile.fromWebContents(tab.getWebContents()))
|| !PowerBookmarkUtils.isPriceTrackingEligible(tab)
|| AdaptiveToolbarFeatures.isContextualPageActionUiEnabled()) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
import org.chromium.chrome.browser.preferences.Pref;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.profiles.ProfileJni;
import org.chromium.chrome.browser.readaloud.ReadAloudController;
import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
import org.chromium.chrome.browser.tab.Tab;
Expand Down Expand Up @@ -158,8 +159,8 @@ public class AppMenuPropertiesDelegateUnitTest {
private UpdateMenuItemHelper mUpdateMenuItemHelper;
@Mock
private UserPrefs.Natives mUserPrefsJniMock;
@Mock
private Profile mProfile;
@Mock private Profile.Natives mProfileJniMock;
@Mock private Profile mProfile;
@Mock
private PrefService mPrefService;
@Mock
Expand Down Expand Up @@ -232,9 +233,10 @@ public void setUp() {
doReturn(mMenuUiState).when(mUpdateMenuItemHelper).getUiState();

mJniMocker.mock(UserPrefsJni.TEST_HOOKS, mUserPrefsJniMock);
mJniMocker.mock(ProfileJni.TEST_HOOKS, mProfileJniMock);
mJniMocker.mock(WebsitePreferenceBridgeJni.TEST_HOOKS, mWebsitePreferenceBridgeJniMock);
Profile.setLastUsedProfileForTesting(mProfile);
Mockito.when(mUserPrefsJniMock.get(mProfile)).thenReturn(mPrefService);
Mockito.when(mProfileJniMock.fromWebContents(mWebContents)).thenReturn(mProfile);
FeatureList.setTestCanUseDefaultsForTesting();
PowerBookmarkUtils.setPriceTrackingEligibleForTesting(false);
WebappRegistry.refreshSharedPrefsForTesting();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,19 @@
public class ShoppingFeatures {
private static Boolean sShoppingListEligibleForTestsing;

/** Wrapper function for ShoppingService.isShoppingListEligibile(). */
/** Use {@link #isShoppingListEligible(Profile)} */
@Deprecated
public static boolean isShoppingListEligible() {
if (sShoppingListEligibleForTestsing != null) return sShoppingListEligibleForTestsing;

if (!ProfileManager.isInitialized()) return false;
return isShoppingListEligible(Profile.getLastUsedRegularProfile());
}

/** Wrapper function for ShoppingService.isShoppingListEligibile(). */
public static boolean isShoppingListEligible(Profile profile) {
if (sShoppingListEligibleForTestsing != null) return sShoppingListEligibleForTestsing;

Profile profile = Profile.getLastUsedRegularProfile();
if (profile == null) return false;
ShoppingService service = ShoppingServiceFactory.getForProfile(profile);
if (service == null) return false;
Expand Down

0 comments on commit 6dc07c4

Please sign in to comment.