From e80e2854d96c9673bafd21a34d93919dc397fede Mon Sep 17 00:00:00 2001 From: Zach Katz Date: Mon, 27 Mar 2023 21:39:20 +0000 Subject: [PATCH] [PCCT-SideSheet] Use PCCT in discover feed settings -Include feature flag (cherry picked from commit add35f1d3fa3168dd82c34e2567bebd64ae3da26) Bug: 1425232 Change-Id: I50833ccfd0be034055b18205c9334e81bbca734d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4347577 Commit-Queue: Zach Katz Reviewed-by: Justin DeWitt Reviewed-by: Theresa Sullivan Reviewed-by: Kevin Grosu Cr-Original-Commit-Position: refs/heads/main@{#1121881} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4375055 Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/branch-heads/5672@{#57} Cr-Branched-From: 5f2a72468eda1eb945b3b5a2298b5d1cd678521e-refs/heads/main@{#1121455} --- .../browser/app/flags/ChromeCachedFlags.java | 1 + chrome/browser/about_flags.cc | 6 ++ .../FeedManagementMediator.java | 57 ++++++++++--- .../FeedManagementMediatorTest.java | 79 ++++++++++++++++++- chrome/browser/flag-metadata.json | 5 ++ chrome/browser/flag_descriptions.cc | 4 + chrome/browser/flag_descriptions.h | 2 + .../flags/android/chrome_feature_list.cc | 5 ++ .../flags/android/chrome_feature_list.h | 1 + .../browser/flags/ChromeFeatureList.java | 4 + tools/metrics/histograms/enums.xml | 4 + 11 files changed, 152 insertions(+), 16 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/app/flags/ChromeCachedFlags.java b/chrome/android/java/src/org/chromium/chrome/browser/app/flags/ChromeCachedFlags.java index 459195d1c2b220..eede55e6f616d6 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/app/flags/ChromeCachedFlags.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/app/flags/ChromeCachedFlags.java @@ -81,6 +81,7 @@ public void cacheNativeFlags() { ChromeFeatureList.sCctResizable90MaximumHeight, ChromeFeatureList.sCctResizableForThirdParties, ChromeFeatureList.sCctResizableSideSheet, + ChromeFeatureList.sCctResizableSideSheetDiscoverFeedSettings, ChromeFeatureList.sCctResizableSideSheetForThirdParties, ChromeFeatureList.sCctRetainableStateInMemory, ChromeFeatureList.sCctToolbarCustomizations, diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index 4732c84e45b323..b0cf4a8755249a 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc @@ -6571,6 +6571,12 @@ const FeatureEntry kFeatureEntries[] = { {"cct-resizable-side-sheet", flag_descriptions::kCCTResizableSideSheetName, flag_descriptions::kCCTResizableSideSheetDescription, kOsAndroid, FEATURE_VALUE_TYPE(chrome::android::kCCTResizableSideSheet)}, + {"cct-resizable-side-sheet-discover-feed-settings", + flag_descriptions::kCCTResizableSideSheetDiscoverFeedSettingsName, + flag_descriptions::kCCTResizableSideSheetDiscoverFeedSettingsDescription, + kOsAndroid, + FEATURE_VALUE_TYPE( + chrome::android::kCCTResizableSideSheetDiscoverFeedSettings)}, {"cct-resizable-side-sheet-for-third-parties", flag_descriptions::kCCTResizableSideSheetForThirdPartiesName, flag_descriptions::kCCTResizableSideSheetForThirdPartiesDescription, diff --git a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/feedmanagement/FeedManagementMediator.java b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/feedmanagement/FeedManagementMediator.java index a8e416723bd0d3..ecf1e599c9b520 100644 --- a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/feedmanagement/FeedManagementMediator.java +++ b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/feedmanagement/FeedManagementMediator.java @@ -14,7 +14,9 @@ import android.view.View.OnClickListener; import androidx.annotation.VisibleForTesting; +import androidx.browser.customtabs.CustomTabsClient; import androidx.browser.customtabs.CustomTabsIntent; +import androidx.browser.customtabs.CustomTabsSession; import org.chromium.base.Log; import org.chromium.base.compat.ApiHelperForM; @@ -22,6 +24,7 @@ import org.chromium.chrome.browser.feed.R; import org.chromium.chrome.browser.feed.StreamKind; import org.chromium.chrome.browser.feed.v2.FeedUserActionType; +import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.ui.modelutil.MVCListAdapter.ModelList; import org.chromium.ui.modelutil.ModelListAdapter; import org.chromium.ui.modelutil.PropertyModel; @@ -41,6 +44,8 @@ public class FeedManagementMediator { private final FollowManagementLauncher mFollowManagementLauncher; private final AutoplayManagementLauncher mAutoplayManagementLauncher; private final @StreamKind int mInitiatingStreamKind; + private CustomTabsClient mClient; + private CustomTabsSession mCustomTabsSession; /** * Interface to supply a method which can launch the FollowManagementActivity. @@ -103,25 +108,51 @@ private PropertyModel generateListItem( // TODO(petewil): Borrowed these from code we can't link to. How do I keep them in sync? static final String TRUSTED_APPLICATION_CODE_EXTRA = "trusted_application_code_extra"; + // TODO(katzz): Replace with intent extras to be defined in AndroidX; + static final String EXTRA_ACTIVITY_INITIAL_WIDTH_PX = + "androidx.browser.customtabs.extra.INITIAL_ACTIVITY_WIDTH_PX"; + static final String EXTRA_ACTIVITY_INITIAL_HEIGHT_PX = + "androidx.browser.customtabs.extra.INITIAL_ACTIVITY_HEIGHT_PX"; + static final String EXTRA_ACTIVITY_SIDE_SHEET_BREAKPOINT_DP = + "androidx.browser.customtabs.extra.ACTIVITY_SIDE_SHEET_BREAKPOINT_DP"; // Launch a new activity in the same task with the given uri as a CCT. private void launchUriActivity(String uri) { CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder(); builder.setShowTitle(true); builder.setShareState(CustomTabsIntent.SHARE_STATE_ON); - Intent intent = builder.build().intent; - intent.setData(Uri.parse(uri)); - intent.setAction(Intent.ACTION_VIEW); - intent.setClassName(mContext, "org.chromium.chrome.browser.customtabs.CustomTabActivity"); - - // Do the things that createCustomTabActivityIntent does: - intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); // Needed for pre-N versions of android. - intent.putExtra(Browser.EXTRA_APPLICATION_ID, mContext.getPackageName()); - - // Adding trusted extras lets us know that the intent came from Chrome. - intent.setPackage(mContext.getPackageName()); - intent.putExtra(TRUSTED_APPLICATION_CODE_EXTRA, getAuthenticationToken()); - mContext.startActivity(intent); + if (ChromeFeatureList.sCctResizableSideSheetDiscoverFeedSettings.isEnabled()) { + int displayHeight = mContext.getResources().getDisplayMetrics().heightPixels; + int displayWidth = mContext.getResources().getDisplayMetrics().widthPixels; + int initialHeight = Math.max(displayHeight, displayWidth); + int initialWidth = Math.max(displayHeight, displayWidth) / 2; + int breakPoint = (int) (Math.min(displayHeight, displayWidth) + / mContext.getResources().getDisplayMetrics().density + + 1); + + CustomTabsIntent customTabsIntent = builder.build(); + customTabsIntent.intent.setPackage(mContext.getPackageName()); + // Adding trusted extras lets us know that the intent came from Chrome. + customTabsIntent.intent.putExtra( + TRUSTED_APPLICATION_CODE_EXTRA, getAuthenticationToken()); + customTabsIntent.intent.setData(Uri.parse(uri)); + customTabsIntent.intent.putExtra(EXTRA_ACTIVITY_INITIAL_HEIGHT_PX, initialHeight); + customTabsIntent.intent.putExtra(EXTRA_ACTIVITY_INITIAL_WIDTH_PX, initialWidth); + customTabsIntent.intent.putExtra(EXTRA_ACTIVITY_SIDE_SHEET_BREAKPOINT_DP, breakPoint); + customTabsIntent.launchUrl(mContext, Uri.parse(uri)); + } else { + Intent intent = builder.build().intent; + intent.setPackage(mContext.getPackageName()); + // Adding trusted extras lets us know that the intent came from Chrome. + intent.putExtra(TRUSTED_APPLICATION_CODE_EXTRA, getAuthenticationToken()); + intent.setData(Uri.parse(uri)); + intent.setAction(Intent.ACTION_VIEW); + intent.setClassName( + mContext, "org.chromium.chrome.browser.customtabs.CustomTabActivity"); + intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); // Needed for pre-N versions of android. + intent.putExtra(Browser.EXTRA_APPLICATION_ID, mContext.getPackageName()); + mContext.startActivity(intent); + } // TODO(https://crbug.com/1195209): Record uma by calling ReportOtherUserAction // on the stream. } diff --git a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/feedmanagement/FeedManagementMediatorTest.java b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/feedmanagement/FeedManagementMediatorTest.java index 4d3cf832186813..70c418d6854d94 100644 --- a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/feedmanagement/FeedManagementMediatorTest.java +++ b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/feedmanagement/FeedManagementMediatorTest.java @@ -5,6 +5,7 @@ package org.chromium.chrome.browser.feed.feedmanagement; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.mockito.Mockito.verify; import android.app.Activity; @@ -27,6 +28,7 @@ import org.chromium.chrome.browser.feed.FeedServiceBridgeJni; import org.chromium.chrome.browser.feed.StreamKind; import org.chromium.chrome.browser.feed.v2.FeedUserActionType; +import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.ui.modelutil.MVCListAdapter.ModelList; /** @@ -67,7 +69,8 @@ public void setUpTest() { } @Test - public void testHandleActivityClick() { + public void testHandleActivityClick_FlagDisabled() { + ChromeFeatureList.sCctResizableSideSheetDiscoverFeedSettings.setForTesting(false); // Act mFeedManagementMediator.handleActivityClick(null); @@ -75,12 +78,54 @@ public void testHandleActivityClick() { Intent intent = mShadowActivity.peekNextStartedActivityForResult().intent; assertEquals( intent.getData(), Uri.parse("https://myactivity.google.com/myactivity?product=50")); + assertEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_WIDTH_PX, 0)); + assertEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_HEIGHT_PX, 0)); + verify(mFeedServiceBridgeJniMock) + .reportOtherUserAction(TEST_STREAM_KIND, FeedUserActionType.TAPPED_MANAGE_ACTIVITY); + } + + @Test + public void testHandleActivityClick_FlagEnabled() { + ChromeFeatureList.sCctResizableSideSheetDiscoverFeedSettings.setForTesting(true); + // Act + mFeedManagementMediator.handleActivityClick(null); + + // Assert + Intent intent = mShadowActivity.peekNextStartedActivityForResult().intent; + assertEquals( + intent.getData(), Uri.parse("https://myactivity.google.com/myactivity?product=50")); + assertNotEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_WIDTH_PX, 0)); + assertNotEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_HEIGHT_PX, 0)); verify(mFeedServiceBridgeJniMock) .reportOtherUserAction(TEST_STREAM_KIND, FeedUserActionType.TAPPED_MANAGE_ACTIVITY); } @Test - public void testHandleInterestsClick() { + public void testHandleInterestsClick_FlagDisabled() { + ChromeFeatureList.sCctResizableSideSheetDiscoverFeedSettings.setForTesting(false); + // Act + mFeedManagementMediator.handleInterestsClick(null); + + // Assert + Intent intent = mShadowActivity.peekNextStartedActivityForResult().intent; + assertEquals(intent.getData(), + Uri.parse("https://www.google.com/preferences/interests/yourinterests?sh=n")); + assertEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_WIDTH_PX, 0)); + assertEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_HEIGHT_PX, 0)); + verify(mFeedServiceBridgeJniMock) + .reportOtherUserAction( + TEST_STREAM_KIND, FeedUserActionType.TAPPED_MANAGE_INTERESTS); + } + + @Test + public void testHandleInterestsClick_FlagEnabled() { + ChromeFeatureList.sCctResizableSideSheetDiscoverFeedSettings.setForTesting(true); // Act mFeedManagementMediator.handleInterestsClick(null); @@ -88,13 +133,37 @@ public void testHandleInterestsClick() { Intent intent = mShadowActivity.peekNextStartedActivityForResult().intent; assertEquals(intent.getData(), Uri.parse("https://www.google.com/preferences/interests/yourinterests?sh=n")); + assertNotEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_WIDTH_PX, 0)); + assertNotEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_HEIGHT_PX, 0)); + verify(mFeedServiceBridgeJniMock) + .reportOtherUserAction( + TEST_STREAM_KIND, FeedUserActionType.TAPPED_MANAGE_INTERESTS); + } + + @Test + public void testHandleHiddenClick_FlagDisabled() { + ChromeFeatureList.sCctResizableSideSheetDiscoverFeedSettings.setForTesting(false); + // Act + mFeedManagementMediator.handleHiddenClick(null); + + // Assert + Intent intent = mShadowActivity.peekNextStartedActivityForResult().intent; + assertEquals(intent.getData(), + Uri.parse("https://www.google.com/preferences/interests/hidden?sh=n")); + assertEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_WIDTH_PX, 0)); + assertEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_HEIGHT_PX, 0)); verify(mFeedServiceBridgeJniMock) .reportOtherUserAction( TEST_STREAM_KIND, FeedUserActionType.TAPPED_MANAGE_INTERESTS); } @Test - public void testHandleHiddenClick() { + public void testHandleHiddenClick_FlagEnabled() { + ChromeFeatureList.sCctResizableSideSheetDiscoverFeedSettings.setForTesting(true); // Act mFeedManagementMediator.handleHiddenClick(null); @@ -102,6 +171,10 @@ public void testHandleHiddenClick() { Intent intent = mShadowActivity.peekNextStartedActivityForResult().intent; assertEquals(intent.getData(), Uri.parse("https://www.google.com/preferences/interests/hidden?sh=n")); + assertNotEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_WIDTH_PX, 0)); + assertNotEquals( + 0, intent.getIntExtra(FeedManagementMediator.EXTRA_ACTIVITY_INITIAL_HEIGHT_PX, 0)); verify(mFeedServiceBridgeJniMock) .reportOtherUserAction( TEST_STREAM_KIND, FeedUserActionType.TAPPED_MANAGE_INTERESTS); diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json index 326fa12f177664..ac0f4f570a6cd3 100644 --- a/chrome/browser/flag-metadata.json +++ b/chrome/browser/flag-metadata.json @@ -987,6 +987,11 @@ "owners": ["kgrosu", "jinsukkim", "twellington"], "expiry_milestone": 121 }, + { + "name": "cct-resizable-side-sheet-discover-feed-settings", + "owners": ["katzz", "kgrosu", "jinsukkim"], + "expiry_milestone": 130 + }, { "name": "cct-resizable-side-sheet-for-third-parties", "owners": ["katzz", "kgrosu", "jinsukkim"], diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc index dd747bf8f0d22e..6b5722927098bd 100644 --- a/chrome/browser/flag_descriptions.cc +++ b/chrome/browser/flag_descriptions.cc @@ -3619,6 +3619,10 @@ const char kCCTResizableForThirdPartiesDescription[] = const char kCCTResizableSideSheetName[] = "Side sheet Custom Tabs"; const char kCCTResizableSideSheetDescription[] = "Enable side sheet Custom Tabs"; +const char kCCTResizableSideSheetDiscoverFeedSettingsName[] = + "Discover feed settings Partial Custom Tab"; +const char kCCTResizableSideSheetDiscoverFeedSettingsDescription[] = + "Enable discover feed settings Partial Custom Tabs"; const char kCCTResizableSideSheetForThirdPartiesName[] = "Side sheet Custom Tabs (third party)"; const char kCCTResizableSideSheetForThirdPartiesDescription[] = diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h index 441bd3e373466d..2711d7f1fbefce 100644 --- a/chrome/browser/flag_descriptions.h +++ b/chrome/browser/flag_descriptions.h @@ -2074,6 +2074,8 @@ extern const char kCCTResizableForThirdPartiesName[]; extern const char kCCTResizableForThirdPartiesDescription[]; extern const char kCCTResizableSideSheetName[]; extern const char kCCTResizableSideSheetDescription[]; +extern const char kCCTResizableSideSheetDiscoverFeedSettingsName[]; +extern const char kCCTResizableSideSheetDiscoverFeedSettingsDescription[]; extern const char kCCTResizableSideSheetForThirdPartiesName[]; extern const char kCCTResizableSideSheetForThirdPartiesDescription[]; diff --git a/chrome/browser/flags/android/chrome_feature_list.cc b/chrome/browser/flags/android/chrome_feature_list.cc index a8eae33443bfdd..e906c8c47db00a 100644 --- a/chrome/browser/flags/android/chrome_feature_list.cc +++ b/chrome/browser/flags/android/chrome_feature_list.cc @@ -201,6 +201,7 @@ const base::Feature* const kFeaturesExposedToJava[] = { &kCCTResizable90MaximumHeight, &kCCTResizableForThirdParties, &kCCTResizableSideSheet, + &kCCTResizableSideSheetDiscoverFeedSettings, &kCCTResizableSideSheetForThirdParties, &kCCTRetainingStateInMemory, &kCCTResourcePrefetch, @@ -608,6 +609,10 @@ BASE_FEATURE(kCCTResizableSideSheet, "CCTResizableSideSheet", base::FEATURE_DISABLED_BY_DEFAULT); +BASE_FEATURE(kCCTResizableSideSheetDiscoverFeedSettings, + "CCTResizableSideSheetDiscoverFeedSettings", + base::FEATURE_DISABLED_BY_DEFAULT); + BASE_FEATURE(kCCTResizableSideSheetForThirdParties, "CCTResizableSideSheetForThirdParties", base::FEATURE_DISABLED_BY_DEFAULT); diff --git a/chrome/browser/flags/android/chrome_feature_list.h b/chrome/browser/flags/android/chrome_feature_list.h index 0215909f3b477c..82e4cdea740169 100644 --- a/chrome/browser/flags/android/chrome_feature_list.h +++ b/chrome/browser/flags/android/chrome_feature_list.h @@ -59,6 +59,7 @@ BASE_DECLARE_FEATURE(kCCTReportParallelRequestStatus); BASE_DECLARE_FEATURE(kCCTResizable90MaximumHeight); BASE_DECLARE_FEATURE(kCCTResizableForThirdParties); BASE_DECLARE_FEATURE(kCCTResizableSideSheet); +BASE_DECLARE_FEATURE(kCCTResizableSideSheetDiscoverFeedSettings); BASE_DECLARE_FEATURE(kCCTResizableSideSheetForThirdParties); BASE_DECLARE_FEATURE(kCCTResourcePrefetch); BASE_DECLARE_FEATURE(kCCTRetainingStateInMemory); diff --git a/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java b/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java index 23102c197b82c1..514f2adb6428d3 100644 --- a/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java +++ b/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java @@ -248,6 +248,8 @@ public static boolean getFieldTrialParamByFeatureAsBoolean( "CCTResizableAllowResizeByUserGesture"; public static final String CCT_RESIZABLE_FOR_THIRD_PARTIES = "CCTResizableForThirdParties"; public static final String CCT_RESIZABLE_SIDE_SHEET = "CCTResizableSideSheet"; + public static final String CCT_RESIZABLE_SIDE_SHEET_DISCOVER_FEED_SETTINGS = + "CCTResizableSideSheetDiscoverFeedSettings"; public static final String CCT_RESIZABLE_SIDE_SHEET_FOR_THIRD_PARTIES = "CCTResizableSideSheetForThirdParties"; public static final String CCT_RESOURCE_PREFETCH = "CCTResourcePrefetch"; @@ -602,6 +604,8 @@ public static boolean getFieldTrialParamByFeatureAsBoolean( new CachedFlag(CCT_RESIZABLE_FOR_THIRD_PARTIES, true); public static final CachedFlag sCctResizableSideSheet = new CachedFlag(CCT_RESIZABLE_SIDE_SHEET, false); + public static final CachedFlag sCctResizableSideSheetDiscoverFeedSettings = + new CachedFlag(CCT_RESIZABLE_SIDE_SHEET_DISCOVER_FEED_SETTINGS, false); public static final CachedFlag sCctResizableSideSheetForThirdParties = new CachedFlag(CCT_RESIZABLE_SIDE_SHEET_FOR_THIRD_PARTIES, false); public static final CachedFlag sCctRetainableStateInMemory = diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index 339888f8694ef9..80fbb278f22ac2 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -64622,6 +64622,8 @@ from previous Chrome versions. + @@ -65360,6 +65362,8 @@ from previous Chrome versions. +