Forked from issue 755733

assigning to yusufo for comment.

The issue is that ChromeActivity only tells the tab that it was shown after it gets Window Focus, which is *way* after the tab is actually shown.

You made this change in https://codereview.chromium.org/1470383003/. Was there a reason for doing this? Is that reason still valid?

It seems like the proper place to notify of visibility changes is in onStart, and doing so would fix the issue we're seeing.

We are saying the issues are forked, but I think the descriptions are the same? Since this one is the one assigned to me, I will assume this is the one about onStart vs onWindowFocusChanged debate.

See crbug.com/559197. This is a fairly old issue, but I will try to summarize what I remember.

You are saying we "show" the tab in onStart, but I think you mean, we show the Activity which holds the tab in onStart, so technically we should be able to show the tab as well?

Adding Ted, as well, to keep me honest about the technicalities, but I think the way our bindings to the renderer work, we do need the window's focus for the binding to occur and for us to actually draw anything related with the tab.

There are a few other subtleties to that change (focus change is not enough by itself to hide, because we can still lose focus but still be shown etc), but I think the way the BindingManager works is the main reason why we need the window's focus to actually be able to call show on the tab.

From the android side, I thought window focus was the best signal we had that the activity was visible to the user.  I'm actually quite surprised that the time is that different between onStart and onWindowFocused.  They could be in different UI loops, but I wouldn't think they'd be hugely divergent in time.

I guess you'll always have to do with getting frames when the tab is no longer visible, so this still seems like something VR should handle.  We could investigate showing the tab earlier if there were things to gain, but I would have expected this to come up for non-VR if that were the case.

Just to put some numbers behind this, when minimizing and restoring Chrome, the time delta between onStartWithNative and onWindowFocusChanged is around 100ms on a Pixel phone. When in VR, the time between onStart and onWindowFocusChanged is around 400-600ms, again on a Pixel phone.

Also, I was under the impression that onStart is the signal that your activity is becoming visible, and that focus only comes after onResume, when your activity is foregrounded.

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

commit 9e73e0623e6b22c6266d0362e7b3bf5db9f634ef
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Aug 23 22:24:09 2017

WebVR: Work around page visibility being set late on Android.

This CL introduces a hopefully short-term hacky fix to the problem of
the page visibility not being set when the page actually becomes visible
on android, leading WebVR to hit DCHECKS checking that the page is
visible.

This should be safe because we know for sure that the page is actually
visible when we allow vr presentation, because we only do so after the
activity is resumed.

Bug:  756476 
Change-Id: If9a264fc33793af959ceaf23ab14cc29fcfc0404
Reviewed-on: https://chromium-review.googlesource.com/619888
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496830}
[modify] https://crrev.com/9e73e0623e6b22c6266d0362e7b3bf5db9f634ef/third_party/WebKit/Source/modules/vr/VRDisplay.cpp

Okay, I hate Android (as usual). onWindowFocusChanged means different things for Activitys and Views.

Looks like you're right that onWindowFocusChanged is the signal that your Activity is actually visible. It's very strange that the time delay between onStart and becoming visible is so large. Also I would expect that we might want to start things like WebGL rendering before the window is *actually* visible, or we're guaranteed to not have content for the first frame after becoming visible, right?

We have a workaround for M62, this isn't urgent.

I don't know that there's actionable for Chrome in general here, so I'll just fix this for WebVR and call it a day.

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

commit 3bec6294ed2412ea4d194b30ce5eb00dff5327e3
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Sat Sep 09 00:18:59 2017

VR: Early return on WebVr SubmitFrame if page isn't 'visible'

Adds a check that the page is visible when submitting webVR frames,
returning early if the page isn't visible.

Bug: 755733,  756476 
Change-Id: I60f6eaffab5b0ce94e91b1f922522d2d3c5889ef
Reviewed-on: https://chromium-review.googlesource.com/655777
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500752}
[modify] https://crrev.com/3bec6294ed2412ea4d194b30ce5eb00dff5327e3/third_party/WebKit/Source/modules/vr/VRDisplay.cpp

Michael, could you provide some clear manual steps to verify this? It sounds similar to  issue 770245 . But I'd like to mark this one as verified as possible to remove it from manual testing radar. Thanks!

Oh, and if it's not possible to manually verify, that's fine too, I'll just remove it from our hotlist. 

Don't think this one's manually verifiable, feel free to remove it from the hotlist.