V56 work well.Nexus 5x n71,M1 n71 also have this issue.

satyavathir@, please assign or repro then bisect.  Possibly related to https://bugs.chromium.org/p/chromium/issues/detail?id=723576

another one: https://bugs.chromium.org/p/chromium/issues/detail?id=726109  dupe them to the original bug if bisect is same

battun@,
Could you check this issue? if it repro's, could you bisect and update?
Thanks!

Issue able to repro on M58:58.0.2995.0 

Bisect Information:
Per-Version bisect information:
Good build: 58.0.2993.0
Bad build:  58.0.2995.0

Regression range: https://chromium.googlesource.com/chromium/src/+log/58.0.2993.0..58.0.2995.0?pretty=fuller&n=10000

Note:  can't to do per-cl bisect due to this error"There are problems with the security certificate for this site"

Thanks! 

I would imagine this being related to Philip's revert (https://chromium.googlesource.com/chromium/src/+/675e4f2fe55c349ac3ba67405c15c36ec7419cb6) w.r.t. fullscreen behaviour.
Philip, I'm assigning this to you, if you think that's incorrect. please un-/re-assign.

"If chrome version is v56" suggests that this is a revert between stable versions, and https://chromium.googlesource.com/chromium/src/+/675e4f2fe55c349ac3ba67405c15c36ec7419cb6 reverted something that never reached stable.

battun@, can you repeat the bisect, but between 56 and just before https://chromium.googlesource.com/chromium/src/+/e1d42d636990425056ede44086ef49a1f8c6a0a5?

foolip@ issue doesn't repro on M56:56.0.2924.99, M57:57.0.2987.97 i have seen Good build: 58.0.2993.0 and Bad build:  58.0.2995.0.

Thanks!

So, it sounds like the problem did not exist before https://chromium.googlesource.com/chromium/src/+/e1d42d636990425056ede44086ef49a1f8c6a0a5, but was introduced by its revert in https://chromium.googlesource.com/chromium/src/+/675e4f2fe55c349ac3ba67405c15c36ec7419cb6.

One possibility is that it was introduced by some change in between, but masked by the changes that were then reverted. To determine that would require bisecting by trying to revert the changes at various points between the two commits.

Unfortunately, I am about to go OOO July 10-28, and will not be able to fix this in the two remaining days.

There is no commit to revert that'll fix it instantly, but I would suggest:
1. Check if it was fixed by https://chromium.googlesource.com/chromium/src/+/81e33d3b1082212c0cc3732719167004dcfb63fc, which is a reland but with many changes. Probably not.
2. Start debugging from the basics. Are any JavaScript console errors thrown? Are Fullscreen::RequestFullscreen and FullscreenController::EnterFullscreen reached? Why not?

We are seeing the same issue in Firefox Focus [1] which is based on WebView.

Some additional information:

* The callback for "exit full screen mode" (onHideCustomView) is called immediately after the "enter full screen mode" (onShowCustomView) [WebChromeClient]. Often you can see that the video is in full screen mode for a split second.

* This only happens on YouTube. Other sites with video content like vimeo.com or ted.com just work.

