New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment
link

Issue 907602: VR DDView tests on Oreo suddenly taking >50% more time

Reported by bsheedy@chromium.org, Nov 21 Project Member

Issue description

As of https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Oreo%20Phone%20Tester/1882, the VR tests on Oreo are suddenly taking more than 50% longer to complete, causing the 1 hour timeout to be hit and a large number of tests to not be run.

This only appears to affect the VR standard tests with Daydream View paired on Oreo - The other VR tests on Oreo (e.g. with Cardboard paired or that dynamically change settings during a test) seem to be running the same as usual. The same tests are also running fine on Nougat.

There's nothing relevant looking in the blamelist for that build, but this seems to correspond to a sudden change in the way the flakiness dashboard displays information, so that might be related? https://test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&testType=chrome_public_test_vr_apk-ddready-ddview&builder=chromium.android%3AOreo%20Phone%20Tester
 

Comment 1 by bsheedy@chromium.org, Nov 21

The issue appears to be that it can take an absurd amount of time (2+ minutes) to actually start a test case:

I 2542.104s run_tests_on_device(HT74C0209267)  preparing to run org.chromium.chrome.browser.vr.VrBrowserWebInputEditingTest#testTextSelection: {'annotations': {u'Test': {u'expected': u'class org.junit.Test$None', u'timeout': u'0'}, u'Restriction': {u'value': [u'Daydream_View_Or_Standalone_VR']}, u'Add': {u'value': [u'disable-fre']}, u'MediumTest': {}, u'RunWith': {u'value': u'class org.chromium.chrome.test.ChromeJUnit4ClassRunner'}}, 'class': u'org.chromium.chrome.browser.vr.VrBrowserWebInputEditingTest', 'is_junit4': True, 'method': u'testTextSelection'}
I 2602.554s run_tests_on_device(HT74C0209267)  Still working on StartInstrumentation(HT74C0209267, org.chromium.chrome/org.chromium.base.test.BaseChromiumAndroidJUnitRunner, raw=True, extras={'org.chromium.base.test.util.Screenshooter.ScreenshotDir': '/sdcard/tmp-5624c9238d44f', 'class': u'org.chromium.chrome.browser.vr.VrBrowserWebInputEditingTest#testTextSelection', 'org.chromium.base.test.ScreenshotOnFailureStatement.ScreenshotFile': '/sdcard/temp_file-a5975069aa3dc.png'}, timeout=180, retries=0)
I 2662.648s run_tests_on_device(HT74C0209267)  Still working on StartInstrumentation(HT74C0209267, org.chromium.chrome/org.chromium.base.test.BaseChromiumAndroidJUnitRunner, raw=True, extras={'org.chromium.base.test.util.Screenshooter.ScreenshotDir': '/sdcard/tmp-5624c9238d44f', 'class': u'org.chromium.chrome.browser.vr.VrBrowserWebInputEditingTest#testTextSelection', 'org.chromium.base.test.ScreenshotOnFailureStatement.ScreenshotFile': '/sdcard/temp_file-a5975069aa3dc.png'}, timeout=180, retries=0)

Comment 2 by bsheedy@chromium.org, Nov 21

Components: -Infra>Flakiness>Dashboard
While I don't see how, this does appear to be tied to some src-side change. I'm able to repro locally on ToT, but not at an earlier revision. I'll run a bisect.

Comment 3 by bsheedy@chromium.org, Nov 21

Components: UI>Browser>VR
Labels: -Pri-1 Pri-2
Owner: jinsuk...@chromium.org
Status: Assigned (was: Untriaged)
I don't know how, but the bisect points to https://chromium-review.googlesource.com/c/1272779 as the culprit. I've double checked, and it is indeed the culprit, so I'll go ahead and revert.

Comment 4 by bsheedy@chromium.org, Nov 21

Still not sure what's happening under the hood, but visually, Chrome was starting right away and entering VR like expected, but then just sat there until the test timed out after 180 seconds.

Comment 5 by jinsuk...@chromium.org, Nov 22

> This only appears to affect the VR standard tests with Daydream View paired on Oreo 

Just to clarify - do I need a Daydream viewer device hooked up to Oreo for reproducing the failing test? I'm now locally using OPR1 on Marlin (Pixel 1) but can't reproduce the described issue.

Comment 6 by jinsuk...@chromium.org, Nov 22

