Commit 88a133da authored by Sky Malice's avatar Sky Malice Committed by Commit Bot

[Feed] Initialize Feed after parameterized delay.

Bug: 968611
Change-Id: I1816950bef9db9b6b85203035cf62fc708f7e467
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1644012
Commit-Queue: Sky Malice <skym@chromium.org>
Reviewed-by: default avatarDan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666492}
parent 52a35247
...@@ -11,9 +11,14 @@ import com.google.android.libraries.feed.api.client.lifecycle.AppLifecycleListen ...@@ -11,9 +11,14 @@ import com.google.android.libraries.feed.api.client.lifecycle.AppLifecycleListen
import org.chromium.base.ActivityState; import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus; import org.chromium.base.ApplicationStatus;
import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.task.PostTask;
import org.chromium.base.task.TaskTraits;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.signin.SigninManager; import org.chromium.chrome.browser.signin.SigninManager;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
...@@ -47,9 +52,30 @@ public class FeedAppLifecycle ...@@ -47,9 +52,30 @@ public class FeedAppLifecycle
private AppLifecycleListener mAppLifecycleListener; private AppLifecycleListener mAppLifecycleListener;
private FeedLifecycleBridge mLifecycleBridge; private FeedLifecycleBridge mLifecycleBridge;
private FeedScheduler mFeedScheduler; private FeedScheduler mFeedScheduler;
private TaskDelegate mTaskDelegate;
private int mTabbedActivityCount; private int mTabbedActivityCount;
private boolean mInitializeCalled; private boolean mInitializeCalled;
private boolean mDelayedInitializeStarted;
/** Abstraction for posting delayed tasks. */
interface TaskDelegate {
/**
* @param taskTraits The TaskTraits that describe the desired TaskRunner.
* @param task The task to be run with the specified traits.
* @param delay The delay in milliseconds before the task can be run.
*/
void postDelayedTask(TaskTraits taskTraits, Runnable task, long delay);
}
/** The implementation used at runtime that calls into {@link PostTask}. */
@VisibleForTesting
static class DefaultTaskDelegate implements TaskDelegate {
@Override
public void postDelayedTask(TaskTraits taskTraits, Runnable task, long delay) {
PostTask.postDelayedTask(taskTraits, task, delay);
}
}
/** /**
* Create a FeedAppLifecycle instance. In normal use, this should only be called by {@link * Create a FeedAppLifecycle instance. In normal use, this should only be called by {@link
...@@ -58,12 +84,21 @@ public class FeedAppLifecycle ...@@ -58,12 +84,21 @@ public class FeedAppLifecycle
* interface that we will call into. * interface that we will call into.
* @param lifecycleBridge FeedLifecycleBridge JNI bridge over which native lifecycle events are * @param lifecycleBridge FeedLifecycleBridge JNI bridge over which native lifecycle events are
* delivered. * delivered.
* @param feedScheduler Scheduler to be notified of several events.
*/ */
public FeedAppLifecycle(AppLifecycleListener appLifecycleListener, public FeedAppLifecycle(AppLifecycleListener appLifecycleListener,
FeedLifecycleBridge lifecycleBridge, FeedScheduler feedScheduler) { FeedLifecycleBridge lifecycleBridge, FeedScheduler feedScheduler) {
this(appLifecycleListener, lifecycleBridge, feedScheduler, new DefaultTaskDelegate());
}
/** Package private constructor used directly by tests to inject a mock TaskDelegate. */
@VisibleForTesting
FeedAppLifecycle(AppLifecycleListener appLifecycleListener, FeedLifecycleBridge lifecycleBridge,
FeedScheduler feedScheduler, TaskDelegate taskDelegate) {
mAppLifecycleListener = appLifecycleListener; mAppLifecycleListener = appLifecycleListener;
mLifecycleBridge = lifecycleBridge; mLifecycleBridge = lifecycleBridge;
mFeedScheduler = feedScheduler; mFeedScheduler = feedScheduler;
mTaskDelegate = taskDelegate;
int resumedActivityCount = 0; int resumedActivityCount = 0;
for (Activity activity : ApplicationStatus.getRunningActivities()) { for (Activity activity : ApplicationStatus.getRunningActivities()) {
...@@ -128,6 +163,7 @@ public class FeedAppLifecycle ...@@ -128,6 +163,7 @@ public class FeedAppLifecycle
mLifecycleBridge = null; mLifecycleBridge = null;
mAppLifecycleListener = null; mAppLifecycleListener = null;
mFeedScheduler = null; mFeedScheduler = null;
mTaskDelegate = null;
} }
@Override @Override
...@@ -170,6 +206,23 @@ public class FeedAppLifecycle ...@@ -170,6 +206,23 @@ public class FeedAppLifecycle
private void onEnterForeground() { private void onEnterForeground() {
reportEvent(AppLifecycleEvent.ENTER_FOREGROUND); reportEvent(AppLifecycleEvent.ENTER_FOREGROUND);
mAppLifecycleListener.onEnterForeground(); mAppLifecycleListener.onEnterForeground();
if (!mDelayedInitializeStarted) {
mDelayedInitializeStarted = true;
int disableByDefault = -1;
int delayMs = ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS, "init_feed_after_delay_ms",
disableByDefault);
if (delayMs >= 0) {
mTaskDelegate.postDelayedTask(UiThreadTaskTraits.BEST_EFFORT, () -> {
// Since this is being run asynchronously, it's possible #destroy() is called
// before the delay finishes. Must guard against this.
if (mLifecycleBridge != null) {
initialize();
}
}, delayMs);
}
}
} }
private void onEnterBackground() { private void onEnterBackground() {
......
...@@ -5,8 +5,11 @@ ...@@ -5,8 +5,11 @@
package org.chromium.chrome.browser.feed; package org.chromium.chrome.browser.feed;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.anyString; import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
...@@ -22,6 +25,8 @@ import org.junit.Before; ...@@ -22,6 +25,8 @@ import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
...@@ -51,7 +56,6 @@ import org.chromium.content_public.browser.LoadUrlParams; ...@@ -51,7 +56,6 @@ import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.UiThreadTaskTraits; import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.content_public.browser.test.util.TestThreadUtils; import org.chromium.content_public.browser.test.util.TestThreadUtils;
import java.util.Map;
import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeoutException;
/** /**
...@@ -59,10 +63,10 @@ import java.util.concurrent.TimeoutException; ...@@ -59,10 +63,10 @@ import java.util.concurrent.TimeoutException;
*/ */
@RunWith(ChromeJUnit4ClassRunner.class) @RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add(ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE) @CommandLineFlags.Add(ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE)
@EnableFeatures({ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS})
public class FeedAppLifecycleTest { public class FeedAppLifecycleTest {
@Rule @Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule(); public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
@Mock @Mock
private FeedScheduler mFeedScheduler; private FeedScheduler mFeedScheduler;
@Mock @Mock
...@@ -72,7 +76,11 @@ public class FeedAppLifecycleTest { ...@@ -72,7 +76,11 @@ public class FeedAppLifecycleTest {
@Mock @Mock
private AppLifecycleListener mAppLifecycleListener; private AppLifecycleListener mAppLifecycleListener;
@Mock @Mock
private Map<String, Boolean> mMockFeatureList; private FeedAppLifecycle.TaskDelegate mTestDelegate;
@Captor
ArgumentCaptor<Runnable> mRunnableCaptor;
private ChromeTabbedActivity mActivity; private ChromeTabbedActivity mActivity;
private FeedAppLifecycle mAppLifecycle; private FeedAppLifecycle mAppLifecycle;
private FeedLifecycleBridge mLifecycleBridge; private FeedLifecycleBridge mLifecycleBridge;
...@@ -82,8 +90,6 @@ public class FeedAppLifecycleTest { ...@@ -82,8 +90,6 @@ public class FeedAppLifecycleTest {
@Before @Before
public void setUp() throws InterruptedException { public void setUp() throws InterruptedException {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
when(mMockFeatureList.get(anyString())).thenReturn(true);
ChromeFeatureList.setTestFeatures(mMockFeatureList);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
try { try {
ChromeBrowserInitializer.getInstance().handleSynchronousStartup(); ChromeBrowserInitializer.getInstance().handleSynchronousStartup();
...@@ -92,8 +98,8 @@ public class FeedAppLifecycleTest { ...@@ -92,8 +98,8 @@ public class FeedAppLifecycleTest {
} }
Profile profile = Profile.getLastUsedProfile().getOriginalProfile(); Profile profile = Profile.getLastUsedProfile().getOriginalProfile();
mLifecycleBridge = new FeedLifecycleBridge(profile); mLifecycleBridge = new FeedLifecycleBridge(profile);
mAppLifecycle = mAppLifecycle = new FeedAppLifecycle(
new FeedAppLifecycle(mAppLifecycleListener, mLifecycleBridge, mFeedScheduler); mAppLifecycleListener, mLifecycleBridge, mFeedScheduler, mTestDelegate);
FeedProcessScopeFactory.createFeedProcessScopeForTesting(mFeedScheduler, mNetworkClient, FeedProcessScopeFactory.createFeedProcessScopeForTesting(mFeedScheduler, mNetworkClient,
mOfflineIndicator, mAppLifecycle, mOfflineIndicator, mAppLifecycle,
new FeedLoggingBridge(profile)); new FeedLoggingBridge(profile));
...@@ -105,14 +111,14 @@ public class FeedAppLifecycleTest { ...@@ -105,14 +111,14 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testConstructionChecksActiveTabbedActivities() { public void testConstructionChecksActiveTabbedActivities() {
verify(mAppLifecycleListener, times(1)).onEnterForeground(); verify(mAppLifecycleListener, times(1)).onEnterForeground();
} }
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testActivityStateChangesIncrementStateCounters() public void testActivityStateChangesIncrementStateCounters()
throws InterruptedException, TimeoutException { throws InterruptedException, TimeoutException {
verifyHistogram(mHistogramAppLifecycleEvents, AppLifecycleEvent.ENTER_BACKGROUND, 0); verifyHistogram(mHistogramAppLifecycleEvents, AppLifecycleEvent.ENTER_BACKGROUND, 0);
...@@ -128,7 +134,8 @@ public class FeedAppLifecycleTest { ...@@ -128,7 +134,8 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
@EnableFeatures({ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS})
public void testNtpOpeningTriggersInitializeOnlyOnce() throws InterruptedException { public void testNtpOpeningTriggersInitializeOnlyOnce() throws InterruptedException {
// We open to about:blank initially so we shouldn't have called initialize() yet. // We open to about:blank initially so we shouldn't have called initialize() yet.
verify(mAppLifecycleListener, times(0)).initialize(); verify(mAppLifecycleListener, times(0)).initialize();
...@@ -143,7 +150,7 @@ public class FeedAppLifecycleTest { ...@@ -143,7 +150,7 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testOnHistoryDeleted() { public void testOnHistoryDeleted() {
verify(mAppLifecycleListener, times(0)).onClearAll(); verify(mAppLifecycleListener, times(0)).onClearAll();
verify(mAppLifecycleListener, times(0)).onClearAllWithRefresh(); verify(mAppLifecycleListener, times(0)).onClearAllWithRefresh();
...@@ -167,7 +174,7 @@ public class FeedAppLifecycleTest { ...@@ -167,7 +174,7 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testOnCachedDataCleared() { public void testOnCachedDataCleared() {
verify(mAppLifecycleListener, times(0)).onClearAll(); verify(mAppLifecycleListener, times(0)).onClearAll();
verify(mAppLifecycleListener, times(0)).onClearAllWithRefresh(); verify(mAppLifecycleListener, times(0)).onClearAllWithRefresh();
...@@ -189,7 +196,7 @@ public class FeedAppLifecycleTest { ...@@ -189,7 +196,7 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testOnSignedOut() { public void testOnSignedOut() {
verify(mAppLifecycleListener, times(0)).onClearAll(); verify(mAppLifecycleListener, times(0)).onClearAll();
verify(mAppLifecycleListener, times(0)).onClearAllWithRefresh(); verify(mAppLifecycleListener, times(0)).onClearAllWithRefresh();
...@@ -211,7 +218,7 @@ public class FeedAppLifecycleTest { ...@@ -211,7 +218,7 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testOnSignedIn() { public void testOnSignedIn() {
verify(mAppLifecycleListener, times(0)).onClearAll(); verify(mAppLifecycleListener, times(0)).onClearAll();
verify(mAppLifecycleListener, times(0)).onClearAllWithRefresh(); verify(mAppLifecycleListener, times(0)).onClearAllWithRefresh();
...@@ -233,7 +240,7 @@ public class FeedAppLifecycleTest { ...@@ -233,7 +240,7 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testSecondWindowDoesNotTriggerForegroundOrBackground() public void testSecondWindowDoesNotTriggerForegroundOrBackground()
throws InterruptedException, TimeoutException { throws InterruptedException, TimeoutException {
verify(mAppLifecycleListener, times(1)).onEnterForeground(); verify(mAppLifecycleListener, times(1)).onEnterForeground();
...@@ -258,7 +265,8 @@ public class FeedAppLifecycleTest { ...@@ -258,7 +265,8 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
@EnableFeatures({ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS})
public void testMultiWindowDoesNotCauseMultipleInitialize() throws InterruptedException { public void testMultiWindowDoesNotCauseMultipleInitialize() throws InterruptedException {
mActivityTestRule.loadUrl(UrlConstants.NTP_URL); mActivityTestRule.loadUrl(UrlConstants.NTP_URL);
verify(mAppLifecycleListener, times(1)).initialize(); verify(mAppLifecycleListener, times(1)).initialize();
...@@ -272,7 +280,7 @@ public class FeedAppLifecycleTest { ...@@ -272,7 +280,7 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testResumeTriggersSchedulerForegrounded() public void testResumeTriggersSchedulerForegrounded()
throws InterruptedException, TimeoutException { throws InterruptedException, TimeoutException {
verify(mFeedScheduler, times(1)).onForegrounded(); verify(mFeedScheduler, times(1)).onForegrounded();
...@@ -282,7 +290,7 @@ public class FeedAppLifecycleTest { ...@@ -282,7 +290,7 @@ public class FeedAppLifecycleTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"InterestFeedContentSuggestions"}) @Feature({"Feed"})
public void testClearDataAfterDisablingDoesNotCrash() { public void testClearDataAfterDisablingDoesNotCrash() {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
FeedProcessScopeFactory.clearFeedProcessScopeForTesting(); FeedProcessScopeFactory.clearFeedProcessScopeForTesting();
...@@ -292,6 +300,72 @@ public class FeedAppLifecycleTest { ...@@ -292,6 +300,72 @@ public class FeedAppLifecycleTest {
}); });
} }
@Test
@SmallTest
@Feature({"Feed"})
public void testDelayedInitNoParam() {
verify(mAppLifecycleListener, times(1)).onEnterForeground();
verify(mAppLifecycleListener, times(0)).initialize();
verify(mTestDelegate, never()).postDelayedTask(any(), any(), anyLong());
}
@Test
@SmallTest
@Feature({"Feed"})
@CommandLineFlags.
Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group",
"force-fieldtrial-params=Trial.Group:init_feed_after_delay_ms/99"})
public void
testDelayedInitWithParam() {
verify(mAppLifecycleListener, times(1)).onEnterForeground();
verify(mAppLifecycleListener, times(0)).initialize();
verify(mTestDelegate, times(1))
.postDelayedTask(
eq(UiThreadTaskTraits.BEST_EFFORT), mRunnableCaptor.capture(), eq(99L));
mRunnableCaptor.getValue().run();
verify(mAppLifecycleListener, times(1)).initialize();
}
@Test
@SmallTest
@Feature({"Feed"})
@CommandLineFlags.
Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group",
"force-fieldtrial-params=Trial.Group:init_feed_after_delay_ms/0"})
public void
testDelayedInitZeroParam() {
verify(mAppLifecycleListener, times(1)).onEnterForeground();
// While the real implementation will likely synchronously invoke the callback when given a
// delay of 0, our mocks have no logic in them.
verify(mAppLifecycleListener, times(0)).initialize();
verify(mTestDelegate, times(1))
.postDelayedTask(
eq(UiThreadTaskTraits.BEST_EFFORT), mRunnableCaptor.capture(), eq(0L));
mRunnableCaptor.getValue().run();
verify(mAppLifecycleListener, times(1)).initialize();
}
@Test
@SmallTest
@Feature({"Feed"})
@CommandLineFlags.
Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group",
"force-fieldtrial-params=Trial.Group:init_feed_after_delay_ms/99"})
public void
testDelayedInitWithDestroy() {
verify(mAppLifecycleListener, times(1)).onEnterForeground();
verify(mAppLifecycleListener, times(0)).initialize();
verify(mTestDelegate, times(1))
.postDelayedTask(
eq(UiThreadTaskTraits.BEST_EFFORT), mRunnableCaptor.capture(), eq(99L));
// Must be on the UI thread, one of the dependencies checks.
TestThreadUtils.runOnUiThreadBlocking(() -> mAppLifecycle.destroy());
// Initialize shouldn't be called after we're destroyed.
mRunnableCaptor.getValue().run();
verify(mAppLifecycleListener, never()).initialize();
}
private void signalActivityStart(Activity activity) private void signalActivityStart(Activity activity)
throws InterruptedException, TimeoutException { throws InterruptedException, TimeoutException {
signalActivityState(activity, ActivityState.STARTED); signalActivityState(activity, ActivityState.STARTED);
......
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