New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 665182 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

CastPositionTransferTests failing on Marshmallow recently

Project Member Reported by donnd@chromium.org, Nov 14 2016

Issue description

avayvod@, can you take a look or assign to a better owner?

Something caused several CastPositionTransferTest test cases to start failing recently on Marshmallow machines.  Seems like some change on 11/11 between 9 and 11:00 caused these tests to fail, but it's not obvious what the change was, or whether it's related to bot health, etc.

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=chrome_public_test_apk&tests=CastPositionTransferTest


I  989.698s run_tests_on_device(0ac4bb5f)  detected failure in org.chromium.chrome.browser.media.remote.CastPositionTransferTest#testPositionUpdate. raw output:
I  989.698s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS: numtests=1
I  989.698s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS: stream=
I  989.698s run_tests_on_device(0ac4bb5f)    org.chromium.chrome.browser.media.remote.CastPositionTransferTest:
I  989.698s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS: id=InstrumentationTestRunner
I  989.699s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS: test=testPositionUpdate
I  989.699s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS: class=org.chromium.chrome.browser.media.remote.CastPositionTransferTest
I  989.699s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS: current=1
I  989.699s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS_CODE: 1
I  989.699s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS: numtests=1
I  989.699s run_tests_on_device(0ac4bb5f)    INSTRUMENTATION_STATUS: stream=
I  989.699s run_tests_on_device(0ac4bb5f)    Failure in testPositionUpdate:
I  989.699s run_tests_on_device(0ac4bb5f)    junit.framework.AssertionFailedError: Criteria not met in allotted time.
I  989.699s run_tests_on_device(0ac4bb5f)    	at org.chromium.content.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:74)
I  989.699s run_tests_on_device(0ac4bb5f)    	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:112)
I  989.699s run_tests_on_device(0ac4bb5f)    	at org.chromium.chrome.browser.media.RouterTestUtils.waitForView(RouterTestUtils.java:98)
I  989.700s run_tests_on_device(0ac4bb5f)    	at org.chromium.chrome.browser.media.RouterTestUtils.waitForRouteButton(RouterTestUtils.java:35)
I  989.700s run_tests_on_device(0ac4bb5f)    	at org.chromium.chrome.browser.media.remote.CastTestBase.castVideo(CastTestBase.java:310)
I  989.700s run_tests_on_device(0ac4bb5f)    	at org.chromium.chrome.browser.media.remote.CastTestBase.castVideoAndWaitUntilPlaying(CastTestBase.java:280)
I  989.700s run_tests_on_device(0ac4bb5f)    	at org.chromium.chrome.browser.media.remote.CastPositionTransferTest.testPositionUpdate(CastPositionTransferTest.java:147)
I  989.700s run_tests_on_device(0ac4bb5f)    	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
I  989.700s run_tests_on_device(0ac4bb5f)    	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
I  989.700s run_tests_on_device(0ac4bb5f)    	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
I  989.700s run_tests_on_device(0ac4bb5f)    	at org.chromium.chrome.test.ChromeActivityTestCaseBase.runTest(ChromeActivityTestCaseBase.java:726)
I  989.700s run_tests_on_device(0ac4bb5f)    	at org.chromium.base.test.BaseTestResult.runParameterized(BaseTestResult.java:161)
I  989.700s run_tests_on_device(0ac4bb5f)    	at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:124)
I  989.700s run_tests_on_device(0ac4bb5f)    	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
I  989.700s run_tests_on_device(0ac4bb5f)    	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
I  989.701s run_tests_on_device(0ac4bb5f)    	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
I  989.701s run_tests_on_device(0ac4bb5f)    	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1879)

 

Comment 2 by xhw...@chromium.org, Nov 14 2016

Components: -Internals>Media>Encrypted Internals>Cast
Doesn't seem to be encrypted-media related.

Comment 3 by donnd@chromium.org, Nov 15 2016

avayvod@ or xhwang@, can you please triage and assign?  Thanks!
- Android Sheriff for the day.
Cc: avayvod@chromium.org mlamouri@chromium.org
Components: -Internals>Cast Internals>Cast>MediaFling
Owner: zqzh...@chromium.org
Status: Assigned (was: Untriaged)
Zhiqiang, could you take a look?
I think device discovery was fine, see the following logcat:

