WIP: working copy-no-compositor path

WIP: working copy-no-compositor path

Prerequisite patches:

https://codereview.chromium.org/2586803003/ "Enable creation of offscreen contexts which own their backing surface."

https://codereview.chromium.org/2456213002/ "WebVR: implement SetSurfaceHandleCHROMIUM extension for gvr_device."

- use dynamic 55%-of-1:1 WebVR content render resolution

- delete corner pixel frame index hack.

- delete fullscreen hack.

- disable background compositor via document->view->hide()

- draw WebVR content directly to Surface via Blink-side rebinding

- add return value to SubmitFrame, track outstanding frames, skip if >1.

- detect and suppress reused or invalid poses

- new rAF throttling, per-frame layer bounds

- dynamic resolution adjustment can happen before or after getting pose, adjust accordingly.

- use pose age prediction for rendering

- use GetSurfaceHandle to resize, make async

- async getPose

- 60fps frame limiter

BUG=655722
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

[klausw, 2016-12-19 04:26:07]

Description was changed from

==========
WIP: working copy-no-compositor path

- use dynamic 55%-of-1:1 WebVR content render resolution

- delete corner pixel frame index hack.

- delete fullscreen hack.

- disable background compositor via document->view->hide()

- draw WebVR content directly to Surface via Blink-side rebinding

- add return value to SubmitFrame, track outstanding frames, skip if >1.

- detect and suppress reused or invalid poses

- new rAF throttling, per-frame layer bounds

- dynamic resolution adjustment can happen before or after getting pose, adjust
accordingly.

- use pose age prediction for rendering

- use GetSurfaceHandle to resize, make async

- async getPose

- 60fps frame limiter
==========

to

==========
WIP: working copy-no-compositor path

- use dynamic 55%-of-1:1 WebVR content render resolution

- delete corner pixel frame index hack.

- delete fullscreen hack.

- disable background compositor via document->view->hide()

- draw WebVR content directly to Surface via Blink-side rebinding

- add return value to SubmitFrame, track outstanding frames, skip if >1.

- detect and suppress reused or invalid poses

- new rAF throttling, per-frame layer bounds

- dynamic resolution adjustment can happen before or after getting pose, adjust
accordingly.

- use pose age prediction for rendering

- use GetSurfaceHandle to resize, make async

- async getPose

- 60fps frame limiter
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[klausw, 2016-12-19 04:28:00]

Description was changed from

==========
WIP: working copy-no-compositor path

- use dynamic 55%-of-1:1 WebVR content render resolution

- delete corner pixel frame index hack.

- delete fullscreen hack.

- disable background compositor via document->view->hide()

- draw WebVR content directly to Surface via Blink-side rebinding

- add return value to SubmitFrame, track outstanding frames, skip if >1.

- detect and suppress reused or invalid poses

- new rAF throttling, per-frame layer bounds

- dynamic resolution adjustment can happen before or after getting pose, adjust
accordingly.

- use pose age prediction for rendering

- use GetSurfaceHandle to resize, make async

- async getPose

- 60fps frame limiter
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
WIP: working copy-no-compositor path

Prerequisite patches:

https://codereview.chromium.org/2586803003/ "Enable creation of offscreen
contexts which own their backing surface."

https://codereview.chromium.org/2456213002/ "WebVR: implement
SetSurfaceHandleCHROMIUM extension for gvr_device."

- use dynamic 55%-of-1:1 WebVR content render resolution

- delete corner pixel frame index hack.

- delete fullscreen hack.

- disable background compositor via document->view->hide()

- draw WebVR content directly to Surface via Blink-side rebinding

- add return value to SubmitFrame, track outstanding frames, skip if >1.

- detect and suppress reused or invalid poses

- new rAF throttling, per-frame layer bounds

- dynamic resolution adjustment can happen before or after getting pose, adjust
accordingly.

- use pose age prediction for rendering

- use GetSurfaceHandle to resize, make async

- async getPose

