mthiesse@, do you think you could look into this? Looks like we lost both window.requestAnimationFrame and window.requestIdleCallback processing as part of pausing the compositing loop.

Does an active VR display mean the main tab isn't visible? If so, then stopping rAF/rIC sounds expected. What kind of processing would you need to do with those APIs while in VR mode?

For some time now platform advocates have been insisting on relying on rAF for operations that can be spaced between frames (instead of setTimeout or setInterval). Batched evaluation of data, or processing of queues; with rAF you have more control of the actual load per frame. I'll admit it's not widely used, so not having window.rAF, while inconvenient, it can be dealt with.

window.requestIdleCallback, on the other hand, it's been promoted as a way of scheduling work after the frame has been rendered (all this is from DOM patterns, but i think they should translate to VR all the same). I'm updating some textures and buffers with requestIdleCallback.

I understand that from a "pure" WebVR app perspective it makes sense considering that there's a display that is rendering content, so the main tab isn't visible, etc. but in practice, most WebVR sites play standard magic window content too, so they rely on rAF and rIC. It could be solved in code by making a common rAF handler, or by updating all interested parts in what display is presenting, but that seems a bit of a smell.

I'm not quite up to speed on the latest WebVR specs, but is it possible to display the non-VR content in the VR view? If so, then maybe we shouldn't consider the main tab to be hidden in VR mode.

skyostil, do you mean something like displaying DOM in the VR canvas? No, that's not currently possible - you can't create a texture from your DOM, for example.

Also, at least on Android, the page is never visible while in VR, so it's reasonable to at least stop the compositor from actually rendering frames. Whether we want to continue firing rAF/rIC is a separate question, and we may need some path not tied to VSync to trigger these.

skyostil@, it depends. The headset properties include a "has external display" property. It's true for devices such as HTC Vive or Oculus Rift that have their own built-in display where Chrome can continue showing the normal content while presenting. It's false for phone-based VR where the VR rendering basically takes over the entire display.

In either case, the displayed VR content is the content of a canvas element, the rest of the page is not visible in the headset.

I think one way of looking at it would be that in the "no external display" case, the non-canvas content is hidden in the same way as an iframe that was scrolled offscreen or covered by an opaque element. The browser could do optimizations in that case to skip drawing steps since it knows the content isn't visible, but I think that rAF and idle callbacks should continue to work.

Basically this would be splitting up "hidden" into the two categories "currently not visible on a page that's otherwise active" and "really hidden since it's a non-foreground tab". Would that make sense?

I see. It might be a little tricky to coax Chrome into doing anything useful with the requestAnimationFrame callback while the main activity is hidden. Maybe we should introduce some rAF-like API on the VR display instead? Similarly we could have a rIC-equivalent there too.

All it needs to do is execute the JS code. We already have a separate rAF
on the vrDisplay object, and that's what's used for actual rendering.

The issue is that lack of window.rAF and IC breaks applications that rely
on them for scheduling non-rendering background work. This often happens in
libraries where it would not be reasonable to expect them to be modified to
use WebVR methods instead.

The problem is that rAF is tied to display updates, and if the display isn't updating we'd need to introduce some kind of fake display to drive it. Also, in general I wouldn't recommend scheduling non-rendering work with rAF. It's possible that browsers will start throttling such work in the near future to save power.

For rIC I think we should also consider something based on the VR display. The reason is that if rAF isn't active, rIC is given 50ms time slots, which would adversely affect VR rendering. Something that uses the VR display's timings for deadlines would be better I think.

For testing purposes see https://threejs.org/examples/webvr_video.html where the video freezes after entering VR due to using un-namespaced requestAnimationFrame which resolves to window.rAF: https://github.com/mrdoob/three.js/blob/master/src/textures/VideoTexture.js

Arguably this specific example should be fixed to use a more appropriate update method, but in general I think it's unfriendly to developers to require possibly invasive changes to make them work in WebVR, especially if it's a fairly trivial call that doesn't need significant execution time.

The "time remaining" calculation is a good point. If it's purely based on time consumed by window.rAF, it would be misleading in a VR context. In practical terms, it seems that a majority of existing WebVR applications don't hit 60fps on mobile, so there's no idle time as such. We may want to consider adjusting the assumed scheduling to target 30fps instead if it's not keeping up.

For background, see also  issue 698923  "pause standard compositor while presenting on Android".

Originally I was doing something like this:

  // hide in beginPresent() if no external display:
  m_navigatorVR->document()->view()->hide();

  // unhide in stopPresenting()
  m_navigatorVR->document()->view()->show();
  doc->view()->show();

That worked for the purpose of suppressing most of the normal compositing work while keeping window.rAF and idle callback active, but it caused an assertion failure (!m_isHidden) in debug builds for DrawingBuffer::prepareTextureMailboxInternal() via FrameView.cpp:

        if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
          if (view.compositor()->inCompositingMode())
            scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();

I was able to fix this assertion failure by adding a visibility check in that code:

          if (view.compositor()->inCompositingMode() && isSelfVisible())

Alternatively, I had tried disabling compositing mode via doc->layoutView()->compositor()->setCompositingModeEnabled(false), but that seems irreversible, the view is frozen after exiting presentation and re-enabling compositing mode.

skyostil@, do you think something along those lines would be preferable over the current approach where we're suppressing vsync? 

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

commit 5873ff63b705173921eddc8741704fbde962a41f
Author: klausw <klausw@chromium.org>
Date: Thu Apr 13 01:39:07 2017

WebVR: handle window.rAF while presenting

We currently aren't processing the normal vsync/BeginFrame events
during WebVR presentation, this means that only vrDisplay.rAF
callbacks get executed, not window.rAF callbacks. This breaks
applications that depend on window.rAF for background processing,
so we should continue executing those callbacks too.

This patch does not enable requestIdleCallback handling, that is
significantly more complex and we're looking into that separately.

BUG= 704341 

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

[modify] https://crrev.com/5873ff63b705173921eddc8741704fbde962a41f/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
[modify] https://crrev.com/5873ff63b705173921eddc8741704fbde962a41f/third_party/WebKit/Source/modules/vr/VRDisplay.h

I've filed the new issue 712865 to track window.rIC support which still doesn't work. Retroactively redefining this one to track window.rAF specifically so that we can keep it separated, and closing the issue since window.rAF now works again.

Do we have a test to catch regressions in the future? It should be pretty easy to, say, except some number of rAFs after presenting. This might require a browser or instrumentation test, though. +bsheedy. If this is non-trivial, we could open a separate bug for rAF and rIC regression tests.

I filed  issue 716647  to add such tests.

The consensus appears to be that the previous behavior was correct. I've filed  issue 718246  to restore the behavior, which it turns out was "fixed" by the "regression" tracked in this issue.

The CL in this issue is now being reverted here:

https://chromium-review.googlesource.com/c/chromium/src/+/628963