New issue
Advanced search Search tips

Issue 806214 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 18
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 784331



Sign in to add a comment

TabModelMergingTest#testMergeOnColdStartFromChromeTabbedActivity2 fails on Android N

Reported by mirko.r...@mips.com, Jan 26 2018

Issue description

Steps to reproduce the problem:
1. Use device with Android N (reproduced on Nexus 10 with custom Android N)
2. ./out/Release/bin/run_chrome_public_test_apk  -f org.chromium.chrome.browser.tabmodel.TabModelMergingTest#testMergeOnColdStartFromChromeTabbedActivity2

What is the expected behavior?
test to pass 

What went wrong?
Tests failed:
C   37.239s Main  [FAIL] org.chromium.chrome.browser.tabmodel.TabModelMergingTest#testMergeOnColdStartFromChromeTabbedActivity2:
C   37.240s Main  java.lang.AssertionError: Wrong number of normal tabs expected:<7> but was:<6>
C   37.240s Main      at org.junit.Assert.fail(Assert.java:88)
C   37.240s Main      at org.junit.Assert.failNotEquals(Assert.java:834)
C   37.240s Main      at org.junit.Assert.assertEquals(Assert.java:645)
C   37.240s Main      at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.assertTabModelMatchesExpectations(TabModelMergingTest.java:253)
C   37.240s Main      at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.startNewChromeTabbedActivityAndAssert(TabModelMergingTest.java:245)
C   37.240s Main      at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.testMergeOnColdStartFromChromeTabbedActivity2(TabModelMergingTest.java:361) 

Did this work before? N/A 

Chrome version: Top of master  Channel: n/a
OS Version: 7.1.1
Flash Version: 

The test is failing due to 6 out of 7 tabs being restored.
Tab in position 1. (in 0-6) is not being restored since it's URL is recognised as NTP URL, as per logacat:

01-25 18:10:23.310   731   731 I cr_tabmodel: Skipping restore of non-selected NTP.

This tab has url "chrome-native://newtab/".

Five tabs's urls are: "data:text/html;utf-8,%3Chtml%3Etest_url_0.%3C%2Fhtml%", with numbers 0 to 4, as defined by the ts and one url is "about:blank".
 
Blocking: 784331
Components: UI>Browser>Mobile>MultiWindow
Owner: twelling...@chromium.org
Status: Started (was: Unconfirmed)
This also appears to be failing on our O test bot. I'll investigate.
We landed a change [i] several months ago that uses a brand new blank intent for launching the second activity tab. Previously we were re-using the first activity's intent. This explains the change to chrome://newtab.

I think the right fix for the purposes of this test is to load a URL in ChromeTabbedActivity2's so that all tabs are restored when reading the tab state from disk after a cold start.

CL out for review: https://chromium-review.googlesource.com/#/c/chromium/src/+/899868


[i] https://chromium-review.googlesource.com/c/chromium/src/+/707359
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/147cb419ee7e7a42eb2c5eb7310c5374fdb6d42f

commit 147cb419ee7e7a42eb2c5eb7310c5374fdb6d42f
Author: Theresa <twellington@chromium.org>
Date: Fri Feb 02 19:58:34 2018

Fix TabModelMergingTest#testMergeOnColdStartFromChromeTabbedActivity2

The test was failing because we changed how ChromeTabbedActivity2 is
launched in Oct 17. Previously we created ChromeTabbedActivity2 re-using
ChromeTabbedActivity's intent, so the CTA2 activity tab was about:blank.
After the change, we now create CTA2 with a brand new, blank intent, so
the CTA2 activity tab is chrome://newtab. We ignore chrome://newtab on
cold start, so the expected number of tabs loaded differed from the
actual number of tabs loaded after a cold-start.

This patch loads a URL in CTA2's activity tab so that all of its tabs
are created when reading tab state from disk after a cold start.

BUG= 806214 

Change-Id: I72fc8261fb899704409d34be210c623346e1397b
Reviewed-on: https://chromium-review.googlesource.com/899868
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534130}
[modify] https://crrev.com/147cb419ee7e7a42eb2c5eb7310c5374fdb6d42f/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelMergingTest.java

FYI bot to confirm this is fixed once the change rolls through: https://ci.chromium.org/buildbot/chromium.android.fyi/Nougat%20Phone%20Tester/
This test is fixed, but it looks like the change broke TabModelMergingTest#testMergeWhileInTabSwitcher and made #testMergeOnColdStart flaky:

org.junit.ComparisonFailure: Wrong tab at position 4 expected:<[data:text/html;utf-8,%3Chtml%3Etest_url_5.%3C%2Fhtml%3E]> but was:<[chrome-native://newtab/]>
	at org.junit.Assert.assertEquals(Assert.java:115)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.assertTabModelMatchesExpectations(TabModelMergingTest.java:258)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.mergeTabsAndAssert(TabModelMergingTest.java:205)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.mergeTabsAndAssert(TabModelMergingTest.java:176)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.testMergeWhileInTabSwitcher(TabModelMergingTest.java:370)
	at java.lang.reflect.Method.invoke(Native Method)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:52)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.chromium.base.test.ScreenshotOnFailureStatement.evaluate(ScreenshotOnFailureStatement.java:37)
	at org.chromium.chrome.test.ChromeActivityTestRule$1.evaluate(ChromeActivityTestRule.java:113)
	at android.support.test.internal.statement.UiThreadStatement.evaluate(UiThreadStatement.java:55)
	at android.support.test.rule.ActivityTestRule$ActivityStatement.evaluate(ActivityTestRule.java:270)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.chromium.base.test.BaseJUnit4ClassRunner.runChild(BaseJUnit4ClassRunner.java:201)
	at org.chromium.base.test.BaseJUnit4ClassRunner.runChild(BaseJUnit4ClassRunner.java:43)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.chromium.base.test.BaseJUnit4ClassRunner.run(BaseJUnit4ClassRunner.java:183)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
	at android.support.test.internal.runner.TestExecutor.execute(TestExecutor.java:59)
	at android.support.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:262)
	at org.chromium.base.test.BaseChromiumAndroidJUnitRunner.onStart(BaseChromiumAndroidJUnitRunner.java:99)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1932)