98316:  11-11 19:07:52.996  3282  3282 D cr_MediaFling: [RemoteMediaPlayerBridge.java:241] onPlayerCreated
98316:  11-11 19:07:52.996  3282  3282 D cr_MediaFling: [AbstractMediaRouteController.java:228] Started device discovery
98316:  11-11 19:07:52.997  3282  3282 D cr_MediaFling: [RemoteMediaPlayerBridge.java:322] onRouteAvailabilityChange: false, false
98316:  11-11 19:07:53.023 28110 28110 I cr_TestMRP: discoveryRequestChanged : DiscoveryRequest{ selector=MediaRouteSelector{ controlCategories=[com.google.android.gms.cast.CATEGORY_CAST_REMOTE_PLAYBACK/CC1AD845] }, activeScan=false, isValid=true }
98316:  11-11 19:07:53.023 28110 28110 I cr_CastEmulator: publishing routes
98316:  11-11 19:07:53.027  3282  3282 D cr_MediaFling: [AbstractMediaRouteController.java:425] Added route Cast Test Route org.chromium.chrome.tests.support/org.chromium.chrome.browser.media.TestMediaRouteProviderService:variable_session
98316:  11-11 19:07:53.028  3282  3282 D cr_MediaFling: [AbstractMediaRouteController.java:86] Remote media route availability changed, updating listeners
98316:  11-11 19:07:53.028  3282  3282 D cr_MediaFling: [RemoteMediaPlayerBridge.java:322] onRouteAvailabilityChange: true, false
98316:  11-11 19:07:53.033  3282  3282 D cr_MediaFling: [RemoteMediaPlayerBridge.java:322] onRouteAvailabilityChange: true, true
98316:  11-11 19:07:53.306  3282  3295 I cr_CastTestBase: castVideo, videoRect = Rect(8, 8 - 508, 383)

Then it gets stuck at waiting for the Cast dialog and timed out. However I see RouterTestUtils prints logs when waiting for the dialog but  I see none in the logcat.
I'll investigate more.
I tried locally and can reproduce it. Manually tapping the cast button can let the test pass. I believe it is a result of recent change in media controls (maybe the cast button position was changed slightly).

This also applies to other Cast test failures.
Ping, do you have a fix? This is still failing.
Status: Started (was: Assigned)
Hi, I'm working on it but still haven't got a solution yet. Will try harder.
Mergedinto: 623526
Status: Duplicate (was: Started)
Labels: -Pri-3 Pri-2
Status: Started (was: Duplicate)
Maybe it's not a duplicate of #623526 (where the tests became flaky a long time ago).

I'm sending a fix which hopefully solves both.
Cc: mustaq@chromium.org
Labels: M-57
I'm moving the input event discussion in #623526 here, since it's more related to this bug. Posting original message from mustaq@:

We recently changed the mouse behavior in Android M (and later) to fix "half-baked" mouse event dispatches. Here is the PSA we sent out:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/cNaFvMaYtNA/AQYz_FQlAwAJ

It seems that change by us caused this bug but I don't have enough background to comment. Let me start with some questions to clarify the importance of a mouse input here:

A. WhyRouterTestUtils is "simulating touch" with TOOL_TYPE_MOUSE? Do the users use any mouse-type input for your device/platform? Or the device is touch-only, and the TOOL_TYPE_MOUSE was chosen only to ease testing?

B. The CL description mentions that the cast button responds to TouchEvents. In case it was meant to work with both touch & mouse, it should need a few MouseEvent listeners for an ideal fix. Could you please point us to the JS code?
A. I think we don't care about TOOL_TYPE_MOUSE that much (it was from some legacy tests). We simulate a click/touch and expect the cast button being clicked.

B. The implementation is in C++, but is equivalent implementing it in C++. The code is MediaControlCastButtonElement::defaultEventHandler():
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp?l=935

The CastButton is a HTMLInputElement, and the event handler response to "click" event. I wonder why it does not receive a click if it's a mouse click, while finger tap works. 
Steps to reproduce the failure:

1. Apply https://codereview.chromium.org/2518063006/ (or wait after it's landed)
2. s/TOOL_TYPE_FINGER/TOOL_TYPE_MOUSE/ in RouterTestUtils.java
3. Run:
out/Debug/bin/run_chrome_public_test_apk_incremental -f CastStartStopTest#testCastingGenericVideo 

and see if it fails. You may want to log MediaControlCastButtonElement::defaultEventHandler() to check if the Cast button receives the event.
Cc: aber...@chromium.org
Anthony (aberent@, CCed) might remember why we used TOOL_TYPE_MOUSE vs TOOL_TYPE_TAP in the Cast instrumentation tests?
I couldn't remember, but "git log -G" (on clank) found this for me:

commit e59d9475b1a78aac70a64361c75f1e926b691079
Author: Anthony Berent <aberent@google.com>
Date:   Mon Jan 12 15:13:38 2015 -0800

    Avoid invoking disambiguation pop-up in cast tests
    
    This re-enables the cast tests, disabled by commit
    b93c0feffdcd62ef1736695a614359c2f035184d, and modifies them to use
    virtual mouse clicks instead of virtual finger touches. Android
    and Chrome treat mouse clicks as precise, so we never get the
    disambiguation pop-up. Getting this pop-up was previously causing
    the tests to fail.
    
    BUG=448186
    
    Change-Id: I83fc8c1a2127694744798878fcbca6eab94e7609
    Reviewed-on: https://chrome-internal-review.googlesource.com/191746
    Reviewed-by: Nicolas Dossou-gbete <dgn@google.com>
    Commit-Queue: Anthony Berent <aberent@google.com>
    Tryjob-Request: Anthony Berent <aberent@google.com>
    Tested-by: clank-autoroller <clank-buildbot@google.com>

This seems to make sense, even if it is arguably a bit of a hack. We would need another way of avoiding the pop-up to change this. 

Hmm, maybe the old bug is no longer applicable (since there are lots of changes to MediaControls)? Not very sure though.

If the bots are happy, we should probably switch back to use TestTouchUtils instead.
I don't think the problem is specific to the media controls, it depends on the distance between touch targets and screen size vs. assumed finger size. As such using touch could cause flakiness in future.

Why doesn't mouse input work? Although mouse input on Android is very rare, my understanding is that it is supposed to be supported.

I have always rather disliked the way we do this. Ideally we would access these buttons at a slightly lower level, not based on the precise geometry of the media controls, but since only the renderer knows about them I don't see any way of doing so without adding some major plumbing to Chrome (and/or some ability to access the buttons from Javascript).
Project Member

Comment 18 by bugdroid1@chromium.org, Nov 24 2016

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

commit b295326bdf5fd50fd4c782c7dda3024b426a5521
Author: zqzhang <zqzhang@chromium.org>
Date: Thu Nov 24 16:37:06 2016

Fixing MediaFling tests

The MediaFling tests started failing due to an issue related to
injecting MouseEvents, which causes the cast button not receiving
touch event, therefore the tests timed out waiting for the Cast
route selection dialog. This CL changes the test util from using
TOOL_TYPE_MOUSE to TOOL_TYPE_FINGER, which fixes the issue.

This CL also re-enables tests that were disabled/marked as flaky
due to this touch event issue.

BUG=623526, 665182

