related? https://bugs.chromium.org/p/chromium/issues/detail?id=786638#c4

I didn't check if this one is waiting for a result of a DOMUtil click though

Seems to only be timing out on the Lollipop phone tester bot.

Disabling this test for now and removing from sheriff queue since the bug is assigned.

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

commit 532c0b9a129883b3caa4f0f39da7ff39d8c84f45
Author: Peter Kasting <pkasting@chromium.org>
Date: Thu Dec 07 23:44:09 2017

Disable flaky test.

Bug:  789306 
Change-Id: I909db9bbc0373af1170f7a6471e9795cc6e0a3b4
TBR: ntfschr
Reviewed-on: https://chromium-review.googlesource.com/815289
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522620}
[modify] https://crrev.com/532c0b9a129883b3caa4f0f39da7ff39d8c84f45/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java

I'll try running this test overnight on my Nexus 6 (Android L)

I can repro the timeout locally now. I'm running into some issues switching to JS over DOMUtils--if I can't resolve my problems, I'll reenable the test with @RetryOnFailure since the flake seems pretty rare locally.

 Issue 797197  has been merged into this issue.

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

commit 08696cc241a2a8df1134322a4ba3d2e547063e51
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Thu Dec 28 00:42:34 2017

Disable flaky test

testFullscreenForNonVideoElementIsSupportedInSoftwareMode is flaky,
disabling it.

Bug:  797197 ,  789306 
Change-Id: I61d5e2cb4458fe5c6346a1f80398831c7dd9691e
TBR: ntfschr@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/845147
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526257}
[modify] https://crrev.com/08696cc241a2a8df1134322a4ba3d2e547063e51/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java

 Issue 808444  has been merged into this issue.

Not actively looking at this, someone else from the team should take this if they have bandwidth.

Nate, I'm afraid that we may end up getting regression if we keep these tests disabled. Could you spend some time investigating this and other DOM util related flakiness?

Also cc'ing tobiasjs@ and boliu@ from issue 791019

Also, issue 618749...

Anyone object to removing RBD? Rationale:

 * WebView Dev population is extremely small. I think it's worse (in this particular case) to block a clank dev release than it would be to release a potentially regressed WebView
 * Test was disabled a while ago, not during M71 (that's just an arbitrary milestone)
 * I have no reason to particularly suspect regressions in this code area; there is not a particularly high risk for regression

I'm still intending to focus on this, but it'll have to wait until I'm back from OOO (or, any team member may steal this from me).

No objection. Thanks!

Any update on this?

Haven't had time to look at this. I'll dedicate some time during makeitbetter week.

Took a look today. I'm trying to switch to JS, but blink checks for user gesture [1], and I haven't found a way to turn this off/mock this.

Some questions I'd like to answer:
 1. When DOMUtils flakes, is it because 'user gesture' is false? Or some other reason?
 2. If I hardcode user-gesture to true, is JS just as flaky as DOMUtils, or is it viable?

I can't get answers until tomorrow, because it can take hours to produce a flake locally...

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/user_gesture_indicator.cc?l=131&rcl=eb28e7e34aba87013ba804e64faa214cce5caf94

Update on #2: I think JS is indeed less flaky (it passed 1000 consecutive runs).

I think I have a prototype which switches to JS, but it plumbs ExecuteJavaScriptWithUserGestureForTests() [1] to Java, which might have security concerns (although, we already plumb ExecuteJavaScriptForTests, so perhaps not much worse). I'm running this tonight to verify if this really fixes flakiness before proceeding.

Alternatives:

 1. We could try using TouchCommon#singleClickView(View v, int x, int y). This requires that we can get the X/Y coordinates for the custom fullscreen button--I'm not sure if we can.
 2. We could plumb ExecuteJavaScriptWithUserGestureForTests to Java, but via a testonly build target. This means it won't be compiled into the product, and the linker might even strip the native method (assuming it's not called other places). This is probably "better" but involves quite a bit more work (jinsukkim@ attempted this in http://crrev/c/1203490, but was reverted).

[1] https://cs.chromium.org/chromium/src/content/public/browser/render_frame_host.h?l=191&rcl=c073209954ab8bd9893bbeeaa0ff9c526ed2d560

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

commit 52c77521913da72057f84f159b21fc68c8808a04
Author: Nate Fischer <ntfschr@chromium.org>
Date: Wed Oct 17 22:45:14 2018

AW: fix and enable flaky test

This enables
testExitFullscreenEndsIfAppInvokesCallbackFromOnHideCustomView. This
test flaked because DOMUtils was unreliable for clicking links. This CL
swaps out DOMUtils for EvaluateJavaScriptWithUserGesture (since the
relevant blink code requires user gesture and has no way to turn it off
for testing).

This plumbs EvaluateJavaScriptWithUserGesture to Java in a testonly
target, so the Java method will not be present in production (and so
this change should have no security implications).

R=boliu@chromium.org

    --repeat=1000 \
    --gtest_filter=AwContentsClientFullScreenTest#testExitFullscreenEndsIfAppInvokesCallbackFromOnHideCustomView

Bug:  789306 
Test: run_webview_instrumentation_test_apk --break-on-failure \
Change-Id: I4407ce0daa399fbb289d4c9d74e3d9b64bccfcbf
Reviewed-on: https://chromium-review.googlesource.com/c/1286031
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600579}
[modify] https://crrev.com/52c77521913da72057f84f159b21fc68c8808a04/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
[modify] https://crrev.com/52c77521913da72057f84f159b21fc68c8808a04/android_webview/javatests/src/org/chromium/android_webview/test/util/JSUtils.java
[modify] https://crrev.com/52c77521913da72057f84f159b21fc68c8808a04/android_webview/test/BUILD.gn
[modify] https://crrev.com/52c77521913da72057f84f159b21fc68c8808a04/content/public/test/android/javatests/src/org/chromium/content_public/browser/test/util/WebContentsUtils.java
[modify] https://crrev.com/52c77521913da72057f84f159b21fc68c8808a04/content/public/test/android/web_contents_utils.cc