org.junit.ComparisonFailure: Wrong tab at position 4 expected:<[data:text/html;utf-8,%3Chtml%3Etest_url_5.%3C%2Fhtml%3E]> but was:<[chrome-native://newtab/]>
	at org.junit.Assert.assertEquals(Assert.java:115)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.assertTabModelMatchesExpectations(TabModelMergingTest.java:258)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.startNewChromeTabbedActivityAndAssert(TabModelMergingTest.java:241)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.testMergeOnColdStart(TabModelMergingTest.java:309)
	at java.lang.reflect.Method.invoke(Native Method)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:52)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.chromium.base.test.ScreenshotOnFailureStatement.evaluate(ScreenshotOnFailureStatement.java:37)
	at org.chromium.chrome.test.ChromeActivityTestRule$1.evaluate(ChromeActivityTestRule.java:113)
	at android.support.test.internal.statement.UiThreadStatement.evaluate(UiThreadStatement.java:55)
	at android.support.test.rule.ActivityTestRule$ActivityStatement.evaluate(ActivityTestRule.java:270)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.chromium.base.test.BaseJUnit4ClassRunner.runChild(BaseJUnit4ClassRunner.java:201)
	at org.chromium.base.test.BaseJUnit4ClassRunner.runChild(BaseJUnit4ClassRunner.java:43)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.chromium.base.test.BaseJUnit4ClassRunner.run(BaseJUnit4ClassRunner.java:183)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
	at android.support.test.internal.runner.TestExecutor.execute(TestExecutor.java:59)
	at android.support.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:262)
	at org.chromium.base.test.BaseChromiumAndroidJUnitRunner.onStart(BaseChromiumAndroidJUnitRunner.java:99)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2074)
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f00ed4b6bea2e76488184160e1d1e26b60331edf

commit f00ed4b6bea2e76488184160e1d1e26b60331edf
Author: Theresa <twellington@chromium.org>
Date: Mon Feb 05 19:26:54 2018

Deflake TabModelMergingTest

Deflake TabModelMergingTest by saving tab state after loading URLs and
creating tabs. Some tests started flaking after changing CTA2's activity
tab to a test URL rather than chrome://newtab. Forcing a tab save
ensures the new tab state is written to disk in #setup.

BUG= 806214 

Change-Id: I97a2324c5f8bd8e387d7dd73ee61cdc1633111e6
Reviewed-on: https://chromium-review.googlesource.com/902193
Commit-Queue: Theresa <twellington@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534457}
[modify] https://crrev.com/f00ed4b6bea2e76488184160e1d1e26b60331edf/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelMergingTest.java

It looks like the attempt to deflake in #7 was not successful, despite my local testing. I'm going to try starting ChromeTabbedActivity2 with an intent for TEST_URL5 rather than allowing it to load with chrome://newtab.
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed4f1f2e1dbe0982fa0618cd816aa6f07eaad655

commit ed4f1f2e1dbe0982fa0618cd816aa6f07eaad655
Author: Theresa <twellington@chromium.org>
Date: Thu Feb 08 17:04:18 2018

Another attempt to deflake TabModelMergingTest

TabModelMergingTest#testMergeWhileInTabSwitcher is still flaky. Attempt
to deflake by launching CTA2 using an intent to open TEST_URL_7 rather
than a blank intent.

TabModelMergingTest#testMergeOnColdStart is also flakily failing due to
CTA2 not being fully destroyed. Destroy each activity individually in an
attempt to deflake (passed 15/15 locally).

BUG= 806214 

Change-Id: I3b360553f6f789ed8919f3f85253f253fbe75e19
Reviewed-on: https://chromium-review.googlesource.com/904854
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535412}
[modify] https://crrev.com/ed4f1f2e1dbe0982fa0618cd816aa6f07eaad655/chrome/android/javatests/src/org/chromium/chrome/browser/multiwindow/MultiWindowTestHelper.java
[modify] https://crrev.com/ed4f1f2e1dbe0982fa0618cd816aa6f07eaad655/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelMergingTest.java

It looks like that did the trick for issues with the wrong URL.

I am still seeing some flakes on #testMergeOnColdStartFromChromeTabbedActivity2 on the N fyi bot due to CTA2 not finishing:

java.lang.AssertionError: Both activitie should be destroyed.CTA state: 6 - CTA2State: 3
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.chromium.content.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:93)
	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:172)
	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:194)
	at org.chromium.chrome.browser.tabmodel.TabModelMergingTest.testMergeOnColdStartFromChromeTabbedActivity2(TabModelMergingTest.java:348)
Status: Fixed (was: Started)

Sign in to add a comment