New issue
Advanced search Search tips

Issue 733890 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 733678



Sign in to add a comment

TabModelMergingTests consistently fail on N

Project Member Reported by jbudorick@chromium.org, Jun 16 2017

Issue description

Components: UI>Browser>Mobile>MultiWindow
Labels: -Pri-3 Pri-2
Owner: twelling...@chromium.org
Status: Assigned (was: Untriaged)
This is possibly fallout from the conversion to junit 4, which is the only recent change to that test class.
that was my first guess too, but it failed for me locally even after reverting to the junit3 version. this may not have started recently
What's our target for getting the N bot healthy and online?

The change that throws an illegal state exception while trying to access the tab model selector, while fairly old, was made after the last non-mechanical change to TabModelMergingTest.

Last non-mechanical TabModelMergingTest on Aug 15, 2016: https://chromium.googlesource.com/chromium/src/+/1382473e75f7ae9fc20ce45356e1c94855ede461

Change to throw IllegalStateException in #getTabModelSelector on Oct 22, 2016: https://chromium.googlesource.com/chromium/src/+/820a95b0b81d33e42712f9198c215f703412e1a1

We can probably change the criteria helper to rely on ChromeActivity#areTabModelsInitialized() instead of #getTabModelSelector()#isTabStateInitialized()

Stack trace:
Caused by: java.lang.IllegalStateException: Attempting to access TabModelSelector before initialization
	at org.chromium.chrome.browser.ChromeActivity.getTabModelSelector(ChromeActivity.java:1480)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest$1.isSatisfied(TabModelMergingTest.java:98)
	at org.chromium.content.browser.test.util.CriteriaHelper$1.call(CriteriaHelper.java:113)
	at org.chromium.content.browser.test.util.CriteriaHelper$1.call(CriteriaHelper.java:110)
	at java.util.concurrent.FutureTask.run(FutureTask.java:237)
	at android.os.Handler.handleCallback(Handler.java:751)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:154)
	at android.app.ActivityThread.main(ActivityThread.java:6077)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:865)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:755)
There are two issues. The first is the IllegalStateException, which can be fixed by checking #areTabModelsInitialized() before TabModelSelector#isTabStateInitialized().

The second is the changes I made a while back to ChromeTabbedActivity#isStartedUpCorrectly() to check if there are multiple instances running in fullscreen mode. In the TabModelMerging tests, isStartedUpCorrectly() returns false and the second activity's launch is aborted.


Fix out for review: https://chromium-review.googlesource.com/c/538071/
Fix landed:

[Android] Fix TabModelMergingTests

The TabModelMergingTests have been failing on an IllegalStateException
because they are trying to access the tab models before they have been
initialized. The CriteriaHelper accessing the tab models has been
updated to check that that are initialized before accessing them.

The tests are also failing because
ChromeTabbedActivity#isStartedUpCorrectly() returns false when two
fullscreen ChromeTabbedActivity classes are showing. This change also
adds a call to ChromeTabbedActivity#onMultiInstanceModeStarted() so that
the #isStartedUpCorrectly() check doesn't fail.

BUG= 733890 

Change-Id: I4d3e857f357756f68dfcf7a3efb5567c0d50fa31
Reviewed-on: https://chromium-review.googlesource.com/538071
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480464}
Status: Fixed (was: Assigned)
Thanks for fixing this.

To answer your question from #3, we don't have a set timeline at the moment but the ongoing instability in both this bot and other N+ bots is something I'd like to address sooner rather than later -- probably Q3. That I've let the N bot languish in its current state is something I'm not proud of :(

Sign in to add a comment