Review-Url: https://codereview.chromium.org/2518063006
Cr-Commit-Position: refs/heads/master@{#434346}

[modify] https://crrev.com/b295326bdf5fd50fd4c782c7dda3024b426a5521/chrome/android/javatests/src/org/chromium/chrome/browser/media/RouterTestUtils.java
[modify] https://crrev.com/b295326bdf5fd50fd4c782c7dda3024b426a5521/chrome/android/javatests/src/org/chromium/chrome/browser/media/remote/CastStartStopTest.java
[modify] https://crrev.com/b295326bdf5fd50fd4c782c7dda3024b426a5521/chrome/android/javatests/src/org/chromium/chrome/browser/media/remote/CastSwitchVideoTest.java
[modify] https://crrev.com/b295326bdf5fd50fd4c782c7dda3024b426a5521/chrome/android/javatests/src/org/chromium/chrome/browser/media/remote/CastVideoControlsTest.java
[modify] https://crrev.com/b295326bdf5fd50fd4c782c7dda3024b426a5521/chrome/test/data/android/media/simple_video2.html
[modify] https://crrev.com/b295326bdf5fd50fd4c782c7dda3024b426a5521/chrome/test/data/android/media/two_videos.html

Yeah, it should affect other tests using TestTouchUtils.java (which actually use TOOL_TYPE_UNKNOWN).

Maybe the our Cast button is smaller than buttons in other tests so we have a greater chance of opening the pop-up if we simulate a tap instead of a mouse click.

mustaq should have better answer on mouse click vs. tap.
The recent change to Android mouse event plumbing fully supports mouse clicks, and in fact makes them better for right & middle clicks:
https://docs.google.com/document/d/1mpBR7J7kgTXvp0QACVjhxtwNJ7bgGoTMmxfxN2dupGg/edit#heading=h.yzj7hbxto2vr

So your tests should be unaffected with TOOL_TYPE_MOUSE. Let me look closely.

Looks like we don't need to change the TOOL_TYPE_MOUSE in RouterTestUtils, instead have RouterTestUtils.mouseSingleClick() fire ACTION_BUTTON_PRESS and ACTION_BUTTON_RELEASE. We are now relying on these actions in Android M+ to fire mosuedown/mouseup because these new actions better represent mouse button change which is crucial for JS MouseEvents (so ACTION_DOWN/ACTION_UP no longer fires mousedown/mouseup).

If RouterTestUtils is used in any tests that are run on old Android versions (L or older), you will need the existing ACTION_DOWN/ACTION_UP too. In that case, you may want to have mouseSingleClick() mimic real Android event order (IIRC, ACTION_BUTTON_PRESS then _DOWN then _BUTTON_RELEASE then _UP).

Blocking: 669115
Seems the tests are getting more flaky than before. I'll try mustaq's solution and see if it works.
mustaq, I tried firing _BUTTON_RELEASE and then _UP with TOOL_TYPE_MOUSE in RouterTestUtils locally. However the click event is not received in DOM again.
Did you mean _BUTTON_PRESS + _BUTTON_RELEASE instead?
Yes. I tried the following:

1. s/TOOL_TYPE_FINGER/TOOL_TYPE_MOUSE
2a. _BUTTON_PRESS -> _BUTTON_RELEASE  *doesn't work*
2b. _BUTTON_PRESS -> _DOWN -> _BUTTON_RELEASE -> _UP  *doesn't work either*

The last one (2b) is like real Android-M sequence, not sure why it's failing.

Do you see any problem outside these tests? I mean through manual interaction?
I'm getting a USB-to-MicroUSB converter and see if I can reproduce it with a real mouse. Will let you know after I done it.
Got the adapter and did manual test. The cast button can receive click event when I initiate a manual mouse click.

Maybe I can do bisect to check which change causes the issue (when I have free time).
Thanks for confirming that clicks work in manual tests. Assuming this is after your change# 2b above (right?), there's a chance that the ACTION_BUTTON* events from RouterTestUtils is getting ignored latter in the test pipeline.
Ping; any update on status here?
Blocking: -669115
Cc: -mustaq@chromium.org
The connection to Android mouse event change I suspected above seems wrong: the mouse event change affects only the C++ side of Chrome for Android.
Labels: -M-57
Owner: mlamouri@chromium.org
Owner: avayvod@chromium.org
Status: Assigned (was: Started)
avayvod@, assigning to you for triaging.
Owner: mlamouri@chromium.org
Status: Untriaged (was: Assigned)
Assigning all my bugs to Mounir for him to triage and close/reassign later.
Status: Assigned (was: Untriaged)
Assigning to Mounir for decision so that these get out of the general untriaged bucket.  
Components: Tests>Disabled
Labels: Test-Disabled
Owner: ----
Status: Available (was: Assigned)
Components: Tests>Flaky

Sign in to add a comment