- 60fps frame limiter
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[klausw, 2017-01-05 20:36:36]

Description was changed from

==========
WIP: working copy-no-compositor path

Prerequisite patches:

https://codereview.chromium.org/2586803003/ "Enable creation of offscreen
contexts which own their backing surface."

https://codereview.chromium.org/2456213002/ "WebVR: implement
SetSurfaceHandleCHROMIUM extension for gvr_device."

- use dynamic 55%-of-1:1 WebVR content render resolution

- delete corner pixel frame index hack.

- delete fullscreen hack.

- disable background compositor via document->view->hide()

- draw WebVR content directly to Surface via Blink-side rebinding

- add return value to SubmitFrame, track outstanding frames, skip if >1.

- detect and suppress reused or invalid poses

- new rAF throttling, per-frame layer bounds

- dynamic resolution adjustment can happen before or after getting pose, adjust
accordingly.

- use pose age prediction for rendering

- use GetSurfaceHandle to resize, make async

- async getPose

- 60fps frame limiter
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
WIP: working copy-no-compositor path

Prerequisite patches:

https://codereview.chromium.org/2586803003/ "Enable creation of offscreen
contexts which own their backing surface."

https://codereview.chromium.org/2456213002/ "WebVR: implement
SetSurfaceHandleCHROMIUM extension for gvr_device."

- use dynamic 55%-of-1:1 WebVR content render resolution

- delete corner pixel frame index hack.

- delete fullscreen hack.

- disable background compositor via document->view->hide()

- draw WebVR content directly to Surface via Blink-side rebinding

- add return value to SubmitFrame, track outstanding frames, skip if >1.

- detect and suppress reused or invalid poses

- new rAF throttling, per-frame layer bounds

- dynamic resolution adjustment can happen before or after getting pose, adjust
accordingly.

- use pose age prediction for rendering

- use GetSurfaceHandle to resize, make async

- async getPose

- 60fps frame limiter

BUG=655722
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[artem.bolgar, 2017-02-14 03:34:27]

artem.bolgar@oculusvr.com changed reviewers:
+ artem.bolgar@oculusvr.com, klausw@chromium.org

[artem.bolgar, 2017-02-14 03:34:29]

https://codereview.chromium.org/2584343002/diff/60001/device/vr/android/gvr/g...
File device/vr/android/gvr/gvr_delegate.h (right):

https://codereview.chromium.org/2584343002/diff/60001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_delegate.h:21: class DEVICE_VR_EXPORT GvrDelegate {
Shouldn't this class have a virtual dtor?

