Re-land^2 WebVR compositor bypass via BrowserMain context + mailbox

chrome/browser/android/vr_shell/vr_shell_gl.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/vr_shell/vr_shell_gl.h"
 
 #include <limits>
 #include <utility>
 
 #include "base/android/jni_android.h"
 #include "base/callback_helpers.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/android/vr_shell/mailbox_to_surface_bridge.h"
 #include "chrome/browser/android/vr_shell/ui_elements.h"
 #include "chrome/browser/android/vr_shell/ui_scene.h"
 #include "chrome/browser/android/vr_shell/vr_controller.h"
 #include "chrome/browser/android/vr_shell/vr_gl_util.h"
 #include "chrome/browser/android/vr_shell/vr_math.h"
 #include "chrome/browser/android/vr_shell/vr_shell.h"
 #include "chrome/browser/android/vr_shell/vr_shell_delegate.h"
 #include "chrome/browser/android/vr_shell/vr_shell_renderer.h"
 #include "device/vr/android/gvr/gvr_device.h"
 #include "third_party/WebKit/public/platform/WebInputEvent.h"
@@ skipping 51 matching lines @@
 
 // The GVR viewport list has two entries (left eye and right eye) for each
 // GVR buffer.
 static constexpr int kViewportListPrimaryOffset = 0;
 static constexpr int kViewportListHeadlockedOffset = 2;
 
 // Buffer size large enough to handle the current backlog of poses which is
 // 2-3 frames.
 static constexpr unsigned kPoseRingBufferSize = 8;
 
-// Magic numbers used to mark valid pose index values encoded in frame
-// data. Must match the magic numbers used in blink's VRDisplay.cpp.
-static constexpr std::array<uint8_t, 2> kWebVrPosePixelMagicNumbers{{42, 142}};
+// Default downscale factor for computing the recommended WebVR
+// renderWidth/Height from the 1:1 pixel mapped size. Using a rather
+// aggressive downscale due to the high overhead of copying pixels
+// twice before handing off to GVR. For comparison, the polyfill
+// uses approximately 0.55 on a Pixel XL.
+static constexpr float kWebVrRecommendedResolutionScale = 0.5;

[mthiesse, 2017/03/08 01:00:05]

Why does gvr choose a size so large that we have to half it?

[klausw, 2017/03/08 02:59:22]

GVR reports a maximum recommended render resolution at which
the central region would have a 1:1 pixel mapping after distortion.

A Pixel XL has native 2560x1440 resolution and a 1:1
render resolution of 3604x2268 or so. For comparison, an HTC Vive
has native 2160x1200 resolution and a 1:1 render resolution of
3024x1680, and obviously a far more powerful GPU.

While it's typical to do supersampling with even higher 
than recommended resolutions on desktop, we need to go
the other way on mobile, it's not realistic to render any
moderately complex content at the 1:1 pixel ratio. Native GVR
apps often render at 0.7, but due to the additional overhead
in current WebVR we need to downscale it even more.

Now that we support canvas resizing, applications can always
opt to use larger sizes if they think they can handle it.

 
 float Distance(const gvr::Vec3f& vec1, const gvr::Vec3f& vec2) {
   float xdiff = (vec1.x - vec2.x);
   float ydiff = (vec1.y - vec2.y);
   float zdiff = (vec1.z - vec2.z);
   float scale = xdiff * xdiff + ydiff * ydiff + zdiff * zdiff;
   return std::sqrt(scale);
 }
 
 // Generate a quaternion representing the rotation from the negative Z axis