* In rare cases onHideCustomView() is not called immediately on YouTube and the video stays in full screen mode. However in this situation the video player button for leaving full screen mode does not work (as if it's not expected to actually be in full screen mode). 

* As this is only observed on YouTube I'm not sure if this is a WebView issue or maybe something the YouTube website does accidentally?

* I do not see this issue when using Chrome directly.

[1] https://github.com/mozilla-mobile/focus-android

Removing 'Needs-bisect' label, as it has been updated by battun@ few times for regression range and other information. 

I find that the video enters fullscreen if I try clicking the button 1-3 times. Is this consistent with what others see?

ntfschr@,
#18- yes, that is the behavior we see with YouTube in many apps on M61 builds.

I'm able to repro this issue on webview shell browser/Du Browser/Doplhine browser especially on m.youtube.com, where trying to make the video full screen, by pressing the square button, the screen just flicker and goes back to regular screen.

this issue repro on webview : 61.0.3162.0 (on most of the device Android M/N/O)

Apps: webview shell browser/Du browser/dolphine

this issue repro on landscape and portrait mode

STEPS:-
1. URL -> m.youtube.com
2. Select any video > play > press the square button to make it for full screen.

Results:-
The screen just flicker and come back to regular screen

Note: 
I don't see this issues on Chrome.

i'll try this locally, probably without a video element.  looking at the CL in #15, i don't see anything that's deeply media related.

Passing off to liberato@, please pass this back if this isn't your area.

i'm able to repro this also locally, but it takes several tries to get fullscreen to fail:

- while playing, it works once (or more).  eventually, it stops working no matter how many times i press the fullscreen button.

- if i pause it, then it starts behaving badly immediately.

- if i start playback again, then i get another 1 or more transitions to fullscreen that work as expected.

when i use a simple src= video, i've never seen it fail.  i'll dig a little more.

when it works properly, i see roughly these events:

- (lots of stuff that's the same between working and broken cases)
- resize is dispatched
- HTMLMediaElement::DidEnterFullscreen
- webkitfullscreenchange is dispatched

in the broken case:
- (lots of stuff...cases)
- HTMLMediaElement::DidEnterFullscreen
- resize is dispatched
- blazer:orientatation_change_0 is dispatched
- FullscreenManager:ExitFullscreen is called to initiate the transition back out of fullscreen.

i'm going to verify if ExitFullscreen is coming from js.  i think so, since i annotated all the other places that it might come from.

ExitFullscreen does seem to be coming from JS.

i'm pretty sure that this is unrelated to media.  in working traces, the resize and webkitfullscreenchanged messages are present, but missing / reordered in the non-working traces.  i suspect that something's racing, somewhere.

no idea what's going on, but whatever it is is causing the site to exit fullscreen.

interestingly, WebViewImpl is being notified of a browser-side resize at the same point in each trace before the first resize (didn't check the actual resulting size, though).  so, unclear that it's a browser -> renderer timing issue causing the first place where the traces differ.

back over to ntfschr@ .

Thanks, liberato@

I won't have time to investigate today (the team is moving buildings). Putting this back in our bug queue. timav@ can you investigate next week?

can the next bug cop look at this please? QA team is worried in full screen regressions.

I am the next bug cop.

I will take a look.

I was able to reproduce on marlin-userdebug OMR1 OPM1.170726.001 4217084 dev-keys and Du browser, latest ToT (62.0.3169.0). I had to press pause and resume.

blink::DocumentFullscreen::exitFullscreen(blink::Document&) was called by V8.

Line numbers are slighly off.

blink::Fullscreen::ExitFullscreen(blink::Document&)
/usr/local/google/code/clankium/src/out_webview_o/Debug/../../third_party/WebKit/Source/core/fullscreen/Fullscreen.cpp:640
blink::DocumentFullscreen::exitFullscreen(blink::Document&)
/usr/local/google/code/clankium/src/out_webview_o/Debug/../../third_party/WebKit/Source/core/fullscreen/DocumentFullscreen.cpp:48
v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
/usr/local/google/code/clankium/src/out_webview_o/Debug/../../v8/src/api-arguments.cc:25
v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments)
/usr/local/google/code/clankium/src/out_webview_o/Debug/../../v8/src/builtins/builtins-api.cc:112
v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*)
/usr/local/google/code/clankium/src/out_webview_o/Debug/../../v8/src/builtins/builtins-api.cc:142
v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*)
/usr/local/google/code/clankium/src/out_webview_o/Debug/../../v8/src/builtins/builtins-api.cc:130

Two more observations:
1. Regular <video src="..." controls/> elements work fine.
2. To observe the issue on Youtube one does not have to have video playing, it can be paused as well.

Philip, I ran out of ideas. Could you please take a look?

This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

John, do you know who could be the best person to take a look at this issue?

I'll try to tackle this. timav@, when you reproduced this, did you build webview_instrumentation_apk, or what is marlin-userdebug? Looks from #33 like you had a debug build at least?

