Commit c41c5c46 authored by Haiyang Pan's avatar Haiyang Pan Committed by Commit Bot

Revert "[HomepagePromo] Add finch param supressing_sign_in_promo"

This reverts commit 25269f24.

Reason for revert: test testSetUp_SuppressingSignInPromo fails constantly in android-arm64-proguard-rel (and fyi builder android-marshmallow-x86-fyi-rel) since
* https://ci.chromium.org/p/chromium/builders/ci/android-arm64-proguard-rel/2020
* https://ci.chromium.org/p/chromium/builders/ci/android-marshmallow-x86-fyi-rel/767

Original change's description:
> [HomepagePromo] Add finch param supressing_sign_in_promo
> 
> Adding the option for homepage promo to suppress the sign in promo when
> user is not signed in.
> 
> Bug: 1068831
> Change-Id: I854109c4623c47f98aab1faa34f94f2d55961474
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252379
> Commit-Queue: Wenyu Fu <wenyufu@chromium.org>
> Reviewed-by: Theresa  <twellington@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#781109}

TBR=twellington@chromium.org,wenyufu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1068831
Change-Id: I97f226e93962469cc8a705e922a3db72d7e84cd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2265398Reviewed-by: default avatarHaiyang Pan <hypan@google.com>
Reviewed-by: default avatarWenyu Fu <wenyufu@chromium.org>
Commit-Queue: Haiyang Pan <hypan@google.com>
Cr-Commit-Position: refs/heads/master@{#782189}
parent 613c9d5c
......@@ -39,7 +39,6 @@ import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
import org.chromium.chrome.browser.ntp.NewTabPageLayout;
import org.chromium.chrome.browser.ntp.SnapScrollHelper;
import org.chromium.chrome.browser.ntp.cards.promo.HomepagePromoController;
import org.chromium.chrome.browser.ntp.cards.promo.HomepagePromoVariationManager;
import org.chromium.chrome.browser.ntp.snippets.SectionHeaderView;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.signin.PersonalizedSigninPromoView;
......@@ -450,16 +449,12 @@ public class FeedSurfaceCoordinator implements FeedSurfaceProvider {
headers.add(new NonDismissibleHeader(mNtpHeader));
}
// TODO(wenyufu): Consider moving priority logic into Mediator
if (mHomepagePromoController != null) {
if (!isSignInPromoVisible
|| HomepagePromoVariationManager.getInstance().isSuppressingSignInPromo()) {
View promoView = mHomepagePromoController.getPromoView();
if (promoView != null) {
mHomepagePromoView = promoView;
headers.add(new HomepagePromoHeader());
isSignInPromoVisible = false;
}
// TODO(wenyufu): check Finch flag for whether sign-in takes precedence over homepage promo
if (!isSignInPromoVisible && mHomepagePromoController != null) {
View promoView = mHomepagePromoController.getPromoView();
if (promoView != null) {
mHomepagePromoView = promoView;
headers.add(new HomepagePromoHeader());
}
}
......
......@@ -122,14 +122,6 @@ public class HomepagePromoVariationManager {
return LayoutStyle.COMPACT;
}
/**
* @return Whether the homepage promo has a higher priority than SignInPromo.
*/
public boolean isSuppressingSignInPromo() {
return ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
ChromeFeatureList.HOMEPAGE_PROMO_CARD, "suppressing_sign_in_promo", false);
}
/**
* If the user has ever seen the homepage promo, add a synthetic tag to the user.
*/
......
......@@ -216,37 +216,6 @@ public class HomepagePromoTest {
METRICS_HOMEPAGE_PROMO, HomepagePromoAction.CREATED));
}
@Test
@SmallTest
@DisableIf.Build(sdk_is_greater_than = VERSION_CODES.O, message = "crbug.com/1097179")
public void testSetUp_SuppressingSignInPromo() {
Mockito.doReturn(true).when(mMockVariationManager).isSuppressingSignInPromo();
mActivityTestRule.loadUrl(UrlConstants.NTP_URL);
Assert.assertTrue("Homepage Promo should match creation criteria.",
HomepagePromoUtils.shouldCreatePromo(null));
View homepagePromo = mActivityTestRule.getActivity().findViewById(R.id.homepage_promo);
Assert.assertNotNull("Homepage promo should be added to NTP.", homepagePromo);
Assert.assertEquals(
"Homepage promo should be visible.", View.VISIBLE, homepagePromo.getVisibility());
Assert.assertEquals("Promo created should be recorded once.", 1,
RecordHistogram.getHistogramValueCountForTesting(
METRICS_HOMEPAGE_PROMO, HomepagePromoAction.CREATED));
// Dismiss the HomepagePromo in shared preference, then load another NTP page.
HomepagePromoUtils.setPromoDismissedInSharedPreference(true);
mActivityTestRule.loadUrlInNewTab(UrlConstants.NTP_URL);
Assert.assertNotNull("SignInPromo should be displayed on the screen.",
mActivityTestRule.getActivity().findViewById(R.id.signin_promo_view_container));
Assert.assertFalse("Homepage Promo should not match creation criteria.",
HomepagePromoUtils.shouldCreatePromo(null));
Assert.assertNull("Homepage promo should not show on the screen.",
mActivityTestRule.getActivity().findViewById(R.id.homepage_promo));
}
/**
* Clicking on dismiss button should hide the promo.
*/
......
......@@ -1472,8 +1472,6 @@ const FeatureEntry::FeatureParam kHomepagePromoCardCompact[] = {
{"promo-card-variation", "Compact"}};
const FeatureEntry::FeatureParam kHomepagePromoCardSlim[] = {
{"promo-card-variation", "Slim"}};
const FeatureEntry::FeatureParam kHomepagePromoCardSupressing[] = {
{"suppressing_sign_in_promo", "SuppressingSignInPromo"}};
const FeatureEntry::FeatureVariation kHomepagePromoCardVariations[] = {
{"Large", kHomepagePromoCardLarge, base::size(kHomepagePromoCardLarge),
......@@ -1481,9 +1479,7 @@ const FeatureEntry::FeatureVariation kHomepagePromoCardVariations[] = {
{"Compact", kHomepagePromoCardCompact,
base::size(kHomepagePromoCardCompact), nullptr},
{"Slim", kHomepagePromoCardSlim, base::size(kHomepagePromoCardSlim),
nullptr},
{"Compact_SuppressingSignInPromo", kHomepagePromoCardSupressing,
base::size(kHomepagePromoCardSupressing), nullptr}};
nullptr}};
#endif // defined(OS_ANDROID)
#if defined(OS_ANDROID)
......
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