chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java - Issue 2553013004: Remove the app banner navigation heuristic.

Unified Diff: chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java

Issue 2553013004: Remove the app banner navigation heuristic. (Closed)

Patch Set: Created 4 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« no previous file with comments | « chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java ('k') | chrome/browser/android/banners/app_banner_manager_android.cc » ('j') | chrome/browser/banners/app_banner_manager.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java

diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java

index 8c04ebb87fcb7afba65a75fc4b86a598662da12f..8fe094e1baa9ad2545416b55e3e00d9f8bb829db 100644

--- a/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java

+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java

@@ -25,11 +25,13 @@ import org.chromium.base.test.util.Feature;

import org.chromium.base.test.util.RetryOnFailure;

import org.chromium.chrome.R;

import org.chromium.chrome.browser.ShortcutHelper;

+import org.chromium.chrome.browser.engagement.SiteEngagementService;

import org.chromium.chrome.browser.infobar.AppBannerInfoBarAndroid;

import org.chromium.chrome.browser.infobar.AppBannerInfoBarDelegateAndroid;

import org.chromium.chrome.browser.infobar.InfoBar;

import org.chromium.chrome.browser.infobar.InfoBarContainer;

import org.chromium.chrome.browser.infobar.InfoBarContainer.InfoBarAnimationListener;

+import org.chromium.chrome.browser.profiles.Profile;

import org.chromium.chrome.browser.webapps.WebappDataStorage;

import org.chromium.chrome.test.ChromeTabbedActivityTestBase;

import org.chromium.chrome.test.util.browser.TabLoadObserver;

@@ -38,6 +40,7 @@ import org.chromium.content.browser.test.util.Criteria;

import org.chromium.content.browser.test.util.CriteriaHelper;

import org.chromium.content.browser.test.util.TouchCommon;

import org.chromium.net.test.EmbeddedTestServer;

+import org.chromium.ui.base.PageTransition;

import java.util.ArrayList;

import java.util.List;

@@ -193,10 +196,7 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

}

});

- // Navigations in this test are all of type ui::PAGE_TRANSITION_LINK, i.e. indirect.

- // Force indirect navigations to be worth the same as direct for testing.

- AppBannerManager.setEngagementWeights(1, 1);

+ AppBannerManager.setTotalEngagementForTesting(10);

mTestServer = EmbeddedTestServer.createAndStartServer(getInstrumentation().getContext());

mNativeAppUrl = mTestServer.getURL(NATIVE_APP_PATH);

mWebAppUrl = mTestServer.getURL(WEB_APP_PATH);

@@ -208,6 +208,16 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

super.tearDown();

}

+ private void resetEngagementForUrl(final String url, final double engagement) {

+ ThreadUtils.runOnUiThreadBlocking(new Runnable() {

+ @Override

+ public void run() {

+ SiteEngagementService.getForProfile(Profile.getLastUsedProfile())

+ .resetScoreForUrl(url, engagement);

+ }

+ });

+ }