The following tests were all disabled around the same time:

 1. testExitFullscreenEndsIfAppInvokesCallbackFromOnHideCustomView
 2. testFullscreenForNonVideoElementIsSupportedInSoftwareMode
 3. testPowerSaveBlockerIsTransferredToFullscreen

I'll enable the other 2 locally and look for flakes.

^ The other two also seem improved, so I've sent http://crrev/c/1290142 for review. After that lands, we can probably close this out.

Hmmm try-flakes seems to be deprecated. I'm trying to search for any flakes in the new replacement, and (as best as I can determine) it's flake-free. Given that, I'm going to land the CL to reenable the other tests.

URL is https://findit-for-me.appspot.com/ranked-flakes?test_filter=AwContentsClientFullScreenTest

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

commit 96c0be24a293ae3dd08592cfe6496729cd2636f9
Author: Nate Fischer <ntfschr@chromium.org>
Date: Mon Oct 22 18:00:26 2018

AW: enable more tests

No change to production logic.

This enables 2 tests which were previously disabled due to flakiness.
The flakiness was fixed in http://crrev/c/1286031, and these appear safe
to enable:

 * testFullscreenForNonVideoElementIsSupportedInSoftwareMode
 * testPowerSaveBlockerIsTransferredToFullscreen

This also cleans up annotations on other disabled tests. It's sufficient
to mark disabled tests as "@DisabledTest" without commenting out other
annotations. It's helpful to actually leave these annotations
uncommented, because it makes enabling the test a one-line change.

Bug:  789306 
Test: run_webview_instrumentation_test_apk \
Test: --gtest_filter=AwContentsClientFullScreenTest#* \
Test: --repeat=1000
Change-Id: I497718177d086c46e1553e1f365b23b27de760e4
Reviewed-on: https://chromium-review.googlesource.com/c/1290142
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601647}
[modify] https://crrev.com/96c0be24a293ae3dd08592cfe6496729cd2636f9/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java

I'll follow up end of this week and look for flakes again. Otherwise, I think this is fixed.

[Auto-generated comment by a script] We noticed that this issue is targeted for M-71; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-71 label, otherwise remove Merge-TBD label. Thanks.

These tests were flaky because of a bad testonly dependency, not because of a product defect. Therefore, no merge necessary.

The NextAction date has arrived: 2018-10-26

Looks like the changes stuck!