@@ skipping 69 matching lines @@
       binding_(this),
       weak_vr_shell_(weak_vr_shell),
       delegate_provider_(delegate_provider),
       main_thread_task_runner_(std::move(main_thread_task_runner)),
       weak_ptr_factory_(this) {
   GvrInit(gvr_api);
 }
 
 VrShellGl::~VrShellGl() {
   vsync_task_.Cancel();
+  binding_.Close();
   if (!callback_.is_null()) {
     // When this VSync provider is going away we have to respond to pending
     // callbacks, so instead of providing a VSync, tell the requester to try
     // again. A VSyncProvider is guaranteed to exist, so the request in response
     // to this message will go through some other VSyncProvider.
     base::ResetAndReturn(&callback_)
         .Run(nullptr, base::TimeDelta(), -1,
-             device::mojom::VRVSyncProvider::Status::RETRY);
-  }
-  if (binding_.is_bound()) {
-    main_thread_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&VrShellDelegate::OnVRVsyncProviderRequest,
-                   delegate_provider_, base::Passed(binding_.Unbind())));
+             device::mojom::VRVSyncProvider::Status::CLOSING);
   }
 }
 
 void VrShellGl::Initialize() {
   scene_.reset(new UiScene);
 
   if (surfaceless_rendering_) {
     // If we're rendering surfaceless, we'll never get a java surface to render
     // into, so we can initialize GL right away.
     InitializeGl(nullptr);
@@ skipping 26 matching lines @@
     LOG(ERROR) << "gl::init::CreateGLContext failed";
     ForceExitVr();
     return;
   }
   if (!context_->MakeCurrent(surface_.get())) {
     LOG(ERROR) << "gl::GLContext::MakeCurrent() failed";
     ForceExitVr();
     return;
   }
 
-  unsigned int textures[2];
-  glGenTextures(2, textures);
+  unsigned int textures[3];
+  glGenTextures(3, textures);
   ui_texture_id_ = textures[0];
   content_texture_id_ = textures[1];
+  webvr_texture_id_ = textures[2];
   ui_surface_texture_ = gl::SurfaceTexture::Create(ui_texture_id_);
   content_surface_texture_ = gl::SurfaceTexture::Create(content_texture_id_);
+  webvr_surface_texture_ = gl::SurfaceTexture::Create(webvr_texture_id_);
   CreateUiSurface();
   CreateContentSurface();
+  // WebVR surface is created below.

[mthiesse, 2017/03/08 01:00:02]

nit: useless comment

[klausw, 2017/03/08 02:59:22]

Done.

   ui_surface_texture_->SetFrameAvailableCallback(base::Bind(
       &VrShellGl::OnUIFrameAvailable, weak_ptr_factory_.GetWeakPtr()));
   content_surface_texture_->SetFrameAvailableCallback(base::Bind(
       &VrShellGl::OnContentFrameAvailable, weak_ptr_factory_.GetWeakPtr()));
+  webvr_surface_texture_->SetFrameAvailableCallback(base::Bind(
+      &VrShellGl::OnWebVRFrameAvailable, weak_ptr_factory_.GetWeakPtr()));
+  ui_surface_texture_->SetDefaultBufferSize(ui_tex_physical_size_.width,
+                                            ui_tex_physical_size_.height);
   content_surface_texture_->SetDefaultBufferSize(
       content_tex_physical_size_.width, content_tex_physical_size_.height);
-  ui_surface_texture_->SetDefaultBufferSize(ui_tex_physical_size_.width,
-                                            ui_tex_physical_size_.height);
   InitializeRenderer();
 
+  // Pick a size for the WebVR transfer surface based on a downscaled

[mthiesse, 2017/03/08 01:00:05]

Can you comment on why it's okay to do this?

[klausw, 2017/03/08 02:59:22]

Expanded the comment, it's arbitrary and just needs to be consistent for the
three places where the size gets used. We keep the two sizes within this class
consistent, and will resize to match the client if they don't
use our recommended size.

+  // recommended render resolution, and also use that size as the
+  // client-recommended renderWidth/renderHeight and for the GVR
+  // framebuffer. If the client chooses a different size or resizes
+  // it while presenting, we'll resize the transfer surface and GVR
+  // framebuffer to match.
+

[mthiesse, 2017/03/08 01:00:04]

nit: remove blank line

[klausw, 2017/03/08 02:59:22]

Done.

+  auto render_target_size = gvr_api_->GetMaximumEffectiveRenderTargetSize();
+
+  gvr::Sizei webvr_size = {static_cast<int>(render_target_size.width *
+                                            kWebVrRecommendedResolutionScale),
+                           static_cast<int>(render_target_size.height *
+                                            kWebVrRecommendedResolutionScale)};
+
+  // TODO(klausw): should the size be rounded to a multiple of N pixels

[mthiesse, 2017/03/08 01:00:03]

File a bug?

[klausw, 2017/03/08 02:59:22]

Done, crbug.com/699350

+  // to be friendlier to the GPU? The exact size doesn't matter.
+
+  CreateOrResizeWebVRSurface(webvr_size);
+
   vsync_task_.Reset(base::Bind(&VrShellGl::OnVSync, base::Unretained(this)));
   OnVSync();
 
   ready_to_draw_ = true;
 }
 
 void VrShellGl::CreateContentSurface() {
   content_surface_ =
       base::MakeUnique<gl::ScopedJavaSurface>(content_surface_texture_.get());
   main_thread_task_runner_->PostTask(
       FROM_HERE, base::Bind(&VrShell::ContentSurfaceChanged, weak_vr_shell_,
                             content_surface_->j_surface().obj()));
 }
 
 void VrShellGl::CreateUiSurface() {
   ui_surface_ =
       base::MakeUnique<gl::ScopedJavaSurface>(ui_surface_texture_.get());
   main_thread_task_runner_->PostTask(
       FROM_HERE, base::Bind(&VrShell::UiSurfaceChanged, weak_vr_shell_,
                             ui_surface_->j_surface().obj()));
 }
 