Cc: bsheedy@chromium.org

Comment 7 by bsheedy@chromium.org, Nov 26

You don't need any additional physical hardware, but VrCore needs to think that it's using the Daydream View headset. The documentation for running the tests properly is at https://chromium.googlesource.com/chromium/src/+/HEAD/chrome/android/javatests/src/org/chromium/chrome/browser/vr/README.md, but the TL;DR now that you have a Pixel is:

1. Make sure the "VR Services" app is up to date in the Playstore
2. Run the test with the --shared-prefs-file=//chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json option

Comment 8 by bugdroid1@chromium.org, Dec 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76d39b3a8d8fbd0e990f92a992d80213724de9bd

commit 76d39b3a8d8fbd0e990f92a992d80213724de9bd
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Wed Dec 05 23:06:59 2018

Reland "Android: Factor InProductHelp UI out of Tab"


https://chromium-review.googlesource.com/c/chromium/src/+/1272779 had a couple
of bugs in |ToolbarButtonInProductHelpController|:

1) Ignored |profile| passed into |maybeShowDownloadContinuingIPH()| and
   always used Profile.getLastUsedProfile()
2) |OnDismissListener| was instantiated (and menu got highlight for
   the text bubble) every time |setupAndMaybeShowIPHForFeature| is called,
   even when the method doesn't meet the condition for IPH text bubble
   and has to early out.

The CL got reverted due to numerous regressions . This CL fixes them and
relands it.

This reverts commit d436a7a2e33ee105a1d85370970fde3c88255302.

Bug: 889682, 907502, 907503, 907770, 907505,  907602 
Change-Id: Id6de148a9fd4fd91ad7df986ca0ba78ed18e79e4
Reviewed-on: https://chromium-review.googlesource.com/c/1349142
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614161}
[modify] https://crrev.com/76d39b3a8d8fbd0e990f92a992d80213724de9bd/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
[modify] https://crrev.com/76d39b3a8d8fbd0e990f92a992d80213724de9bd/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/76d39b3a8d8fbd0e990f92a992d80213724de9bd/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarButtonInProductHelpController.java

Comment 9 by jinsuk...@chromium.org, Dec 5

Relanded the CL after fixing bugs that caused other regressions. https://crrev.com/c/1349142 Unfortunately I wasn't able to reproduce this bug locally - the test crashes with or without the CL, seemingly due to an unrelated issue. Will keep an eye on the VR test on https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Oreo%20Phone%20Tester/1882

Comment 11 by bsheedy@chromium.org, Dec 6

I don't think that's actually your fault - one of the devices went offline, and since capacity was so tight with it in the pool, the VR tests are now timing out waiting for a device because the non-VR tests on that bot are using all of them.

I have a patch going through the CQ that disables the non-VR tests (they were set to experimental and failing all the time anyways), so we'll see once that makes it through.

Comment 12 by bugdroid1@chromium.org, Dec 7

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

commit fbe75ef510a9d9123f6c7f8cbd54d5f5b5680277
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Thu Dec 06 22:10:37 2018

Revert "Reland "Android: Factor InProductHelp UI out of Tab""

This reverts commit 76d39b3a8d8fbd0e990f92a992d80213724de9bd.

Reason for revert:  https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Oreo%20Phone%20Tester/2097 

 https://crbug.com/907602  doesn't seem to be addressed.

Original change's description:
> Reland "Android: Factor InProductHelp UI out of Tab"
> 
> 
> https://chromium-review.googlesource.com/c/chromium/src/+/1272779 had a couple
> of bugs in |ToolbarButtonInProductHelpController|:
> 
> 1) Ignored |profile| passed into |maybeShowDownloadContinuingIPH()| and
>    always used Profile.getLastUsedProfile()
> 2) |OnDismissListener| was instantiated (and menu got highlight for
>    the text bubble) every time |setupAndMaybeShowIPHForFeature| is called,
>    even when the method doesn't meet the condition for IPH text bubble
>    and has to early out.
> 
> The CL got reverted due to numerous regressions . This CL fixes them and
> relands it.
> 
> This reverts commit d436a7a2e33ee105a1d85370970fde3c88255302.
> 
> Bug: 889682, 907502, 907503, 907770, 907505,  907602 
> Change-Id: Id6de148a9fd4fd91ad7df986ca0ba78ed18e79e4
> Reviewed-on: https://chromium-review.googlesource.com/c/1349142
> Reviewed-by: Theresa <twellington@chromium.org>
> Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#614161}