https://codereview.chromium.org/2584343002/diff/60001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_delegate.h:36: class DEVICE_VR_EXPORT
GvrDelegateProvider {
Shouldn't this class have a virtual dtor?

[artem.bolgar, 2017-02-14 05:04:23]

artem.bolgar@oculusvr.com changed reviewers:
+ bajones@chromium.org

[artem.bolgar, 2017-02-14 05:04:24]

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell_gl.cc (right):

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell_gl.cc:844: callback.Run(0);
You can't do this here. The callback may be executed from the thread, where it
was 'established' ('created', 'bound', 'connected') (not sure what the right
term is). Which is the thread of VrShell, while VrShellGL is a separate thread
here. Doing this here causes an assertion in interface_endpoint_client.cc,
Accept method, task_runner_->RunsTasksOnCurrentThread(). The proper way is to
post another task back to VrShell (main) thread and execute the callback from
there.

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell_gl.cc:874:
callback.Run(webvr_surface_handle_);
You can't do this here. The callback may be executed from the thread, where it
was 'established' ('created', 'bound', 'connected') (not sure what the right
term is). Which is the thread of VrShell, while VrShellGL is a separate thread
here. Doing this here causes an assertion in interface_endpoint_client.cc,
Accept method, task_runner_->RunsTasksOnCurrentThread(). The proper way is to
post another task back to VrShell (main) thread and execute the callback from
there.

https://codereview.chromium.org/2584343002/diff/60001/device/vr/android/gvr/g...
File device/vr/android/gvr/gvr_device.cc (right):

https://codereview.chromium.org/2584343002/diff/60001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_device.cc:80: VLOG(1) << __FUNCTION__ << ": DESTRUCTOR
this=" << (void*)this << "
***********************************************************************************************************";
It is possible that callback_map_ is not empty here. In this case, you'll get an
assertion, if the callback is not executed. Must go through them and call, to
avoid the assertion:
====
  // Run pending "frame complete" callbacks
  for (auto it : callback_map_) {
    VLOG(2) << __FUNCTION__ << ": running pending callback for frame: " <<
it.first;
    it.second.Run(0, it.first, -1.0);
  }
  callback_map_.clear();
====

https://codereview.chromium.org/2584343002/diff/60001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_device.cc:308: for (auto it : callback_map_) {
This cleanup code block should be moved to GvrDevice::OnExitPresent (of course,
such override method should be created first). The reason: you may exit
presentation w/o calling exitPresent (by pressing back button, for example, that
calls shutdownVR, that calls GvrDeviceProvider::OnDelegateRemoved(), that calls
Device's OnExitPresent (currently defined only in VRDevice).

[artem.bolgar, 2017-02-14 05:07:03]

On a side note:
It would be nice to abstract whole VrShell a bit more, at least within the
Android platform. I've created a base abstract C++ class VrShellInterface

class VrShellInterface {
 public:
  VrShellInterface() {}

  virtual void ContentWebContentsDestroyed() = 0;
  // Called when our WebContents have been hidden. Usually a sign that something
  // like another tab placed in front of it.
  virtual void ContentWasHidden() = 0;

  virtual void ContentBoundsChanged(
      JNIEnv* env,
      const base::android::JavaParamRef<jobject>& object,
      jint width, jint height, jfloat dpr) = 0;

  virtual void ContentFrameWasResized(bool width_changed) = 0;

 protected:
  virtual ~VrShellInterface() {}
 private:
  DISALLOW_COPY_AND_ASSIGN(VrShellInterface);
};

and inherited the actual VrShell class from it. VrWebContentsObserver was
modified to deal with VrShellInterface rather than with VrShell. This adds me an
ability to implement my own 'VrShell' (incl delegate and GL impl) and keep using
the rest of the code (VrShellRenderer, VrWebContentsObserver, etc).

Similar thing I had to do on Java side, where I had to create a
org.chromium.chrome.browser.VrShellDelegateBase and modify the rest of the
org.chromium.chrome.browser.* classes to deal with it instead of direct usage of
org.chromium.chrome.browser.vr_shell.VrShellDelegate. The only modification I
needed in the core classes is the creation of the proper instance of the
VrShellDelegate in ChromeTabbedActivity.

[artem.bolgar, 2017-02-28 20:15:53]

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell_gl.cc (right):

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell_gl.cc:188: }
It seems like WebVR surface (maybe others too) is not being cleaned up from the
GpuSurfaceTracker. It seems like it is necessary to add something like this code
here:

  if (webvr_surface_handle_) {
    // Cleanup surface from the tracker
    gpu::GpuSurfaceTracker *tracker = gpu::GpuSurfaceTracker::Get();
    tracker->UnregisterViewSurface(webvr_surface_handle_);
  }

[mthiesse, 2017-02-28 20:22:47]

mthiesse@chromium.org changed reviewers:
+ mthiesse@chromium.org

[mthiesse, 2017-02-28 20:22:48]

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell_gl.cc (right):

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell_gl.cc:188: }
On 2017/02/28 20:15:52, artem.bolgar wrote:
> It seems like WebVR surface (maybe others too) is not being cleaned up from
the
> GpuSurfaceTracker. It seems like it is necessary to add something like this
code
> here:
> 
>   if (webvr_surface_handle_) {
>     // Cleanup surface from the tracker
>     gpu::GpuSurfaceTracker *tracker = gpu::GpuSurfaceTracker::Get();
>     tracker->UnregisterViewSurface(webvr_surface_handle_);
>   }