+void VrShellGl::CreateOrResizeWebVRSurface(const gvr::Sizei& size) {
+  if (!webvr_surface_texture_) {
+    LOG(ERROR) << "No WebVR surface texture available";

[mthiesse, 2017/03/08 01:00:05]

nit: DLOG?

[klausw, 2017/03/08 02:59:22]

Done.

+    return;
+  }
+
+  // ContentPhysicalBoundsChanged is getting called twice with
+  // identical sizes? Avoid thrashing the existing context.
+  if (size == webvr_surface_size_) {
+    return;
+  }
+
+  if (!size.width || !size.height) {
+    // Invalid size, defer until a new size arrives on a future bounds update.
+    return;
+  }
+
+  webvr_surface_texture_->SetDefaultBufferSize(size.width, size.height);
+  webvr_surface_size_ = size;
+
+  if (mailbox_bridge_) {
+    mailbox_bridge_->ResizeSurface(size.width, size.height);
+  } else {
+    mailbox_bridge_ = base::MakeUnique<MailboxToSurfaceBridge>();
+    webvr_surface_ = mailbox_bridge_->CreateSurface(webvr_surface_texture_);
+  }
+}
+
+void VrShellGl::SubmitWebVRFrame(int16_t frame_index,
+                                 const gpu::MailboxHolder& mailbox) {
+  TRACE_EVENT0("gpu", "VrShellGl::SubmitWebVRFrame");
+
+  bool swapped = mailbox_bridge_->CopyFrameToSurface(frame_index, mailbox,
+                                                     !pending_frames_.empty());
+  // Expect a new frame on the surface queue if draw was successful.
+  if (swapped) {
+    submit_client_->OnSubmitFrameTransferred();
+    pending_frames_.emplace(frame_index);
+  }
+
+  TRACE_EVENT0("gpu", "VrShellGl::glFinish");
+  // This is a load-bearing glFinish, please don't remove it without
+  // before/after timing comparisons. Goal is to clear the GPU queue
+  // of the native GL context to avoid stalls later in GVR frame
+  // acquire/submit.
+  glFinish();
+}
+
+void VrShellGl::SetSubmitClient(
+    device::mojom::VRSubmitFrameClientPtrInfo submit_client_info) {
+  submit_client_.Bind(std::move(submit_client_info));
+}
+
 void VrShellGl::OnUIFrameAvailable() {
   ui_surface_texture_->UpdateTexImage();
 }
 
 void VrShellGl::OnContentFrameAvailable() {
   content_surface_texture_->UpdateTexImage();
   received_frame_ = true;
 }
 