Also, why is this a M61 stable blocker? Per #15 it was rather introduced in M58.

My phone's system webview is 60.0.3112.97 and I can repro the problem with Firefox Focus. On the third try it works, but then it's hard to exit again.

Unfortunately, with WebViewInstrumentation.apk (following https://www.chromium.org/developers/how-tos/build-instructions-android-webview) the fullscreen button doesn't appear, so still needs more work to repro with a build of my own.

Philip, thank you for looking into this!
No, I used system_webview_shell_apk. It uses the system WebView, so I also built and installed monochrome_apk (this is what we normally do). I'm not sure whether you would be able to build one: you'd need signing_keys/ directory. I can build Monochrome for you though.
sailfish-userdebug or marlin-userdebug.

c#42: I put RBS because thought the issue is serious enough to be fixed in the next build. Usually it is our TPM and the testing team who determines this status though.

OK, I've tried building and install monochrome_public_apk myself, but it won't show up under "WebView implementation" in the system settings. I think I either need to build monochrome_apk (internal repo?) or have a userdebug version of Android, or both.

To perhaps get some help understanding what YouTube is trying to do here, I've filed the internal b/64665165.

You need a userdebug build to use any non-Google-signed WebView implementation. monochrome_public_apk should work fine on a userdebug build.

Thanks, I now have a Nexus 6P with a userdebug build, trying to make progress on this again.

Well, just building monochrome_public_apk and installing on that Nexus 6P isn't enough to have it show up under the "WebView implementation" chooser in the settings. I've requested the access I need to get the signing_keys/ directory, but will that be enough? timav@, were you building from the vanilla Chromium repo, or do I need src-internal? monochrome_public_apk or monochrome_apk?

...oops. Sorry, I'm wrong; monochrome_public_apk just has the wrong package name and won't work no matter what. You either need to build monochrome_apk or override the package name of the public version.

The signing keys aren't relevant here - on userdebug builds the signature is not checked (unless you are trying to install over an existing package, in which case it must match the existing package as is usual for all APKs). On a user build where the signatures are checked, having the signing_keys repo won't help because that's only the dev signing keys and user builds require the release signing keys (i.e. the builds have to come from the official buildbots).

I never tried to build WebView without signing_keys. I built monochrome_apk.
I believe you do not need src-internal, but I think you need src/clank. Do you have it?

Thanks, I have a clank checkout that at least has monochrome_apk as a target, so I'll give that a shot tomorrow without the signing keys first. I have a userdebug build, and it did seem odd that I still would need any special keys, hopefully that's not so then.

Yay, it works!

torne@, do you know where I could file a bug (on Android?) so that using monochrome_public_apk would also work? If a non-Googler causes a regression like this, it seems like a very difficult situation to debug.

Short answer: it's not really possible to make that work in a way that's actually reasonable (and if you filed a bug about it, it would go to me, probably) :)

End user android devices (i.e. not running custom roms) are never able to load a custom webview implementation at all, no matter what you do, by design (since webview is in the privileged position of its code being executed in a majority of applications and thus can be used to compromise the application sandbox). User builds of android require that any potential WebView implementation be either preinstalled on the device or signed by a known key (and these keys are the release keys for the various Chrome channels).