private void waitUntilNoInfoBarsExist() throws Exception {

CriteriaHelper.pollUiThread(new Criteria() {

@Override

@@ -246,18 +256,18 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

private void runFullNativeInstallPathway(String url, String expectedReferrer) throws Exception {

// Visit a site that requests a banner.

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(url);

- waitUntilAppDetailsRetrieved(1);

- assertEquals(mDetailsDelegate.mReferrer, expectedReferrer);

+ resetEngagementForUrl(url, 0);

+ new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(url, PageTransition.TYPED);

waitUntilNoInfoBarsExist();

- // Indicate a day has passed, then revisit the page to get the banner to appear.

+ // Update engagement, then revisit the page to get the banner to appear.

+ resetEngagementForUrl(url, 10);

InfoBarContainer container = getActivity().getActivityTab().getInfoBarContainer();

final InfobarListener listener = new InfobarListener();

container.setAnimationListener(listener);

- AppBannerManager.setTimeDeltaForTesting(1);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(url);

- waitUntilAppDetailsRetrieved(2);

+ new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(url, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(1);

+ assertEquals(mDetailsDelegate.mReferrer, expectedReferrer);

waitUntilAppBannerInfoBarAppears(NATIVE_APP_TITLE);

CriteriaHelper.pollUiThread(new Criteria() {

@Override

@@ -305,8 +315,9 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

public void triggerWebAppBanner(String url, String expectedTitle, boolean installApp)

throws Exception {

// Visit the site in a new tab.

+ resetEngagementForUrl(url, 0);

loadUrlInNewTab("about:blank");

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(url);

+ new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(url, PageTransition.TYPED);

CriteriaHelper.pollUiThread(new Criteria() {

@Override

@@ -322,9 +333,9 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

final InfobarListener listener = new InfobarListener();

container.setAnimationListener(listener);

- // Indicate a day has passed, then revisit the page to show the banner.

- AppBannerManager.setTimeDeltaForTesting(1);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(url);

+ // Update engagement, then revisit the page to get the banner to appear.

+ resetEngagementForUrl(url, 10);

+ new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(url, PageTransition.TYPED);

CriteriaHelper.pollUiThread(new Criteria() {

@Override

public boolean isSatisfied() {

@@ -368,60 +379,83 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

@RetryOnFailure

public void testBannerAppearsThenDoesNotAppearAgainForWeeks() throws Exception {

// Visit a site that requests a banner.

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(1);

+ resetEngagementForUrl(mNativeAppUrl, 0);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

waitUntilNoInfoBarsExist();

- // Indicate a day has passed, then revisit the page.

- AppBannerManager.setTimeDeltaForTesting(1);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(2);

+ // Update engagement, then revisit the page.

+ resetEngagementForUrl(mNativeAppUrl, 10);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(1);

waitUntilAppBannerInfoBarAppears(NATIVE_APP_TITLE);

// Revisit the page to make the banner go away, but don't explicitly dismiss it.

// This hides the banner for two weeks.

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(3);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(2);

waitUntilNoInfoBarsExist();

// Wait a week until revisiting the page.

AppBannerManager.setTimeDeltaForTesting(7);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(4);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(3);

waitUntilNoInfoBarsExist();

AppBannerManager.setTimeDeltaForTesting(8);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(5);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(4);

waitUntilNoInfoBarsExist();

// Wait two weeks until revisiting the page, which should pop up the banner.

- AppBannerManager.setTimeDeltaForTesting(14);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(6);

- waitUntilNoInfoBarsExist();

AppBannerManager.setTimeDeltaForTesting(15);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(7);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(5);

waitUntilAppBannerInfoBarAppears(NATIVE_APP_TITLE);

}

@MediumTest

@Feature({"AppBanners"})

+ @RetryOnFailure

gone 2016/12/08 19:51:21 Don't know if new tests are supposed to automatica

dominickn 2016/12/09 05:27:27 Removed.

+ public void testBannerAppearsThenDoesNotAppearAgainForCustomTime() throws Exception {

+ AppBannerManager.setDaysAfterDismissAndIgnoreForTesting(7, 7);

+ triggerWebAppBanner(mWebAppUrl, WEB_APP_TITLE, false);

+ // Revisit the page to make the banner go away, but don't explicitly dismiss it.

+ // This hides the banner for two weeks.

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mWebAppUrl, PageTransition.TYPED);

+ waitUntilNoInfoBarsExist();

+ // Wait a week until revisiting the page. This should allow the banner.

+ AppBannerManager.setTimeDeltaForTesting(7);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mWebAppUrl, PageTransition.TYPED);

+ waitUntilAppBannerInfoBarAppears(WEB_APP_TITLE);

+ }

+ @MediumTest

+ @Feature({"AppBanners"})

public void testBlockedBannerDoesNotAppearAgainForMonths() throws Exception {

// Visit a site that requests a banner.

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(1);

+ resetEngagementForUrl(mNativeAppUrl, 0);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

waitUntilNoInfoBarsExist();

- // Indicate a day has passed, then revisit the page.

+ // Update engagement, then revisit the page.

+ resetEngagementForUrl(mNativeAppUrl, 10);

InfoBarContainer container = getActivity().getActivityTab().getInfoBarContainer();

final InfobarListener listener = new InfobarListener();

container.setAnimationListener(listener);

- AppBannerManager.setTimeDeltaForTesting(1);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(2);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(1);

waitUntilAppBannerInfoBarAppears(NATIVE_APP_TITLE);

// Explicitly dismiss the banner.

@@ -438,25 +472,56 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

// Waiting two months shouldn't be long enough.

AppBannerManager.setTimeDeltaForTesting(61);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(3);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(2);

waitUntilNoInfoBarsExist();

AppBannerManager.setTimeDeltaForTesting(62);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(4);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(3);

waitUntilNoInfoBarsExist();

// Waiting three months should allow banners to reappear.

AppBannerManager.setTimeDeltaForTesting(91);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(5);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

+ waitUntilAppDetailsRetrieved(4);

+ waitUntilAppBannerInfoBarAppears(NATIVE_APP_TITLE);

+ }

+ @MediumTest

+ @Feature({"AppBanners"})

+ public void testBlockedBannerDoesNotAppearAgainForCustomTime() throws Exception {

+ AppBannerManager.setDaysAfterDismissAndIgnoreForTesting(7, 7);

+ // Update engagement, then visit a page which triggers a banner.

+ resetEngagementForUrl(mWebAppUrl, 10);

+ InfoBarContainer container = getActivity().getActivityTab().getInfoBarContainer();

+ final InfobarListener listener = new InfobarListener();

+ container.setAnimationListener(listener);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mWebAppUrl, PageTransition.TYPED);

+ waitUntilAppBannerInfoBarAppears(WEB_APP_TITLE);

+ // Explicitly dismiss the banner.

+ CriteriaHelper.pollUiThread(new Criteria() {

+ @Override

+ public boolean isSatisfied() {

+ return listener.mDoneAnimating;

+ }

+ });

+ ArrayList<InfoBar> infobars = container.getInfoBarsForTesting();

+ View close = infobars.get(0).getView().findViewById(R.id.infobar_close_button);

+ TouchCommon.singleClickView(close);

waitUntilNoInfoBarsExist();

- AppBannerManager.setTimeDeltaForTesting(92);

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

- waitUntilAppDetailsRetrieved(6);

- waitUntilAppBannerInfoBarAppears(NATIVE_APP_TITLE);

+ // Waiting seven days should be long enough.

+ AppBannerManager.setTimeDeltaForTesting(7);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mWebAppUrl, PageTransition.TYPED);

+ waitUntilAppBannerInfoBarAppears(WEB_APP_TITLE);

}

@MediumTest

@@ -464,17 +529,18 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

@RetryOnFailure

public void testBitmapFetchersCanOverlapWithoutCrashing() throws Exception {

// Visit a site that requests a banner rapidly and repeatedly.

+ resetEngagementForUrl(mNativeAppUrl, 10);

for (int i = 1; i <= 10; i++) {

- new TabLoadObserver(getActivity().getActivityTab()).fullyLoadUrl(mNativeAppUrl);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mNativeAppUrl, PageTransition.TYPED);

final Integer iteration = Integer.valueOf(i);

- CriteriaHelper.pollUiThread(

- Criteria.equals(iteration, new Callable<Integer>() {

- @Override

- public Integer call() {

- return mDetailsDelegate.mNumRetrieved;

- }

- }));

+ CriteriaHelper.pollUiThread(Criteria.equals(iteration, new Callable<Integer>() {

+ @Override

+ public Integer call() {

+ return mDetailsDelegate.mNumRetrieved;

+ }

+ }));

}

@@ -488,6 +554,19 @@ public class AppBannerManagerTest extends ChromeTabbedActivityTestBase {

@SmallTest

@Feature({"AppBanners"})

@RetryOnFailure

+ public void testWebAppBannerDoesNotAppearAfterInstall() throws Exception {

+ triggerWebAppBanner(mWebAppUrl, WEB_APP_TITLE, true);

+ // The banner should not reshow after the site has been installed.

+ AppBannerManager.setTimeDeltaForTesting(100);

+ new TabLoadObserver(getActivity().getActivityTab())

+ .fullyLoadUrl(mWebAppUrl, PageTransition.TYPED);

+ waitUntilNoInfoBarsExist();

+ }

+ @SmallTest

+ @Feature({"AppBanners"})

+ @RetryOnFailure

public void testBannerFallsBackToShortName() throws Exception {

triggerWebAppBanner(WebappTestPage.urlOfPageWithServiceWorkerAndManifest(

mTestServer, WEB_APP_SHORT_TITLE_MANIFEST),