-bool VrShellGl::GetPixelEncodedFrameIndex(uint16_t* frame_index) {
-  TRACE_EVENT0("gpu", "VrShellGl::GetPixelEncodedFrameIndex");
-  if (!received_frame_) {
-    if (last_frame_index_ == (uint16_t)-1)
-      return false;
-    *frame_index = last_frame_index_;
-    return true;
+void VrShellGl::OnWebVRFrameAvailable() {
+  // A "while" loop here is a bad idea. It's legal to call
+  // UpdateTexImage repeatedly even if no frames are available, but
+  // that does *not* wait for a new frame, it just reuses the most
+  // recent one. That would mess up the count.
+  if (pending_frames_.empty()) {
+    // We're expecting a frame, but it's not here yet. Retry in OnVsync.
+    ++premature_received_frames_;
+    return;
   }
-  received_frame_ = false;
 
-  // Read the pose index encoded in a bottom left pixel as color values.
-  // See also third_party/WebKit/Source/modules/vr/VRDisplay.cpp which
-  // encodes the pose index, and device/vr/android/gvr/gvr_device.cc
-  // which tracks poses. Returns the low byte (0..255) if valid, or -1
-  // if not valid due to bad magic number.
-  uint8_t pixels[4];
-  // Assume we're reading from the framebuffer we just wrote to.
-  // That's true currently, we may need to use glReadBuffer(GL_BACK)
-  // or equivalent if the rendering setup changes in the future.
-  glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+  webvr_surface_texture_->UpdateTexImage();
+  int frame_index = pending_frames_.front();
+  TRACE_EVENT1("gpu", "VrShellGl::OnWebVRFrameAvailable", "frame", frame_index);
+  pending_frames_.pop();
 
-  // Check for the magic number written by VRDevice.cpp on submit.
-  // This helps avoid glitches from garbage data in the render
-  // buffer that can appear during initialization or resizing. These
-  // often appear as flashes of all-black or all-white pixels.
-  if (pixels[1] == kWebVrPosePixelMagicNumbers[0] &&
-      pixels[2] == kWebVrPosePixelMagicNumbers[1]) {
-    // Pose is good.
-    *frame_index = pixels[0];
-    last_frame_index_ = pixels[0];
-    return true;
-  }
-  VLOG(1) << "WebVR: reject decoded pose index " << static_cast<int>(pixels[0])
-          << ", bad magic number " << static_cast<int>(pixels[1]) << ", "
-          << static_cast<int>(pixels[2]);
-  return false;
+  // It is be legal for the WebVR client to submit a new frame now,

[mthiesse, 2017/03/08 01:00:02]

nit: s/It is be/It is/

[klausw, 2017/03/08 02:59:22]

Done.

+  // since we've consumed the image. TODO(klausw): would timing be
+  // better to move the rendered notification after draw?
+
+  submit_client_->OnSubmitFrameRendered();
+
+  DrawFrame(frame_index);
 }
 
 void VrShellGl::GvrInit(gvr_context* gvr_api) {
   gvr_api_ = gvr::GvrApi::WrapNonOwned(gvr_api);
   controller_.reset(new VrController(gvr_api));
 
   ViewerType viewerType;
   switch (gvr_api_->GetViewerType()) {
     case gvr::ViewerType::GVR_VIEWER_TYPE_DAYDREAM:
       viewerType = ViewerType::DAYDREAM;
       break;
     case gvr::ViewerType::GVR_VIEWER_TYPE_CARDBOARD:
       viewerType = ViewerType::CARDBOARD;
       break;
     default:
       NOTREACHED();
       viewerType = ViewerType::UNKNOWN_TYPE;
       break;
   }
   UMA_HISTOGRAM_ENUMERATION("VRViewerType", static_cast<int>(viewerType),
                             static_cast<int>(ViewerType::VIEWER_TYPE_MAX));
 }
 
 void VrShellGl::InitializeRenderer() {
-  // While WebVR is going through the compositor path, it shares
-  // the same texture ID. This will change once it gets its own
-  // surface, but store it separately to avoid future confusion.
-  // TODO(klausw,crbug.com/655722): remove this.
-  webvr_texture_id_ = content_texture_id_;
-
   gvr_api_->InitializeGl();
   webvr_head_pose_.assign(kPoseRingBufferSize,
                           gvr_api_->GetHeadSpaceFromStartSpaceRotation(
                               gvr::GvrApi::GetTimePointNow()));
 
   std::vector<gvr::BufferSpec> specs;
   // For kFramePrimaryBuffer (primary VrShell and WebVR content)
   specs.push_back(gvr_api_->CreateBufferSpec());
   render_size_primary_ = specs[kFramePrimaryBuffer].GetSize();
+  render_size_vrshell_ = render_size_primary_;
 
   // For kFrameHeadlockedBuffer (for WebVR insecure content warning).
   // Set this up at fixed resolution, the (smaller) FOV gets set below.
   specs.push_back(gvr_api_->CreateBufferSpec());
   specs.back().SetSize(kHeadlockedBufferDimensions);
   render_size_headlocked_ = specs[kFrameHeadlockedBuffer].GetSize();
 
   swap_chain_.reset(new gvr::SwapChain(gvr_api_->CreateSwapChain(specs)));
 
   vr_shell_renderer_.reset(new VrShellRenderer());
@@ skipping 251 matching lines @@
                             std::unique_ptr<blink::WebInputEvent> event) {
   DCHECK(input_target != InputTarget::NONE);
   auto&& target = input_target == InputTarget::CONTENT
                       ? &VrShell::ProcessContentGesture
                       : &VrShell::ProcessUIGesture;
   main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(target, weak_vr_shell_, base::Passed(std::move(event))));
 }
 
-void VrShellGl::DrawFrame() {
-  TRACE_EVENT0("gpu", "VrShellGl::DrawFrame");
+void VrShellGl::DrawFrame(int frame_index) {
+  TRACE_EVENT1("gpu", "VrShellGl::DrawFrame", "frame", frame_index);
 
   // Reset the viewport list to just the pair of viewports for the
   // primary buffer each frame. Head-locked viewports get added by
   // DrawVrShell if needed.
   buffer_viewport_list_->SetToRecommendedBufferViewports();
 
+  // If needed, resize the primary buffer for use with WebVR.
+  if (web_vr_mode_) {
+    if (render_size_primary_ != webvr_surface_size_) {
+      if (!webvr_surface_size_.width) {
+        return;
+      }
+      render_size_primary_ = webvr_surface_size_;
+      swap_chain_->ResizeBuffer(kFramePrimaryBuffer, render_size_primary_);
+    }
+  } else {
+    if (render_size_primary_ != render_size_vrshell_) {
+      render_size_primary_ = render_size_vrshell_;
+      swap_chain_->ResizeBuffer(kFramePrimaryBuffer, render_size_primary_);
+    }
+  }
+
+  TRACE_EVENT_BEGIN0("gpu", "VrShellGl::AcquireFrame");
   gvr::Frame frame = swap_chain_->AcquireFrame();
+  TRACE_EVENT_END0("gpu", "VrShellGl::AcquireFrame");
   if (!frame.is_valid()) {
     return;
   }
   frame.BindBuffer(kFramePrimaryBuffer);
   if (web_vr_mode_) {
     DrawWebVr();
   }
 
-  uint16_t frame_index;
   gvr::Mat4f head_pose;
 
   // When using async reprojection, we need to know which pose was used in
-  // the WebVR app for drawing this frame. Due to unknown amounts of
-  // buffering in the compositor and SurfaceTexture, we read the pose number
-  // from a corner pixel. There's no point in doing this for legacy
-  // distortion rendering since that doesn't need a pose, and reading back
-  // pixels is an expensive operation. TODO(klausw,crbug.com/655722): stop
-  // doing this once we have working no-compositor rendering for WebVR.
-  if (web_vr_mode_ && gvr_api_->GetAsyncReprojectionEnabled() &&
-      GetPixelEncodedFrameIndex(&frame_index)) {
+  // the WebVR app for drawing this frame. Only needed if reprojection is
+  // in use.
+  if (web_vr_mode_ && gvr_api_->GetAsyncReprojectionEnabled()) {
     static_assert(!((kPoseRingBufferSize - 1) & kPoseRingBufferSize),
                   "kPoseRingBufferSize must be a power of 2");
     head_pose = webvr_head_pose_[frame_index % kPoseRingBufferSize];
     // Process all pending_bounds_ changes targeted for before this frame, being
     // careful of wrapping frame indices.
     static constexpr unsigned max =
         std::numeric_limits<decltype(frame_index_)>::max();
     static_assert(max > kPoseRingBufferSize * 2,
                   "To detect wrapping, kPoseRingBufferSize must be smaller "
                   "than half of frame_index_ range.");
     while (!pending_bounds_.empty()) {
       uint16_t index = pending_bounds_.front().first;
       // If index is less than the frame_index it's possible we've wrapped, so
       // we extend the range and 'un-wrap' to account for this.
       if (index < frame_index)
         index += max;
       // If the pending bounds change is for an upcoming frame within our buffer
       // size, wait to apply it. Otherwise, apply it immediately. This
       // guarantees that even if we miss many frames, the queue can't fill up
       // with stale bounds.
       if (index > frame_index && index <= frame_index + kPoseRingBufferSize)
         break;
 
-      const BoundsPair& bounds = pending_bounds_.front().second;
-      webvr_left_viewport_->SetSourceUv(bounds.first);
-      webvr_right_viewport_->SetSourceUv(bounds.second);
+      const WebVrBounds& bounds = pending_bounds_.front().second;
+      webvr_left_viewport_->SetSourceUv(bounds.left_bounds);
+      webvr_right_viewport_->SetSourceUv(bounds.right_bounds);
+      CreateOrResizeWebVRSurface(bounds.source_size);
       pending_bounds_.pop();
     }
     buffer_viewport_list_->SetBufferViewport(GVR_LEFT_EYE,
                                              *webvr_left_viewport_);
     buffer_viewport_list_->SetBufferViewport(GVR_RIGHT_EYE,
                                              *webvr_right_viewport_);
   } else {
     gvr::ClockTimePoint target_time = gvr::GvrApi::GetTimePointNow();
     target_time.monotonic_system_time_nanos += kPredictionTimeWithoutVsyncNanos;
     head_pose = gvr_api_->GetHeadSpaceFromStartSpaceRotation(target_time);
   }
 
   gvr::Vec3f position = GetTranslation(head_pose);
   if (position.x == 0.0f && position.y == 0.0f && position.z == 0.0f) {
     // This appears to be a 3DOF pose without a neck model. Add one.
     // The head pose has redundant data. Assume we're only using the
     // object_from_reference_matrix, we're not updating position_external.
     // TODO: Not sure what object_from_reference_matrix is. The new api removed
     // it. For now, removing it seems working fine.
     gvr_api_->ApplyNeckModel(head_pose, 1.0f);
   }
 
   // Update the render position of all UI elements (including desktop).
   const float screen_tilt = kDesktopScreenTiltDefault * M_PI / 180.0f;
   scene_->UpdateTransforms(screen_tilt, TimeInMicroseconds());
 
-  UpdateController(GetForwardVector(head_pose));
+  {
+    TRACE_EVENT0("gpu", "VrShellGl::UpdateController");
+    UpdateController(GetForwardVector(head_pose));
+  }
 
-  DrawVrShell(head_pose, frame);
+  // Finish drawing in the primary buffer, and draw the headlocked buffer
+  // if needed. This must be the last drawing call, this method will
+  // return with no frame being bound.
+  DrawVrShellAndUnbind(head_pose, frame);
 
-  frame.Unbind();
-  frame.Submit(*buffer_viewport_list_, head_pose);
+  {
+    TRACE_EVENT0("gpu", "VrShellGl::Submit");
+    frame.Submit(*buffer_viewport_list_, head_pose);
+  }
 
   // No need to swap buffers for surfaceless rendering.
   if (!surfaceless_rendering_) {
     // TODO(mthiesse): Support asynchronous SwapBuffers.
+    TRACE_EVENT0("gpu", "VrShellGl::SwapBuffers");
     surface_->SwapBuffers();
   }
 }
 
-void VrShellGl::DrawVrShell(const gvr::Mat4f& head_pose, gvr::Frame& frame) {
+void VrShellGl::DrawVrShellAndUnbind(const gvr::Mat4f& head_pose,
+                                     gvr::Frame& frame) {
   TRACE_EVENT0("gpu", "VrShellGl::DrawVrShell");
   std::vector<const ContentRectangle*> head_locked_elements;
   std::vector<const ContentRectangle*> world_elements;
   for (const auto& rect : scene_->GetUiElements()) {
     if (!rect->IsVisible())
       continue;
     if (rect->lock_to_fov) {
       head_locked_elements.push_back(rect.get());
     } else {
       world_elements.push_back(rect.get());
@@ skipping 16 matching lines @@
 
     const Colorf& backgroundColor = scene_->GetBackgroundColor();
     glClearColor(backgroundColor.r, backgroundColor.g, backgroundColor.b,
                  backgroundColor.a);
     glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
   }
   if (!world_elements.empty()) {
     DrawUiView(&head_pose, world_elements, render_size_primary_,
                kViewportListPrimaryOffset);
   }
+  frame.Unbind();  // Done with the primary buffer.
 
   if (!head_locked_elements.empty()) {
     // Add head-locked viewports. The list gets reset to just
     // the recommended viewports (for the primary buffer) each frame.
     buffer_viewport_list_->SetBufferViewport(
         kViewportListHeadlockedOffset + GVR_LEFT_EYE,
         *headlocked_left_viewport_);
     buffer_viewport_list_->SetBufferViewport(
         kViewportListHeadlockedOffset + GVR_RIGHT_EYE,
         *headlocked_right_viewport_);
 
     // Bind the headlocked framebuffer.
-    // TODO(mthiesse): We don't unbind this? Maybe some cleanup is in order
-    // here.
     frame.BindBuffer(kFrameHeadlockedBuffer);
     glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
     glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
     DrawUiView(nullptr, head_locked_elements, render_size_headlocked_,
                kViewportListHeadlockedOffset);
+    frame.Unbind();  // Done with the headlocked buffer.
   }
 }
 
-gvr::Sizei VrShellGl::GetWebVRCompositorSurfaceSize() {
-  // This is a stopgap while we're using the WebVR compositor rendering path.
-  // TODO(klausw,crbug.com/655722): Remove this method and member once we're
-  // using a separate WebVR render surface.
-  return content_tex_physical_size_;
-}
-
 void VrShellGl::DrawUiView(const gvr::Mat4f* head_pose,
                            const std::vector<const ContentRectangle*>& elements,
                            const gvr::Sizei& render_size,
                            int viewport_offset) {
   TRACE_EVENT0("gpu", "VrShellGl::DrawUiView");
 
   gvr::Mat4f view_matrix;
   if (head_pose) {
     view_matrix = *head_pose;
   } else {
@@ skipping 186 matching lines @@
 void VrShellGl::DrawWebVr() {
   TRACE_EVENT0("gpu", "VrShellGl::DrawWebVr");
   // Don't need face culling, depth testing, blending, etc. Turn it all off.
   glDisable(GL_CULL_FACE);
   glDepthMask(GL_FALSE);
   glDisable(GL_DEPTH_TEST);
   glDisable(GL_SCISSOR_TEST);
   glDisable(GL_BLEND);
   glDisable(GL_POLYGON_OFFSET_FILL);
 
-  glViewport(0, 0, render_size_primary_.width, render_size_primary_.height);
+  glViewport(0, 0, webvr_surface_size_.width, webvr_surface_size_.height);
   vr_shell_renderer_->GetWebVrRenderer()->Draw(webvr_texture_id_);
 }
 
 void VrShellGl::OnTriggerEvent() {
   // Set a flag to handle this on the render thread at the next frame.
   touch_pending_ = true;
 }
 
 void VrShellGl::OnPause() {
   vsync_task_.Cancel();
@@ skipping 10 matching lines @@
     OnVSync();
   }
 }
 
 void VrShellGl::SetWebVrMode(bool enabled) {
   web_vr_mode_ = enabled;
 }
 
 void VrShellGl::UpdateWebVRTextureBounds(int16_t frame_index,
                                          const gvr::Rectf& left_bounds,
-                                         const gvr::Rectf& right_bounds) {
+                                         const gvr::Rectf& right_bounds,
+                                         const gvr::Sizei& source_size) {
   if (frame_index < 0) {
     webvr_left_viewport_->SetSourceUv(left_bounds);
     webvr_right_viewport_->SetSourceUv(right_bounds);
   } else {
-    pending_bounds_.emplace(
-        std::make_pair(frame_index, std::make_pair(left_bounds, right_bounds)));
+    pending_bounds_.emplace(std::make_pair(
+        frame_index, WebVrBounds(left_bounds, right_bounds, source_size)));
   }
 }
 
 void VrShellGl::ContentBoundsChanged(int width, int height) {
   TRACE_EVENT0("gpu", "VrShellGl::ContentBoundsChanged");
   content_tex_css_width_ = width;
   content_tex_css_height_ = height;
 }
 
 void VrShellGl::ContentPhysicalBoundsChanged(int width, int height) {
@@ skipping 13 matching lines @@
     ui_surface_texture_->SetDefaultBufferSize(width, height);
   ui_tex_physical_size_.width = width;
   ui_tex_physical_size_.height = height;
 }
 
 base::WeakPtr<VrShellGl> VrShellGl::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 void VrShellGl::OnVSync() {
+  while (premature_received_frames_ > 0) {
+    TRACE_EVENT0("gpu", "VrShellGl::OnWebVRFrameAvailableRetry");
+    --premature_received_frames_;
+    OnWebVRFrameAvailable();
+  }
+
   base::TimeTicks now = base::TimeTicks::Now();
   base::TimeTicks target;
 
   // Don't send VSyncs until we have a timebase/interval.
   if (vsync_interval_.is_zero())
     return;
   target = now + vsync_interval_;
   int64_t intervals = (target - vsync_timebase_) / vsync_interval_;
   target = vsync_timebase_ + intervals * vsync_interval_;
   task_runner_->PostDelayedTask(FROM_HERE, vsync_task_.callback(),
                                 target - now);
 
   base::TimeDelta time = intervals * vsync_interval_;
   if (!callback_.is_null()) {
     SendVSync(time, base::ResetAndReturn(&callback_));
   } else {
     pending_vsync_ = true;
     pending_time_ = time;
   }
-  DrawFrame();
+  if (!web_vr_mode_) {
+    DrawFrame(-1);
+  }
 }
 
 void VrShellGl::OnRequest(device::mojom::VRVSyncProviderRequest request) {
   binding_.Close();
   binding_.Bind(std::move(request));
 }
 
 void VrShellGl::GetVSync(const GetVSyncCallback& callback) {
   if (!pending_vsync_) {
     if (!callback_.is_null()) {
@@ skipping 49 matching lines @@
 void VrShellGl::ResetPose() {
   // Should never call RecenterTracking when using with Daydream viewers. On
   // those devices recentering should only be done via the controller.
   if (gvr_api_ && gvr_api_->GetViewerType() == GVR_VIEWER_TYPE_CARDBOARD)
     gvr_api_->RecenterTracking();
 }
 
 void VrShellGl::CreateVRDisplayInfo(
     const base::Callback<void(device::mojom::VRDisplayInfoPtr)>& callback,
     uint32_t device_id) {
+  // This assumes that the initial webvr_surface_size_ was set to the
+  // appropriate recommended render resolution as the default size during
+  // InitializeGl. Revisit if the initialization order changes.
   device::mojom::VRDisplayInfoPtr info = VrShell::CreateVRDisplayInfo(
-      gvr_api_.get(), content_tex_physical_size_, device_id);
+      gvr_api_.get(), webvr_surface_size_, device_id);
   main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&RunVRDisplayInfoCallback, callback, base::Passed(&info)));
 }
 
 }  // namespace vr_shell