TBR=twellington@chromium.org,jinsukkim@chromium.org

Change-Id: Id92d47e6ac65a0b1a50f4ad4cb1c0b9a11189993
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 889682, 907502, 907503, 907770, 907505,  907602 
Reviewed-on: https://chromium-review.googlesource.com/c/1366456
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614501}
[modify] https://crrev.com/fbe75ef510a9d9123f6c7f8cbd54d5f5b5680277/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
[modify] https://crrev.com/fbe75ef510a9d9123f6c7f8cbd54d5f5b5680277/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/fbe75ef510a9d9123f6c7f8cbd54d5f5b5680277/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarButtonInProductHelpController.java

Comment 13 by jinsuk...@chromium.org, Dec 7

Acted too soon :(

Would you mind doing me a favor of running the test locally with my patch, before me relanding it again and hoping it would work?

Comment 14 by bsheedy@chromium.org, Dec 7

Yeah, I can do that when I get into the office tomorrow morning.

Comment 15 by bsheedy@chromium.org, Dec 7

Seems to be running fine for me locally with your patch applied.

Comment 16 by jinsuk...@chromium.org, Dec 7

Thanks a lot Brian. Will reland it.

Comment 17 by bugdroid1@chromium.org, Dec 8

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

commit b643010d742b1323893685b3ded2250d684882c4
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Sat Dec 08 00:06:02 2018

Reland "Android: Factor InProductHelp UI out of Tab" again

This reverts commit fbe75ef510a9d9123f6c7f8cbd54d5f5b5680277.

Reason for revert: Reverted on a false alarm. Landing again.

Original change's description:
> Revert "Reland "Android: Factor InProductHelp UI out of Tab""
> 
> This reverts commit 76d39b3a8d8fbd0e990f92a992d80213724de9bd.
> 
> Reason for revert:  https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Oreo%20Phone%20Tester/2097 
> 
>  https://crbug.com/907602  doesn't seem to be addressed.
> 
> Original change's description:
> > Reland "Android: Factor InProductHelp UI out of Tab"
> > 
> > 
> > https://chromium-review.googlesource.com/c/chromium/src/+/1272779 had a couple
> > of bugs in |ToolbarButtonInProductHelpController|:
> > 
> > 1) Ignored |profile| passed into |maybeShowDownloadContinuingIPH()| and
> >    always used Profile.getLastUsedProfile()
> > 2) |OnDismissListener| was instantiated (and menu got highlight for
> >    the text bubble) every time |setupAndMaybeShowIPHForFeature| is called,
> >    even when the method doesn't meet the condition for IPH text bubble
> >    and has to early out.
> > 
> > The CL got reverted due to numerous regressions . This CL fixes them and
> > relands it.
> > 
> > This reverts commit d436a7a2e33ee105a1d85370970fde3c88255302.
> > 
> > Bug: 889682, 907502, 907503, 907770, 907505,  907602 
> > Change-Id: Id6de148a9fd4fd91ad7df986ca0ba78ed18e79e4
> > Reviewed-on: https://chromium-review.googlesource.com/c/1349142
> > Reviewed-by: Theresa <twellington@chromium.org>
> > Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#614161}
> 
> TBR=twellington@chromium.org,jinsukkim@chromium.org
> 
> Change-Id: Id92d47e6ac65a0b1a50f4ad4cb1c0b9a11189993
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 889682, 907502, 907503, 907770, 907505,  907602 
> Reviewed-on: https://chromium-review.googlesource.com/c/1366456
> Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
> Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#614501}

TBR=twellington@chromium.org,jinsukkim@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 889682, 907502, 907503, 907770, 907505,  907602 
Change-Id: I1cdb497251b4a1f60f955cc6fd90c2be1eae3e96
Reviewed-on: https://chromium-review.googlesource.com/c/1368844
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614875}
[modify] https://crrev.com/b643010d742b1323893685b3ded2250d684882c4/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
[modify] https://crrev.com/b643010d742b1323893685b3ded2250d684882c4/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/b643010d742b1323893685b3ded2250d684882c4/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarButtonInProductHelpController.java

Comment 18 by jinsuk...@chromium.org, Dec 9

Status: Fixed (was: Assigned)

Sign in to add a comment