Commit 7e0507a6 authored by Sky Malice's avatar Sky Malice Committed by Commit Bot

[Feed] Switch to DeferredStartupHandler.

Bug: 968213
Change-Id: I5270ccb000096f0517cb7c15ff14055a1bfb8471
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1649176Reviewed-by: default avatarTheresa <twellington@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667271}
parent 75a4a11a
...@@ -11,14 +11,11 @@ import com.google.android.libraries.feed.api.client.lifecycle.AppLifecycleListen ...@@ -11,14 +11,11 @@ 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.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.DeferredStartupHandler;
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;
...@@ -52,31 +49,11 @@ public class FeedAppLifecycle ...@@ -52,31 +49,11 @@ 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; 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
* FeedAppLifecycleFactory}. * FeedAppLifecycleFactory}.
...@@ -88,17 +65,9 @@ public class FeedAppLifecycle ...@@ -88,17 +65,9 @@ public class FeedAppLifecycle
*/ */
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()) {
...@@ -163,7 +132,6 @@ public class FeedAppLifecycle ...@@ -163,7 +132,6 @@ public class FeedAppLifecycle
mLifecycleBridge = null; mLifecycleBridge = null;
mAppLifecycleListener = null; mAppLifecycleListener = null;
mFeedScheduler = null; mFeedScheduler = null;
mTaskDelegate = null;
} }
@Override @Override
...@@ -209,18 +177,17 @@ public class FeedAppLifecycle ...@@ -209,18 +177,17 @@ public class FeedAppLifecycle
if (!mDelayedInitializeStarted) { if (!mDelayedInitializeStarted) {
mDelayedInitializeStarted = true; mDelayedInitializeStarted = true;
int disableByDefault = -1; boolean initFeed = ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
int delayMs = ChromeFeatureList.getFieldTrialParamByFeatureAsInt( ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS, "init_feed_after_startup",
ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS, "init_feed_after_delay_ms", false);
disableByDefault); if (initFeed) {
if (delayMs >= 0) { DeferredStartupHandler.getInstance().addDeferredTask(() -> {
mTaskDelegate.postDelayedTask(UiThreadTaskTraits.BEST_EFFORT, () -> {
// Since this is being run asynchronously, it's possible #destroy() is called // Since this is being run asynchronously, it's possible #destroy() is called
// before the delay finishes. Must guard against this. // before the delay finishes. Must guard against this.
if (mLifecycleBridge != null) { if (mLifecycleBridge != null) {
initialize(); initialize();
} }
}, delayMs); });
} }
} }
} }
......
...@@ -5,11 +5,7 @@ ...@@ -5,11 +5,7 @@
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.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;
...@@ -20,13 +16,12 @@ import android.support.test.filters.SmallTest; ...@@ -20,13 +16,12 @@ import android.support.test.filters.SmallTest;
import com.google.android.libraries.feed.api.client.lifecycle.AppLifecycleListener; import com.google.android.libraries.feed.api.client.lifecycle.AppLifecycleListener;
import com.google.android.libraries.feed.api.host.network.NetworkClient; import com.google.android.libraries.feed.api.host.network.NetworkClient;
import org.junit.After;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; 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;
...@@ -41,6 +36,7 @@ import org.chromium.base.test.util.Feature; ...@@ -41,6 +36,7 @@ import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches; import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.DeferredStartupHandler;
import org.chromium.chrome.browser.UrlConstants; import org.chromium.chrome.browser.UrlConstants;
import org.chromium.chrome.browser.feed.FeedAppLifecycle.AppLifecycleEvent; import org.chromium.chrome.browser.feed.FeedAppLifecycle.AppLifecycleEvent;
import org.chromium.chrome.browser.init.ChromeBrowserInitializer; import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
...@@ -56,6 +52,8 @@ import org.chromium.content_public.browser.LoadUrlParams; ...@@ -56,6 +52,8 @@ 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.ArrayList;
import java.util.List;
import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeoutException;
/** /**
...@@ -75,11 +73,9 @@ public class FeedAppLifecycleTest { ...@@ -75,11 +73,9 @@ public class FeedAppLifecycleTest {
private FeedOfflineIndicator mOfflineIndicator; private FeedOfflineIndicator mOfflineIndicator;
@Mock @Mock
private AppLifecycleListener mAppLifecycleListener; private AppLifecycleListener mAppLifecycleListener;
@Mock
private FeedAppLifecycle.TaskDelegate mTestDelegate;
@Captor private TestDeferredStartupHandler mTestDeferredStartupHandler =
ArgumentCaptor<Runnable> mRunnableCaptor; new TestDeferredStartupHandler();
private ChromeTabbedActivity mActivity; private ChromeTabbedActivity mActivity;
private FeedAppLifecycle mAppLifecycle; private FeedAppLifecycle mAppLifecycle;
...@@ -87,9 +83,27 @@ public class FeedAppLifecycleTest { ...@@ -87,9 +83,27 @@ public class FeedAppLifecycleTest {
private final String mHistogramAppLifecycleEvents = private final String mHistogramAppLifecycleEvents =
"ContentSuggestions.Feed.AppLifecycle.Events"; "ContentSuggestions.Feed.AppLifecycle.Events";
private static class TestDeferredStartupHandler extends DeferredStartupHandler {
private List<Runnable> mDeferredTaskQueue = new ArrayList<>();
@Override
public void addDeferredTask(Runnable deferredTask) {
mDeferredTaskQueue.add(deferredTask);
}
public void runAllTasks() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
for (Runnable deferredTask : mDeferredTaskQueue) {
deferredTask.run();
}
});
}
}
@Before @Before
public void setUp() throws InterruptedException { public void setUp() throws InterruptedException {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
DeferredStartupHandler.setInstanceForTests(mTestDeferredStartupHandler);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
try { try {
ChromeBrowserInitializer.getInstance().handleSynchronousStartup(); ChromeBrowserInitializer.getInstance().handleSynchronousStartup();
...@@ -98,8 +112,8 @@ public class FeedAppLifecycleTest { ...@@ -98,8 +112,8 @@ public class FeedAppLifecycleTest {
} }
Profile profile = Profile.getLastUsedProfile().getOriginalProfile(); Profile profile = Profile.getLastUsedProfile().getOriginalProfile();
mLifecycleBridge = new FeedLifecycleBridge(profile); mLifecycleBridge = new FeedLifecycleBridge(profile);
mAppLifecycle = new FeedAppLifecycle( mAppLifecycle =
mAppLifecycleListener, mLifecycleBridge, mFeedScheduler, mTestDelegate); new FeedAppLifecycle(mAppLifecycleListener, mLifecycleBridge, mFeedScheduler);
FeedProcessScopeFactory.createFeedProcessScopeForTesting(mFeedScheduler, mNetworkClient, FeedProcessScopeFactory.createFeedProcessScopeForTesting(mFeedScheduler, mNetworkClient,
mOfflineIndicator, mAppLifecycle, mOfflineIndicator, mAppLifecycle,
new FeedLoggingBridge(profile)); new FeedLoggingBridge(profile));
...@@ -109,6 +123,11 @@ public class FeedAppLifecycleTest { ...@@ -109,6 +123,11 @@ public class FeedAppLifecycleTest {
mActivity = mActivityTestRule.getActivity(); mActivity = mActivityTestRule.getActivity();
} }
@After
public void tearDown() {
DeferredStartupHandler.setInstanceForTests(null);
}
@Test @Test
@SmallTest @SmallTest
@Feature({"Feed"}) @Feature({"Feed"})
...@@ -305,8 +324,8 @@ public class FeedAppLifecycleTest { ...@@ -305,8 +324,8 @@ public class FeedAppLifecycleTest {
@Feature({"Feed"}) @Feature({"Feed"})
public void testDelayedInitNoParam() { public void testDelayedInitNoParam() {
verify(mAppLifecycleListener, times(1)).onEnterForeground(); verify(mAppLifecycleListener, times(1)).onEnterForeground();
mTestDeferredStartupHandler.runAllTasks();
verify(mAppLifecycleListener, times(0)).initialize(); verify(mAppLifecycleListener, times(0)).initialize();
verify(mTestDelegate, never()).postDelayedTask(any(), any(), anyLong());
} }
@Test @Test
...@@ -314,15 +333,12 @@ public class FeedAppLifecycleTest { ...@@ -314,15 +333,12 @@ public class FeedAppLifecycleTest {
@Feature({"Feed"}) @Feature({"Feed"})
@CommandLineFlags. @CommandLineFlags.
Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group", Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group",
"force-fieldtrial-params=Trial.Group:init_feed_after_delay_ms/99"}) "force-fieldtrial-params=Trial.Group:init_feed_after_startup/true"})
public void public void
testDelayedInitWithParam() { testDelayedInitWithParamTrue() {
verify(mAppLifecycleListener, times(1)).onEnterForeground(); verify(mAppLifecycleListener, times(1)).onEnterForeground();
verify(mAppLifecycleListener, times(0)).initialize(); verify(mAppLifecycleListener, times(0)).initialize();
verify(mTestDelegate, times(1)) mTestDeferredStartupHandler.runAllTasks();
.postDelayedTask(
eq(UiThreadTaskTraits.BEST_EFFORT), mRunnableCaptor.capture(), eq(99L));
mRunnableCaptor.getValue().run();
verify(mAppLifecycleListener, times(1)).initialize(); verify(mAppLifecycleListener, times(1)).initialize();
} }
...@@ -331,18 +347,12 @@ public class FeedAppLifecycleTest { ...@@ -331,18 +347,12 @@ public class FeedAppLifecycleTest {
@Feature({"Feed"}) @Feature({"Feed"})
@CommandLineFlags. @CommandLineFlags.
Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group", Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group",
"force-fieldtrial-params=Trial.Group:init_feed_after_delay_ms/0"}) "force-fieldtrial-params=Trial.Group:init_feed_after_startup/false"})
public void public void
testDelayedInitZeroParam() { testDelayedInitZeroParamFalse() {
verify(mAppLifecycleListener, times(1)).onEnterForeground(); verify(mAppLifecycleListener, times(1)).onEnterForeground();
// While the real implementation will likely synchronously invoke the callback when given a mTestDeferredStartupHandler.runAllTasks();
// delay of 0, our mocks have no logic in them.
verify(mAppLifecycleListener, times(0)).initialize(); 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 @Test
...@@ -350,20 +360,12 @@ public class FeedAppLifecycleTest { ...@@ -350,20 +360,12 @@ public class FeedAppLifecycleTest {
@Feature({"Feed"}) @Feature({"Feed"})
@CommandLineFlags. @CommandLineFlags.
Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group", Add({"enable-features=InterestFeedContentSuggestions<Trial", "force-fieldtrials=Trial/Group",
"force-fieldtrial-params=Trial.Group:init_feed_after_delay_ms/99"}) "force-fieldtrial-params=Trial.Group:init_feed_after_startup/notboolean"})
public void public void
testDelayedInitWithDestroy() { testDelayedInitZeroParamNotBoolean() {
verify(mAppLifecycleListener, times(1)).onEnterForeground(); verify(mAppLifecycleListener, times(1)).onEnterForeground();
mTestDeferredStartupHandler.runAllTasks();
verify(mAppLifecycleListener, times(0)).initialize(); 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)
......
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