Userdebug/eng images of android do not have these restrictions since the shell user has root access anyway and so attempting to enforce security restrictions is pointless - however, external people don't have access to official builds of this type (Google and other phone vendors do not distribute userdebug system images for devices), and if you're building your own Android you can specify any webview configuration you want already anyway at build time (it's just a straightforward XML config file).

The only case where there's really an easier option is for Googlers who have access to the android flashstation builds to install a userdebug image on their device, and those googlers can already request access to the internal chrome repo and build monochrome_apk, so supporting monochrome_public_apk as well is of limited value and just makes the configuration larger (which has some small cost at runtime) for little benefit.


If you want monochrome_public_apk to work the easiest thing to do is not to change android, but to just introduce a GN configuration parameter to override the package name, and simply set it to the package name of the non-public APK when building. If you have a userdebug/eng build of android that will work fine; the package name is all that's significant here.

Hmm, so I am indeed in the category of "Googlers who have access to the android flashstation builds to install a userdebug image on their device" and I spent some time wondering why monochrome_public_apk wouldn't work, setting up a new repo and so on.

I take it that there isn't currently a GN configuration parameter to set the package name, but that this is all that would be required to make this work, then?

So, reporting on the debugging progress here. Good news is that I can install my own WebView and reproduce the problem, but the bad news is that I can't build one from commit e3783fbf9451ae9369aec0ac400356ccb3123e1c from which I wanted to start a bisect. The problem was "Unresolved dependencies" during gn gen, relating to remoting dependencies.

So, I think I'll give up on that bisect and instead try to understand it based on what currently on master. However, I will put this on hold for now until issue 746802 is fixed. If that means it'll be late for this for M61, I will do some more poking at b/64665165.

Hopefully issue 746802 does not take too long to get fixed. meanwhile, is there anyone else who can help fixing this issue since foolip@ is overloaded with another M61 RBS. M61 goes out in about 2 weeks and we cannot put a fix on hold at this time, unless that is the only option we have.

Issue 746802 is resolved now and I'm looking into this again.

Using a custom build, I've been able to confirm what could be assumed from #33, that the site is calling exitFullscreen (or a webkit-prefixed alias). If that method is turned into a no-op, it works. In other words, the exitFullscreen call isn't just coinciding with the problem, it's the cause.

Sure, but it's also a regression somehow...

If you're building using the internal repro you have to start by bisecting the internal repo, not src. The "root" repository is the internal one, which contains a pinned revision of src, and if you manipulate src separately it won't match and you'll get problems like the one you experienced :)

You'd need to bisect down to a particular DEPS roll of src, and then bisect src within the range of that roll to get to a particular src commit (analogously to bisecting src down to a third_party DEPS roll).

An update. In the internal YouTube bug I've learned that the exitFullscreen() call was from a resize event handler. The (revert of the) change was all about changing the timing and order to the fullscreenchange event, but it's still not clear what changed with the revert compared to when it was landed. A workaround on the YouTube side might happen, but I'll continue and try to bisect.

#64, I turned off managed mode, checked out a roll in src/clank/ and did gclient sync. There's a "unexpected AST node" error which requires --disable-syntax-validation to work around, and it does look like I end up in an inconsistent state. I'll see if I can manually check out some revisions that work.

comment #58,

yes there is a GN argument, i.e.  system_webview_package_name 
set it to
 "com.google.android.webview"

This bug is marked as RBS for M-61.  Please note that the last Beta is planned for this Wednesday which means changes must be merged into branch by Tuesday afternoon.  Please provide an update and get any fixes in asap.  Thanks.

#66, looks like that's the name used for system_webview_apk, so it doesn't help for monochrome_public_apk debugging.

#67, I don't no think that this should be an M61 release blocker, as it was not a regression in M61, and a fix on the YouTube side is likely in b/64665165.

I could put a lot more effort into getting monochrome_apk building from January (the time of the regression) building and bisecting to understand why the revert introduced a change that wasn't there before the original commit. However, it's rather time consuming and I don't feel it's a good use of time given that big refactoring changes to fullscreen have happened after this that have affected the order and timing of the very events that seem to be involved on the YouTube side.

So, I would rather mark this as ExternalDependency and not a release blocker.

I just checked the bug, and sounded like they could not do it (only read the last comment).

Does this only impact Youtube? any other websites?

Hey Satya, please help with some feedback here if this issue only affects Youtube

Removing blocker label per c#68 and this has been present since M58

let's make sure youtube works properly. At least make sure to get ok from youtube, IMO.

I have finally found an explanation of why this appeared as a regression from the revert even though it wasn't a problem before the original change. The code that exists is guarded by a version check and only runs for Chrome >=58. The original change was on M57, the revert on M58, so my suspicions in comment #15 were wrong.

I think the best path here is to work with YouTube on a fix on their side, and I've commented more on that bug.

The fix on Youtube side has been mailed here cl/167007227 and will go out next week. However, they suggested to add a way to explicitly detect PIP on our side . foolip@ is that something you can do? It will be nice to have.

I will go head and remove RBS from this issue since Youtube already did the necessary on their side.

I'm having the exact same problem in my trivial browser app using 60.0.3112.97 webview: a FrameLayout containing the WebView, onShowCustomView adds the custom view into the same FrameLayout, onHideCustomView removes it. Hopefully you guys have already figured out everything, but just in case, here's what I found out.

When switching a video to fullscreen, here's the sequence of events I've see (using a test web page):
1) webkitfullscreenchange event is fired: during JS handler execution, window.outerWidth == 0 and widow.outerHeight == 0
2) resize event is fired: during JS handler execution, window has still size 0 x 0
3) resize event is fired: during JS handler execution, window has the size of the custom view