Our other surfaces are likely okay, we go through an entirely different path for
them.

[artem.bolgar, 2017-02-28 21:40:22]

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2584343002/diff/60001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:183: gl_thread_.reset();
gl_thread_.reset() kills VrShellGl instance along with the WebVR SurfaceTexture
(if it was in use). However, the GPU and VRDisplay still use that and therefore,
you most likely will see the similar to 'BufferQueueProducer:
[SurfaceTexture-3-16381-2] dequeueBuffer: BufferQueue has been abandoned' error
message in the logcat each time you exit WebVR. You may destroy the WebVR
SurfaceTexture after the VRDisplay calls
m_renderingContext->setSurfaceHandle(0).

What I did to fix the problem (I hope, there is a better/easier way, though):
1) Added OnExitWebVR() => (); into 'interface VRDisplayClient'
2) Implemented it the following way:

void VRDisplay::OnExitWebVR(const OnExitWebVRCallback& callback) {
  VLOG(1) << __FUNCTION__;
  if (m_surfaceHandle && m_renderingContext) {
    m_renderingContext->setSurfaceHandle(0);
    VLOG(1) << " setSurfaceHandle(0); was just called";
    m_surfaceHandle = 0;
  }
  callback.Run();
}

3) In vr_shell.cc I've changed the dtor by adding creation of the callback (from
VrShellGl and passing it to delegate_->device_provider():

VrShell::~VrShell() {
  VLOG(1) << __FUNCTION__ << ": Destructor for presenting delegate";

  // must be done before killing GL thread, otherwise you'll get "BufferQueue
has been abandoned" error in logcat.
  GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
  base::Callback<void()> cleanupCallback =
thread->GetVrShellGlUnsafe()->GetWebVrSurfaceCleanupCallback();

....  gl_thread_.reset();

  if (delegate_->device_provider()) {
    delegate_->device_provider()->OnWebVrCleanup(cleanupCallback);
  }


4) In vr_shell_gl.cc I've implemented a WebVRCleaner callback:

// A class that performs async cleanup of WebVR surface. The Clean method is
called by the VRDisplay class
// once WebVR surface is not used anymore.
class WebVRCleaner {
public:
  WebVRCleaner(scoped_refptr<gl::SurfaceTexture>& webvr_surface_texture) :
webvr_surface_texture_(webvr_surface_texture) {}
  ~WebVRCleaner() { DVLOG(1) << __FUNCTION__; }

  void Clean() {
    DVLOG(1) << __FUNCTION__;
    webvr_surface_texture_ = nullptr;
  }

  scoped_refptr<gl::SurfaceTexture> webvr_surface_texture_;
};

base::Callback<void()> VrShellGl::GetWebVrSurfaceCleanupCallback() {
  VLOG(1) << __FUNCTION__;
  WebVRCleaner* cleaner = new WebVRCleaner(webvr_surface_texture_);
  return base::Bind(&WebVRCleaner::Clean, base::Owned(cleaner));
}

5) Added:
 
void GvrDeviceProvider::OnWebVrCleanup(const WebVrCleanupCallback&
cleanupCallback) {
  VLOG(1) << __FUNCTION__;
  if (!vr_device_) {
    cleanupCallback.Run();
    return;
  }

  vr_device_->OnExitWebVR(cleanupCallback);
}

and 

void VRDevice::OnExitWebVR(const OnExitWebVRCallback& callback) {
  for (const auto& display : displays_) {
    display->client()->OnExitWebVR(callback);
    break; // really need only one display for now //??? @revise
  }
}

Thus, destruction of the VrShell will cause async call to content process, that
will reset the surface and then will call the callback that holds a last ref to
the corresponding SurfaceHandle and will destroy it with no errors. A bit too
complicated, would be glad to see a better solution.