Commit 287b998d authored by Michael Thiessen's avatar Michael Thiessen Committed by Chromium LUCI CQ

Fix flaky DeferredStartupHandler test waiting

We can get into a situation where an Activity is finished before
Deferred startup finishes, then the old DeferredStartupHandler finishes
during startup of the next test and a new instance is created before
we wait for deferred startup completion. The solution is just to hold
onto the DeferredStartupHandler instance in tests during startup to
ensure a new instance isn't created.

The test that's actually flaky under these circumstances is
TabbedActivityLaunchCauseMetricsTest#testMainIntentMetrics

Change-Id: I4e172f77c3ebad45f5b3236b68614b2b152c718c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2643030Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846279}
parent 93a5fc58
...@@ -91,9 +91,7 @@ import org.chromium.base.test.util.Feature; ...@@ -91,9 +91,7 @@ import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.FlakyTest; import org.chromium.base.test.util.FlakyTest;
import org.chromium.base.test.util.MinAndroidSdkLevel; import org.chromium.base.test.util.MinAndroidSdkLevel;
import org.chromium.base.test.util.Restriction; import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.ScalableTimeout;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.DeferredStartupHandler;
import org.chromium.chrome.browser.compositor.layouts.Layout; import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.compositor.layouts.StaticLayout; import org.chromium.chrome.browser.compositor.layouts.StaticLayout;
import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager; import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
...@@ -1933,9 +1931,8 @@ public class StartSurfaceLayoutTest { ...@@ -1933,9 +1931,8 @@ public class StartSurfaceLayoutTest {
AtomicBoolean tabModelRestoreCompleted = new AtomicBoolean(false); AtomicBoolean tabModelRestoreCompleted = new AtomicBoolean(false);
AtomicBoolean normalModelIsEmpty = new AtomicBoolean(true); AtomicBoolean normalModelIsEmpty = new AtomicBoolean(true);
DeferredStartupHandler.setExpectingActivityStartupForTesting(); mActivityTestRule.recreateActivity();
ChromeTabbedActivity newActivity = ChromeTabbedActivity newActivity = mActivityTestRule.getActivity();
ApplicationTestUtils.recreateActivity(mActivityTestRule.getActivity());
new TabModelSelectorTabModelObserver(newActivity.getTabModelSelector()) { new TabModelSelectorTabModelObserver(newActivity.getTabModelSelector()) {
@Override @Override
...@@ -1969,9 +1966,7 @@ public class StartSurfaceLayoutTest { ...@@ -1969,9 +1966,7 @@ public class StartSurfaceLayoutTest {
"New Activity current TabModel is not incognito"); "New Activity current TabModel is not incognito");
TestThreadUtils.runOnUiThreadBlocking(IncognitoUtils::closeAllIncognitoTabs); TestThreadUtils.runOnUiThreadBlocking(IncognitoUtils::closeAllIncognitoTabs);
assertTrue("Deferred startup never completed", assertTrue("Deferred startup never completed", mActivityTestRule.waitForDeferredStartup());
DeferredStartupHandler.waitForDeferredStartupCompleteForTesting(
ScalableTimeout.scaleTimeout(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL)));
CriteriaHelper.pollUiThread(() CriteriaHelper.pollUiThread(()
-> tabModelRestoreCompleted.get(), -> tabModelRestoreCompleted.get(),
......
...@@ -90,9 +90,7 @@ import org.chromium.base.test.util.DisableIf; ...@@ -90,9 +90,7 @@ import org.chromium.base.test.util.DisableIf;
import org.chromium.base.test.util.DisabledTest; import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction; import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.ScalableTimeout;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.DeferredStartupHandler;
import org.chromium.chrome.browser.compositor.layouts.phone.StackLayout; import org.chromium.chrome.browser.compositor.layouts.phone.StackLayout;
import org.chromium.chrome.browser.feed.FeedSurfaceCoordinator; import org.chromium.chrome.browser.feed.FeedSurfaceCoordinator;
import org.chromium.chrome.browser.feed.FeedSurfaceMediator; import org.chromium.chrome.browser.feed.FeedSurfaceMediator;
...@@ -177,7 +175,6 @@ public class StartSurfaceTest { ...@@ -177,7 +175,6 @@ public class StartSurfaceTest {
* because of its {@link org.chromium.chrome.browser.tab.Tab} dependency. * because of its {@link org.chromium.chrome.browser.tab.Tab} dependency.
*/ */
private void startMainActivityFromLauncher() { private void startMainActivityFromLauncher() {
DeferredStartupHandler.setExpectingActivityStartupForTesting();
Intent intent = new Intent(Intent.ACTION_MAIN); Intent intent = new Intent(Intent.ACTION_MAIN);
intent.addCategory(Intent.CATEGORY_LAUNCHER); intent.addCategory(Intent.CATEGORY_LAUNCHER);
mActivityTestRule.prepareUrlIntent(intent, null); mActivityTestRule.prepareUrlIntent(intent, null);
...@@ -1221,9 +1218,7 @@ public class StartSurfaceTest { ...@@ -1221,9 +1218,7 @@ public class StartSurfaceTest {
&& mActivityTestRule.getActivity().getLayoutManager().overviewVisible()); && mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
mActivityTestRule.waitForActivityNativeInitializationComplete(); mActivityTestRule.waitForActivityNativeInitializationComplete();
assertTrue("Deferred startup never completed", assertTrue("Deferred startup never completed", mActivityTestRule.waitForDeferredStartup());
DeferredStartupHandler.waitForDeferredStartupCompleteForTesting(
ScalableTimeout.scaleTimeout(20000L)));
boolean isInstantStart = TabUiFeatureUtilities.supportInstantStart(false); boolean isInstantStart = TabUiFeatureUtilities.supportInstantStart(false);
Assert.assertEquals(1, Assert.assertEquals(1,
......
...@@ -79,15 +79,18 @@ public class DeferredStartupHandler { ...@@ -79,15 +79,18 @@ public class DeferredStartupHandler {
* Avoid using CriteriaHelper for waiting for deferred tasks to complete, as the act of polling * Avoid using CriteriaHelper for waiting for deferred tasks to complete, as the act of polling
* can prevent the Looper from going idle, preventing the tasks from running. * can prevent the Looper from going idle, preventing the tasks from running.
* *
* You must call {@link #setExpectingActivityStartupForTesting()} before calling this. * You should wait until the activity has posted its deferred startup tasks before calling this
* function to avoid races.
* *
* @return Whether deferred startup has been completed before the timeout expires. * @return Whether deferred startup has been completed before the timeout expires.
*/ */
public static boolean waitForDeferredStartupCompleteForTesting(long timeoutMillis) { public static boolean waitForDeferredStartupCompleteForTesting(long timeoutMillis) {
ThreadUtils.assertOnBackgroundThread(); ThreadUtils.assertOnBackgroundThread();
// sInstance could become null while executing this function, so keep a ref here. // sInstance could become null while executing this function, so keep a ref here.
DeferredStartupHandler instance = DeferredStartupHandler instance = ThreadUtils.runOnUiThreadBlockingNoException(() -> {
ThreadUtils.runOnUiThreadBlockingNoException(() -> { return sInstance; }); if (sInstance != null) sInstance.mLatchForTesting = new CountDownLatch(1);
return sInstance;
});
// Tasks completed and instance was cleared before we started waiting. // Tasks completed and instance was cleared before we started waiting.
if (instance == null) return true; if (instance == null) return true;
assert instance.mLatchForTesting != null; assert instance.mLatchForTesting != null;
...@@ -97,9 +100,4 @@ public class DeferredStartupHandler { ...@@ -97,9 +100,4 @@ public class DeferredStartupHandler {
return false; return false;
} }
} }
public static void setExpectingActivityStartupForTesting() {
ThreadUtils.runOnUiThreadBlocking(
() -> { getInstance().mLatchForTesting = new CountDownLatch(1); });
}
} }
...@@ -2468,4 +2468,9 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent> ...@@ -2468,4 +2468,9 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
public Configuration getSavedConfigurationForTesting() { public Configuration getSavedConfigurationForTesting() {
return mConfig; return mConfig;
} }
@VisibleForTesting
public boolean deferredStartupPostedForTesting() {
return mDeferredStartupPosted;
}
} }
...@@ -336,7 +336,6 @@ public class FirstRunIntegrationTest { ...@@ -336,7 +336,6 @@ public class FirstRunIntegrationTest {
// policy set in this test case. // policy set in this test case.
FirstRunStatus.setFirstRunSkippedByPolicy(true); FirstRunStatus.setFirstRunSkippedByPolicy(true);
DeferredStartupHandler.setExpectingActivityStartupForTesting();
Intent intent = CustomTabsTestUtils.createMinimalCustomTabIntent(mContext, "about:blank"); Intent intent = CustomTabsTestUtils.createMinimalCustomTabIntent(mContext, "about:blank");
mContext.startActivity(intent); mContext.startActivity(intent);
CustomTabActivity activity = waitForActivity(CustomTabActivity.class); CustomTabActivity activity = waitForActivity(CustomTabActivity.class);
...@@ -344,6 +343,7 @@ public class FirstRunIntegrationTest { ...@@ -344,6 +343,7 @@ public class FirstRunIntegrationTest {
// DeferredStartupHandler could not finish with CriteriaHelper#DEFAULT_MAX_TIME_TO_POLL. // DeferredStartupHandler could not finish with CriteriaHelper#DEFAULT_MAX_TIME_TO_POLL.
// Use longer timeout here to avoid flakiness. See https://crbug.com/1157611. // Use longer timeout here to avoid flakiness. See https://crbug.com/1157611.
CriteriaHelper.pollUiThread(() -> activity.deferredStartupPostedForTesting());
Assert.assertTrue("Deferred startup never completed", Assert.assertTrue("Deferred startup never completed",
DeferredStartupHandler.waitForDeferredStartupCompleteForTesting( DeferredStartupHandler.waitForDeferredStartupCompleteForTesting(
ScalableTimeout.scaleTimeout(DEFERRED_START_UP_POLL_TIME))); ScalableTimeout.scaleTimeout(DEFERRED_START_UP_POLL_TIME)));
......
...@@ -199,7 +199,6 @@ public class ChromeActivityTestRule<T extends ChromeActivity> extends BaseActivi ...@@ -199,7 +199,6 @@ public class ChromeActivityTestRule<T extends ChromeActivity> extends BaseActivi
* Similar to #launchActivity(Intent), but waits for the Activity tab to be initialized. * Similar to #launchActivity(Intent), but waits for the Activity tab to be initialized.
*/ */
public void startActivityCompletely(Intent intent) { public void startActivityCompletely(Intent intent) {
DeferredStartupHandler.setExpectingActivityStartupForTesting();
launchActivity(intent); launchActivity(intent);
waitForActivityNativeInitializationComplete(); waitForActivityNativeInitializationComplete();
...@@ -214,15 +213,18 @@ public class ChromeActivityTestRule<T extends ChromeActivity> extends BaseActivi ...@@ -214,15 +213,18 @@ public class ChromeActivityTestRule<T extends ChromeActivity> extends BaseActivi
NewTabPageTestUtils.waitForNtpLoaded(tab); NewTabPageTestUtils.waitForNtpLoaded(tab);
} }
Assert.assertTrue("Deferred startup never completed. Did you try to start an Activity " Assert.assertTrue(waitForDeferredStartup());
+ "that was already started?",
DeferredStartupHandler.waitForDeferredStartupCompleteForTesting(
ScalableTimeout.scaleTimeout(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL)));
Assert.assertNotNull(tab); Assert.assertNotNull(tab);
Assert.assertNotNull(tab.getView()); Assert.assertNotNull(tab.getView());
} }
public boolean waitForDeferredStartup() {
CriteriaHelper.pollUiThread(() -> getActivity().deferredStartupPostedForTesting());
return DeferredStartupHandler.waitForDeferredStartupCompleteForTesting(
ScalableTimeout.scaleTimeout(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL));
}
@Override @Override
public void launchActivity(Intent startIntent) { public void launchActivity(Intent startIntent) {
// Avoid relying on explicit intents, bypassing LaunchIntentDispatcher, created by null // Avoid relying on explicit intents, bypassing LaunchIntentDispatcher, created by null
......
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