The problem on Youtube, is that there seems to be a piece of JS code that is attached to resize events, compares the window area to the screen area, and if smaller than 1/3 of the screen area, calls exitFullscreen. Something equivalent to:

    if (((window.outerWidth * window.outerHeight)/(window.screen.width * window.screen.height)) < 0.33)
        document.exitFullscreen()

This code gets executed on step 2) above. And exiting fullscreen while the webview is actually in the process of going fullscreen seems to create problems in the webview. Sometimes the webview exits fullscreen, and then you see the video flickering. Sometimes the video gets fullscreen, but the webview doesn't seem to be in sync with that state and you can't get out of fullscreen (the button in the control bar doesn't do anything).

Note also that the above piece of JS code is not served to all browsers and seem to depend on the user agent. If I use standard mobile emulation in Chrome on my desktop, this code is not present. If I use the same user agent as my browser app, then the code is present.

#74, on the question of reliably detecting this situation I'll have to defer to mlamouri@, added to CC.

#75, that's very helpful, thank you. Are you saying that actually two resize events are fired?

#77, yes.

I tried to follow the code in android_webview/java/src/org/chromium/android_webview/ and I noticed the following (I'm really not familiar with chromium code base, so I might be completely wrong):
- AwWebContentsDelegateAdapter::enterFullscreen calls AwContent::enterFullScreen() to create the fullscreen view, at that moment the view is not attached yet to the app window, this will happen when onShowCustomView is called in the application code
- AwContents.enterFullScreen creates an instance of FullScreenView, set it up, then calls setContainerView()
- AwContents.setContainerView ends up calling onContainerViewChanged()
- AwContents.onContainerViewChanged calls awViewMethodsImpl.onSizeChanged(), and I believe that's what is triggering "resize" events in the webview; however at that point, the view is not yet attached to the app window and I believe that mContainerView.getWidth() and mContainerView.getHeight() both returns 0

I haven't verified all assumptions above, but I know for sure that there is a call awViewMethodsImpl.onSizeChanged(0, 0, 0, 0) when entering fullscreen, because I added debug statements in the code and recompiled the webview.

The second "resize" event is most likely triggered when the app code attaches/adds the view in the hierarchy, which then gives it its final size (through Android's layout system).

For the record, I've tried with webview 57.0.2987.132 and the original fullscreen problem on Youtube does not occur, BUT there are 2 important things to note:

- there are still 2 "resize" events fired when going fullscreen: one with window size 0 x 0, and one later on with correct window size; however this does not seem to trigger the same code path in youtube; after some more digging without Youtube's JS code, the code responsible for exiting fullscreen is only active on Android using Chrome >= 58, when Youtube experiment "uniplayer_block_pip" is enabled

- when exiting fullscreen, youtube video is paused; this might be unrelated, but I figured I'd mention it

Does anybody know whether this is has been fixed (or worked around) on Youtube?

This issue is now fixed on the YouTube side, and I've filed issue 769380 about the WebView double resize issue, closing this.