Commit 7ffeb9cc authored by Jinsuk Kim's avatar Jinsuk Kim Committed by Commit Bot

Fix WebappModeTest.testWebappRequiresValidMac

The test passes fine on the release build but shows flakiness on
the debug build whose trybot result I'm trying to stablize. It
takes a bit longer to get the test page fully loaded on the debug
build, therefore the test often timed out before proceeding to
the next step. This CL ensures the scale factor gets matched
by waiting for the tab to be created before the test is performed. 
Local tests shows 100% pass, compared to about 50% failures before
the patch.

BUG=737910, 746476

Change-Id: If270b2c5dba122c899cba0f050ca808878773048
Reviewed-on: https://chromium-review.googlesource.com/559330Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Reviewed-by: default avatarChangwan Ryu <changwan@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488063}
parent 0052be39
...@@ -722,7 +722,7 @@ public class TabsOpenedFromExternalAppTest { ...@@ -722,7 +722,7 @@ public class TabsOpenedFromExternalAppTest {
} }
})); }));
ApplicationTestUtils.assertWaitForPageScaleFactorMatch( ApplicationTestUtils.assertWaitForPageScaleFactorMatch(
mActivityTestRule.getActivity(), 0.5f, false); mActivityTestRule.getActivity(), 0.5f);
// Long press the center of the page, which should bring up the context menu. // Long press the center of the page, which should bring up the context menu.
final TestTabObserver observer = new TestTabObserver(); final TestTabObserver observer = new TestTabObserver();
......
...@@ -50,13 +50,4 @@ public abstract class MultiActivityTestBase extends InstrumentationTestCase ...@@ -50,13 +50,4 @@ public abstract class MultiActivityTestBase extends InstrumentationTestCase
protected void waitForFullLoad(final ChromeActivity activity, final String expectedTitle) { protected void waitForFullLoad(final ChromeActivity activity, final String expectedTitle) {
mTestCommon.waitForFullLoad(activity, expectedTitle); mTestCommon.waitForFullLoad(activity, expectedTitle);
} }
/**
* Approximates when a ChromeActivity is fully ready and loaded, which is hard to gauge
* because Android's Activity transition animations are not monitorable.
*/
protected void waitForFullLoad(final ChromeActivity activity, final String expectedTitle,
boolean waitLongerForLoad) {
mTestCommon.waitForFullLoad(activity, expectedTitle, waitLongerForLoad);
}
} }
...@@ -8,17 +8,26 @@ import android.app.Instrumentation; ...@@ -8,17 +8,26 @@ import android.app.Instrumentation;
import android.content.Context; import android.content.Context;
import android.text.TextUtils; import android.text.TextUtils;
import org.chromium.base.Log;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorObserver;
import org.chromium.chrome.browser.tabmodel.document.DocumentTabModelSelector; import org.chromium.chrome.browser.tabmodel.document.DocumentTabModelSelector;
import org.chromium.chrome.test.util.ApplicationTestUtils; import org.chromium.chrome.test.util.ApplicationTestUtils;
import org.chromium.chrome.test.util.browser.tabmodel.document.MockStorageDelegate; import org.chromium.chrome.test.util.browser.tabmodel.document.MockStorageDelegate;
import org.chromium.content.browser.test.util.Criteria; import org.chromium.content.browser.test.util.Criteria;
import org.chromium.content.browser.test.util.CriteriaHelper; import org.chromium.content.browser.test.util.CriteriaHelper;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
// TODO(yolandyan): move this class to its test rule once JUnit4 migration is over // TODO(yolandyan): move this class to its test rule once JUnit4 migration is over
final class MultiActivityTestCommon { final class MultiActivityTestCommon {
private static final String TAG = "MultiActivityTest";
private final MultiActivityTestCommonCallback mCallback; private final MultiActivityTestCommonCallback mCallback;
MockStorageDelegate mStorageDelegate; MockStorageDelegate mStorageDelegate;
Context mContext; Context mContext;
...@@ -45,12 +54,8 @@ final class MultiActivityTestCommon { ...@@ -45,12 +54,8 @@ final class MultiActivityTestCommon {
} }
void waitForFullLoad(final ChromeActivity activity, final String expectedTitle) { void waitForFullLoad(final ChromeActivity activity, final String expectedTitle) {
waitForFullLoad(activity, expectedTitle, false); waitForTabCreation(activity);
} ApplicationTestUtils.assertWaitForPageScaleFactorMatch(activity, 0.5f);
void waitForFullLoad(
final ChromeActivity activity, final String expectedTitle, boolean waitLongerForLoad) {
ApplicationTestUtils.assertWaitForPageScaleFactorMatch(activity, 0.5f, waitLongerForLoad);
final Tab tab = activity.getActivityTab(); final Tab tab = activity.getActivityTab();
assert tab != null; assert tab != null;
...@@ -64,5 +69,23 @@ final class MultiActivityTestCommon { ...@@ -64,5 +69,23 @@ final class MultiActivityTestCommon {
}); });
} }
private void waitForTabCreation(final ChromeActivity activity) {
final CountDownLatch latch = new CountDownLatch(1);
activity.getTabModelSelector().addObserver(new TabModelSelectorObserver() {
public void onChange() {}
public void onNewTabCreated(Tab tab) {
latch.countDown();
}
public void onTabModelSelected(TabModel newModel, TabModel oldModel) {}
public void onTabStateInitialized() {}
});
try {
latch.await(CallbackHelper.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Log.w(TAG, "CountDownLatch interrupted. The test may fail.");
}
}
public interface MultiActivityTestCommonCallback { Instrumentation getInstrumentation(); } public interface MultiActivityTestCommonCallback { Instrumentation getInstrumentation(); }
} }
...@@ -36,11 +36,6 @@ public class MultiActivityTestRule implements TestRule, MultiActivityTestCommonC ...@@ -36,11 +36,6 @@ public class MultiActivityTestRule implements TestRule, MultiActivityTestCommonC
mTestCommon.waitForFullLoad(activity, expectedTitle); mTestCommon.waitForFullLoad(activity, expectedTitle);
} }
public void waitForFullLoad(
final ChromeActivity activity, final String expectedTitle, boolean waitLongerForLoad) {
mTestCommon.waitForFullLoad(activity, expectedTitle, waitLongerForLoad);
}
@Override @Override
public Statement apply(final Statement base, Description desc) { public Statement apply(final Statement base, Description desc) {
return new Statement() { return new Statement() {
......
...@@ -191,14 +191,6 @@ public class ApplicationTestUtils { ...@@ -191,14 +191,6 @@ public class ApplicationTestUtils {
return activityManager.getAppTasks().size(); return activityManager.getAppTasks().size();
} }
/**
* See {@link #assertWaitForPageScaleFactorMatch(ChromeActivity,float,long)}.
*/
public static void assertWaitForPageScaleFactorMatch(
final ChromeActivity activity, final float expectedScale) {
assertWaitForPageScaleFactorMatch(activity, expectedScale, false);
}
/** /**
* Waits till the ContentViewCore receives the expected page scale factor * Waits till the ContentViewCore receives the expected page scale factor
* from the compositor and asserts that this happens. * from the compositor and asserts that this happens.
...@@ -206,9 +198,8 @@ public class ApplicationTestUtils { ...@@ -206,9 +198,8 @@ public class ApplicationTestUtils {
* Proper use of this function requires waiting for a page scale factor that isn't 1.0f because * Proper use of this function requires waiting for a page scale factor that isn't 1.0f because
* the default seems to be 1.0f. * the default seems to be 1.0f.
*/ */
public static void assertWaitForPageScaleFactorMatch(final ChromeActivity activity, public static void assertWaitForPageScaleFactorMatch(
final float expectedScale, boolean waitLongerForLoad) { final ChromeActivity activity, final float expectedScale) {
long waitTimeInMs = waitLongerForLoad ? 10000 : CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL;
CriteriaHelper.pollInstrumentationThread(new Criteria() { CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override @Override
public boolean isSatisfied() { public boolean isSatisfied() {
...@@ -219,6 +210,6 @@ public class ApplicationTestUtils { ...@@ -219,6 +210,6 @@ public class ApplicationTestUtils {
return Math.abs(activity.getCurrentContentViewCore().getScale() - expectedScale) return Math.abs(activity.getCurrentContentViewCore().getScale() - expectedScale)
< FLOAT_EPSILON; < FLOAT_EPSILON;
} }
}, waitTimeInMs, CriteriaHelper.DEFAULT_POLLING_INTERVAL); }, CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL, CriteriaHelper.DEFAULT_POLLING_INTERVAL);
} }
} }
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