WebVR: implement SetSurfaceHandleCHROMIUM extension for gvr_device.

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index c480b71567dca3814c19aeca0b4f7185bc338e42..5a1cd6b0a98acc34aed0be21651a4828c0263d57 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -65,6 +65,9 @@
 #include "gpu/command_buffer/service/transform_feedback_manager.h"
 #include "gpu/command_buffer/service/vertex_array_manager.h"
 #include "gpu/command_buffer/service/vertex_attrib_manager.h"
+#if defined(OS_ANDROID)
+#include "gpu/ipc/common/gpu_surface_lookup.h"
+#endif
 #include "third_party/angle/src/image_util/loadimage.h"
 #include "third_party/smhasher/src/City.h"
 #include "ui/gfx/buffer_types.h"
@@ -82,6 +85,9 @@
 #include "ui/gl/gl_image.h"
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_surface.h"
+#if defined(OS_ANDROID)
+#include "ui/gl/gl_surface_egl.h"
+#endif
 #include "ui/gl/gl_version_info.h"
 #include "ui/gl/gpu_timing.h"
 
@@ -1034,6 +1040,18 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
   void DoFlushMappedBufferRange(
       GLenum target, GLintptr offset, GLsizeiptr size);
 
+  void DoSetSurfaceHandleCHROMIUM(GLint surface_handle);
+
+  // Apply creation attibutes such as depth_bits for the offscreen
+  // framebuffer or active surface as applicable.
+  GLint ApplySurfaceAttributesAndGetAlphaBits(
+      bool offscreen, const ContextCreationAttribHelper& attrib_helper);
+
+  // Apply feature flags and surface-specific workarounds to match the
+  // capabilities of the current surface. Used by Initialize() and
+  // DoSetSurfaceHandleCHROMIUM.
+  void ApplySurfaceFeatures();
+
   // Creates a Program for the given program.
   Program* CreateProgram(GLuint client_id, GLuint service_id) {
     return program_manager()->CreateProgram(client_id, service_id);
@@ -2131,6 +2149,17 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
   scoped_refptr<gl::GLSurface> surface_;
   scoped_refptr<gl::GLContext> context_;
 
+  ContextCreationAttribHelper attrib_helper_;
+
+  // Direct render mode data for switching and restoring the
+  // underlying destination surface, cf. DoSetSurfaceHandleCHROMIUM.
+  struct DirectRenderState{
+    scoped_refptr<gl::GLSurface> direct_surface;
+    scoped_refptr<gl::GLSurface> orig_surface;
+    std::unique_ptr<BackFramebuffer> orig_offscreen_target_frame_buffer;
+  };
+  std::unique_ptr<DirectRenderState> direct_render_state_;
+
   // The ContextGroup for this decoder uses to track resources.
   scoped_refptr<ContextGroup> group_;
 
@@ -2997,6 +3026,163 @@ GLES2DecoderImpl::GLES2DecoderImpl(ContextGroup* group)
 GLES2DecoderImpl::~GLES2DecoderImpl() {
 }
 
+GLint GLES2DecoderImpl::ApplySurfaceAttributesAndGetAlphaBits(
+    bool offscreen, const ContextCreationAttribHelper& attrib_helper) {
+  GLint alpha_bits = 0;
+
+  if (offscreen) {
+    offscreen_buffer_should_have_alpha_ = attrib_helper.alpha_size > 0;
+
+    // Whether the offscreen buffer texture should have an alpha channel. Does
+    // not include logic from workarounds.
+    bool offscreen_buffer_texture_needs_alpha =
+        offscreen_buffer_should_have_alpha_ ||
+        (ChromiumImageNeedsRGBEmulation() &&
+         attrib_helper.should_use_native_gmb_for_backbuffer);
+
+    if (attrib_helper.samples > 0 && attrib_helper.sample_buffers > 0 &&
+        features().chromium_framebuffer_multisample) {
+      // Per ext_framebuffer_multisample spec, need max bound on sample count.
+      // max_sample_count must be initialized to a sane value.  If
+      // glGetIntegerv() throws a GL error, it leaves its argument unchanged.
+      GLint max_sample_count = 1;
+      glGetIntegerv(GL_MAX_SAMPLES_EXT, &max_sample_count);
+      offscreen_target_samples_ = std::min(attrib_helper.samples,
+                                           max_sample_count);
+    } else {
+      offscreen_target_samples_ = 1;
+    }
+    offscreen_target_buffer_preserved_ = attrib_helper.buffer_preserved;
+
+    if (gl_version_info().is_es) {
+      const bool rgb8_supported = context_->HasExtension("GL_OES_rgb8_rgba8");
+      // The only available default render buffer formats in GLES2 have very
+      // little precision.  Don't enable multisampling unless 8-bit render
+      // buffer formats are available--instead fall back to 8-bit textures.
+      if (rgb8_supported && offscreen_target_samples_ > 1) {
+        offscreen_target_color_format_ =
+            offscreen_buffer_texture_needs_alpha ? GL_RGBA8 : GL_RGB8;
+      } else {
+        offscreen_target_samples_ = 1;
+        offscreen_target_color_format_ =
+            offscreen_buffer_texture_needs_alpha ||
+                    workarounds().disable_gl_rgb_format
+                ? GL_RGBA
+                : GL_RGB;
+      }
+
+      // ANGLE only supports packed depth/stencil formats, so use it if it is
+      // available.
+      const bool depth24_stencil8_supported =
+          feature_info_->feature_flags().packed_depth24_stencil8;
+      VLOG(1) << "GL_OES_packed_depth_stencil "
+              << (depth24_stencil8_supported ? "" : "not ") << "supported.";
+      if ((attrib_helper.depth_size > 0 || attrib_helper.stencil_size > 0) &&
+          depth24_stencil8_supported) {
+        offscreen_target_depth_format_ = GL_DEPTH24_STENCIL8;
+        offscreen_target_stencil_format_ = 0;
+      } else {
+        // It may be the case that this depth/stencil combination is not
+        // supported, but this will be checked later by CheckFramebufferStatus.
+        offscreen_target_depth_format_ = attrib_helper.depth_size > 0 ?
+            GL_DEPTH_COMPONENT16 : 0;
+        offscreen_target_stencil_format_ = attrib_helper.stencil_size > 0 ?
+            GL_STENCIL_INDEX8 : 0;
+      }
+    } else {
+      offscreen_target_color_format_ =
+          offscreen_buffer_texture_needs_alpha ||
+                  workarounds().disable_gl_rgb_format
+              ? GL_RGBA
+              : GL_RGB;
+
+      // If depth is requested at all, use the packed depth stencil format if
+      // it's available, as some desktop GL drivers don't support any non-packed
+      // formats for depth attachments.
+      const bool depth24_stencil8_supported =
+          feature_info_->feature_flags().packed_depth24_stencil8;
+      VLOG(1) << "GL_EXT_packed_depth_stencil "
+              << (depth24_stencil8_supported ? "" : "not ") << "supported.";
+
+      if ((attrib_helper.depth_size > 0 || attrib_helper.stencil_size > 0) &&
+          depth24_stencil8_supported) {
+        offscreen_target_depth_format_ = GL_DEPTH24_STENCIL8;
+        offscreen_target_stencil_format_ = 0;
+      } else {
+        offscreen_target_depth_format_ = attrib_helper.depth_size > 0 ?
+            GL_DEPTH_COMPONENT : 0;
+        offscreen_target_stencil_format_ = attrib_helper.stencil_size > 0 ?
+            GL_STENCIL_INDEX : 0;
+      }
+    }
+
+    offscreen_saved_color_format_ = offscreen_buffer_texture_needs_alpha ||
+                                            workarounds().disable_gl_rgb_format
+                                        ? GL_RGBA
+                                        : GL_RGB;
+  } else {
+    glBindFramebufferEXT(GL_FRAMEBUFFER, GetBackbufferServiceId());
+    // These are NOT if the back buffer has these proprorties. They are
+    // if we want the command buffer to enforce them regardless of what
+    // the real backbuffer is assuming the real back buffer gives us more than
+    // we ask for. In other words, if we ask for RGB and we get RGBA then we'll
+    // make it appear RGB. If on the other hand we ask for RGBA nd get RGB we
+    // can't do anything about that.
+
+    if (!surfaceless_) {
+      GLint depth_bits = 0;
+      GLint stencil_bits = 0;
+
+      bool default_fb = (GetBackbufferServiceId() == 0);
+
+      if (gl_version_info().is_desktop_core_profile) {
+        glGetFramebufferAttachmentParameterivEXT(
+            GL_FRAMEBUFFER,
+            default_fb ? GL_BACK_LEFT : GL_COLOR_ATTACHMENT0,
+            GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE, &alpha_bits);
+        glGetFramebufferAttachmentParameterivEXT(
+            GL_FRAMEBUFFER,
+            default_fb ? GL_DEPTH : GL_DEPTH_ATTACHMENT,
+            GL_FRAMEBUFFER_ATTACHMENT_DEPTH_SIZE, &depth_bits);
+        glGetFramebufferAttachmentParameterivEXT(
+            GL_FRAMEBUFFER,
+            default_fb ? GL_STENCIL : GL_STENCIL_ATTACHMENT,
+            GL_FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE, &stencil_bits);
+      } else {
+        glGetIntegerv(GL_ALPHA_BITS, &alpha_bits);
+        glGetIntegerv(GL_DEPTH_BITS, &depth_bits);
+        glGetIntegerv(GL_STENCIL_BITS, &stencil_bits);
+      }
+
+      // This checks if the user requested RGBA and we have RGBA then RGBA. If
+      // the user requested RGB then RGB. If the user did not specify a
+      // preference than use whatever we were given. Same for DEPTH and STENCIL.
+      back_buffer_color_format_ =
+          (attrib_helper.alpha_size != 0 && alpha_bits > 0) ? GL_RGBA : GL_RGB;
+      back_buffer_has_depth_ = attrib_helper.depth_size != 0 && depth_bits > 0;
+      back_buffer_has_stencil_ =
+          attrib_helper.stencil_size != 0 && stencil_bits > 0;
+    }
+  }
+
+  return alpha_bits;
+}
+
+void GLES2DecoderImpl::ApplySurfaceFeatures() {
+  supports_post_sub_buffer_ = surface_->SupportsPostSubBuffer();
+  if (workarounds()
+          .disable_post_sub_buffers_for_onscreen_surfaces &&
+      !surface_->IsOffscreen())
+    supports_post_sub_buffer_ = false;
+
+  supports_swap_buffers_with_damage_ =
+      surface_->SupportsSwapBuffersWithDamage();
+
+  supports_commit_overlay_planes_ = surface_->SupportsCommitOverlayPlanes();
+
+  supports_async_swap_ = surface_->SupportsAsyncSwap();
+}
+
 bool GLES2DecoderImpl::Initialize(
     const scoped_refptr<gl::GLSurface>& surface,
     const scoped_refptr<gl::GLContext>& context,
@@ -3007,6 +3193,9 @@ bool GLES2DecoderImpl::Initialize(
   DCHECK(context->IsCurrent(surface.get()));
   DCHECK(!context_.get());
 
+  // Save the attrib helper data for future use in ApplySurfaceFeatures.
+  attrib_helper_ = attrib_helper;
+
   surfaceless_ = surface->IsSurfaceless() && !offscreen;
 
   set_initialized();
@@ -3179,100 +3368,12 @@ bool GLES2DecoderImpl::Initialize(
   CHECK_GL_ERROR();
 
   // cache ALPHA_BITS result for re-use with clear behaviour
-  GLint alpha_bits = 0;
+  GLint alpha_bits = ApplySurfaceAttributesAndGetAlphaBits(
+      offscreen, attrib_helper);
 
   if (offscreen) {
-    offscreen_buffer_should_have_alpha_ = attrib_helper.alpha_size > 0;
-
-    // Whether the offscreen buffer texture should have an alpha channel. Does
-    // not include logic from workarounds.
-    bool offscreen_buffer_texture_needs_alpha =
-        offscreen_buffer_should_have_alpha_ ||
-        (ChromiumImageNeedsRGBEmulation() &&
-         attrib_helper.should_use_native_gmb_for_backbuffer);
-
-    if (attrib_helper.samples > 0 && attrib_helper.sample_buffers > 0 &&
-        features().chromium_framebuffer_multisample) {
-      // Per ext_framebuffer_multisample spec, need max bound on sample count.
-      // max_sample_count must be initialized to a sane value.  If
-      // glGetIntegerv() throws a GL error, it leaves its argument unchanged.
-      GLint max_sample_count = 1;
-      glGetIntegerv(GL_MAX_SAMPLES_EXT, &max_sample_count);
-      offscreen_target_samples_ = std::min(attrib_helper.samples,
-                                           max_sample_count);
-    } else {
-      offscreen_target_samples_ = 1;
-    }
-    offscreen_target_buffer_preserved_ = attrib_helper.buffer_preserved;
-
-    if (gl_version_info().is_es) {
-      const bool rgb8_supported = context_->HasExtension("GL_OES_rgb8_rgba8");
-      // The only available default render buffer formats in GLES2 have very
-      // little precision.  Don't enable multisampling unless 8-bit render
-      // buffer formats are available--instead fall back to 8-bit textures.
-      if (rgb8_supported && offscreen_target_samples_ > 1) {
-        offscreen_target_color_format_ =
-            offscreen_buffer_texture_needs_alpha ? GL_RGBA8 : GL_RGB8;
-      } else {
-        offscreen_target_samples_ = 1;
-        offscreen_target_color_format_ =
-            offscreen_buffer_texture_needs_alpha ||
-                    workarounds().disable_gl_rgb_format
-                ? GL_RGBA
-                : GL_RGB;
-      }
-
-      // ANGLE only supports packed depth/stencil formats, so use it if it is
-      // available.
-      const bool depth24_stencil8_supported =
-          feature_info_->feature_flags().packed_depth24_stencil8;
-      VLOG(1) << "GL_OES_packed_depth_stencil "
-              << (depth24_stencil8_supported ? "" : "not ") << "supported.";
-      if ((attrib_helper.depth_size > 0 || attrib_helper.stencil_size > 0) &&
-          depth24_stencil8_supported) {
-        offscreen_target_depth_format_ = GL_DEPTH24_STENCIL8;
-        offscreen_target_stencil_format_ = 0;
-      } else {
-        // It may be the case that this depth/stencil combination is not
-        // supported, but this will be checked later by CheckFramebufferStatus.
-        offscreen_target_depth_format_ = attrib_helper.depth_size > 0 ?
-            GL_DEPTH_COMPONENT16 : 0;
-        offscreen_target_stencil_format_ = attrib_helper.stencil_size > 0 ?
-            GL_STENCIL_INDEX8 : 0;
-      }
-    } else {
-      offscreen_target_color_format_ =
-          offscreen_buffer_texture_needs_alpha ||
-                  workarounds().disable_gl_rgb_format
-              ? GL_RGBA
-              : GL_RGB;
-
-      // If depth is requested at all, use the packed depth stencil format if
-      // it's available, as some desktop GL drivers don't support any non-packed
-      // formats for depth attachments.
-      const bool depth24_stencil8_supported =
-          feature_info_->feature_flags().packed_depth24_stencil8;
-      VLOG(1) << "GL_EXT_packed_depth_stencil "
-              << (depth24_stencil8_supported ? "" : "not ") << "supported.";
-
-      if ((attrib_helper.depth_size > 0 || attrib_helper.stencil_size > 0) &&
-          depth24_stencil8_supported) {
-        offscreen_target_depth_format_ = GL_DEPTH24_STENCIL8;
-        offscreen_target_stencil_format_ = 0;
-      } else {
-        offscreen_target_depth_format_ = attrib_helper.depth_size > 0 ?
-            GL_DEPTH_COMPONENT : 0;
-        offscreen_target_stencil_format_ = attrib_helper.stencil_size > 0 ?
-            GL_STENCIL_INDEX : 0;
-      }
-    }
-
-    offscreen_saved_color_format_ = offscreen_buffer_texture_needs_alpha ||
-                                            workarounds().disable_gl_rgb_format
-                                        ? GL_RGBA
-                                        : GL_RGB;
-
     gfx::Size initial_size = attrib_helper.offscreen_framebuffer_size;
+
     if (initial_size.IsEmpty()) {
       // If we're an offscreen surface with zero width and/or height, set to a
       // non-zero size so that we have a complete framebuffer for operations
@@ -3285,49 +3386,6 @@ bool GLES2DecoderImpl::Initialize(
     state_.viewport_width = initial_size.width();
     state_.viewport_height = initial_size.height();
   } else {
-    glBindFramebufferEXT(GL_FRAMEBUFFER, GetBackbufferServiceId());
-    // These are NOT if the back buffer has these proprorties. They are
-    // if we want the command buffer to enforce them regardless of what
-    // the real backbuffer is assuming the real back buffer gives us more than
-    // we ask for. In other words, if we ask for RGB and we get RGBA then we'll
-    // make it appear RGB. If on the other hand we ask for RGBA nd get RGB we
-    // can't do anything about that.
-
-    if (!surfaceless_) {
-      GLint depth_bits = 0;
-      GLint stencil_bits = 0;
-
-      bool default_fb = (GetBackbufferServiceId() == 0);
-
-      if (gl_version_info().is_desktop_core_profile) {
-        glGetFramebufferAttachmentParameterivEXT(
-            GL_FRAMEBUFFER,
-            default_fb ? GL_BACK_LEFT : GL_COLOR_ATTACHMENT0,
-            GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE, &alpha_bits);
-        glGetFramebufferAttachmentParameterivEXT(
-            GL_FRAMEBUFFER,
-            default_fb ? GL_DEPTH : GL_DEPTH_ATTACHMENT,
-            GL_FRAMEBUFFER_ATTACHMENT_DEPTH_SIZE, &depth_bits);
-        glGetFramebufferAttachmentParameterivEXT(
-            GL_FRAMEBUFFER,
-            default_fb ? GL_STENCIL : GL_STENCIL_ATTACHMENT,
-            GL_FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE, &stencil_bits);
-      } else {
-        glGetIntegerv(GL_ALPHA_BITS, &alpha_bits);
-        glGetIntegerv(GL_DEPTH_BITS, &depth_bits);
-        glGetIntegerv(GL_STENCIL_BITS, &stencil_bits);
-      }
-
-      // This checks if the user requested RGBA and we have RGBA then RGBA. If
-      // the user requested RGB then RGB. If the user did not specify a
-      // preference than use whatever we were given. Same for DEPTH and STENCIL.
-      back_buffer_color_format_ =
-          (attrib_helper.alpha_size != 0 && alpha_bits > 0) ? GL_RGBA : GL_RGB;
-      back_buffer_has_depth_ = attrib_helper.depth_size != 0 && depth_bits > 0;
-      back_buffer_has_stencil_ =
-          attrib_helper.stencil_size != 0 && stencil_bits > 0;
-    }
-
     state_.viewport_width = surface->GetSize().width();
     state_.viewport_height = surface->GetSize().height();
   }
@@ -3466,17 +3524,7 @@ bool GLES2DecoderImpl::Initialize(
     }
   }
 
-  supports_post_sub_buffer_ = surface->SupportsPostSubBuffer();
-  if (workarounds()
-          .disable_post_sub_buffers_for_onscreen_surfaces &&
-      !surface->IsOffscreen())
-    supports_post_sub_buffer_ = false;
-
-  supports_swap_buffers_with_damage_ = surface->SupportsSwapBuffersWithDamage();
-
-  supports_commit_overlay_planes_ = surface->SupportsCommitOverlayPlanes();
-
-  supports_async_swap_ = surface->SupportsAsyncSwap();
+  ApplySurfaceFeatures();
 
   if (workarounds().reverse_point_sprite_coord_origin) {
     glPointParameteri(GL_POINT_SPRITE_COORD_ORIGIN, GL_LOWER_LEFT);
@@ -16937,6 +16985,143 @@ void GLES2DecoderImpl::DoApplyScreenSpaceAntialiasingCHROMIUM() {
   }
 }
 
+void GLES2DecoderImpl::DoSetSurfaceHandleCHROMIUM(GLint surface_handle) {
+#if defined(OS_ANDROID)
+  if (surface_handle) {
+    // Use the specified surface as a render target, and save decoder
+    // state so that this can be reverted later.
+
+    // We don't support switching between different surfaces. If you
+    // need this, you have to make a call with surface_handle=0 in
+    // between.
+    if (!surface_->IsOffscreen()) {
+      LOCAL_SET_GL_ERROR(
+          GL_INVALID_OPERATION,
+          "glSetSurfaceHandleCHROMIUM",
+          "Current surface is not offscreen, cannot redirect drawing");
+      return;
+    }
+
+    // One-time initialization when first activating a direct draw surface.
+    ANativeWindow* window =
+        gpu::GpuSurfaceLookup::GetInstance()->AcquireNativeWidget(
+            surface_handle);
+    if (!window) {
+      LOCAL_SET_GL_ERROR(
+          GL_INVALID_OPERATION,
+          "glSetSurfaceHandleCHROMIUM",
+          "Could not acquire a window for this surface handle");
+      return;
+    }
+
+    direct_render_state_.reset(new DirectRenderState());
+    direct_render_state_->direct_surface =
+        new gl::NativeViewGLSurfaceEGL(window);
+
+    // It would be useful if we could get a custom surface format such
+    // as one with a depth buffer here, but the new surface must use
+    // the same format as the one originally used for creating the GL
+    // context. Anything else just results in BAD_MATCH errors.
+    bool initialize_success = direct_render_state_->direct_surface->Initialize(
+        surface_->GetFormat());
+    if (!initialize_success) {
+      LOG(ERROR) << "Direct render surface init failed for handle " <<
+          surface_handle;
+    }
+    ANativeWindow_release(window);
+
+    direct_render_state_->orig_surface = surface_;
+
+    // Offscreen surfaces don't support SwapBuffers, make sure it's not
+    // considered offscreen. Various code in this file checks for is_offscreen
+    // = !!offscreen_target_frame_buffer_.get().
+    direct_render_state_->orig_offscreen_target_frame_buffer =
+        std::move(offscreen_target_frame_buffer_);
+    offscreen_target_frame_buffer_.reset(nullptr);
+
+    // Activate the surface. This does *not* rebind the active framebuffer,
+    // callers must do so themselves as needed.
+    SetSurface(direct_render_state_->direct_surface);
+    ApplySurfaceAttributesAndGetAlphaBits(false, attrib_helper_);
+    ApplySurfaceFeatures();
+
+    // If we're using the default draw framebuffer, make the surface current.
+    if (!framebuffer_state_.bound_draw_framebuffer.get()) {
+      context_->MakeCurrent(surface_.get());

[piman, 2016/11/09 22:26:09]

You need to do the MakeCurrent before the ApplySurfaceAttributesAndGetAlphaBits,
otherwise the queries will return properties of the old surface.

Either way, you want to do the MakeCurrent regardless of whether there's a bound
FBO or not, otherwise if the client unbinds the FBO and tries to draw, it'll go
to the wrong surface.

[klausw, 2016/11/10 02:28:07]

Done, I was misunderstanding how MakeCurrent interacts with FBOs. So MakeCurrent
basically maps the default backbuffer destination to the specified surface, even
if you're not currently using the default backbuffer due to a bound FBO?

+      // This is a new surface, so MakeCurrent resets viewport and scissor
+      // dimensions. Re-apply the settings from the current state.

[piman, 2016/11/09 22:26:09]

Actually, MakeCurrent only sets viewport/scissor on a new context, not a new
surface, so I don't think you need this.

[klausw, 2016/11/10 02:28:08]

Done.

+      glViewport(state_.viewport_x, state_.viewport_y,
+                 state_.viewport_width, state_.viewport_height);
+      glScissor(state_.scissor_x,
+                state_.scissor_y,
+                state_.scissor_width,
+                state_.scissor_height);
+
+      // DoReadBuffer and DoDrawBuffersEXT may have modified bindings.
+      // We don't care for modifications to a user-bound framebuffer
+      // since those will still be valid. Target GL_NONE also remains
+      // valid, so we just need to undo the GL_BACK ->
+      // GL_COLOR_ATTACHMENT0 mapping. Caveat: the current buffer
+      // content is not preserved across this surface switch.
+      if (back_buffer_read_buffer_ == GL_BACK) {
+        glReadBuffer(GL_BACK);
+      }
+      if (back_buffer_draw_buffer_ == GL_BACK) {
+        GLenum buf = GL_BACK;
+        glDrawBuffersARB(1, &buf);
+      }
+    }
+  } else {
+    // Restore original state for drawing offscreen.
+
+    // Ignore redundant calls.
+    if (!direct_render_state_) {
+      LOCAL_SET_GL_ERROR(
+          GL_INVALID_OPERATION,
+          "glSetSurfaceHandleCHROMIUM",
+          "Surface was not set by glSetSurfaceHandleCHROMIUM, cannot unset");
+    }
+
+    offscreen_target_frame_buffer_ =
+        std::move(direct_render_state_->orig_offscreen_target_frame_buffer);
+
+    SetSurface(direct_render_state_->orig_surface);
+    ApplySurfaceAttributesAndGetAlphaBits(true, attrib_helper_);

[piman, 2016/11/09 22:26:10]

You probably want to MakeCurrent here to the offscreen surface too. It doesn't
exactly matter for correctness, but as long as the direct surface is current, it
won't be freed.

[klausw, 2016/11/10 02:28:07]

Done.

+    ApplySurfaceFeatures();
+
+    // The viewport size may have been changed, resize it.
+    if (!ResizeOffscreenFramebuffer(
+            gfx::Size(state_.viewport_width, state_.viewport_height))) {

[piman, 2016/11/09 22:26:09]

You want to use offscreen_size_, not the viewport size, which is orthogonal to
the back buffer size.
Actually, what is the expectation relative to the back buffer size, if it was
resized while a separate surface was attached, and we go back to the offscreen
one? I.e. should we reset offscreen_size_ to direct_surface->GetSize() ?

[klausw, 2016/11/10 02:28:07]

Changed, I think the most intuitive behavior would be to copy the
current size from offscreen to surface on activation and the reverse
on deactivation.

+      LOCAL_SET_GL_ERROR(
+          GL_INVALID_OPERATION,
+          "glSetSurfaceHandleCHROMIUM",
+          "Failed to resize offscreen framebuffer");
+    }
+
+    // If we're using the default draw framebuffer, fix things.
+    if (!framebuffer_state_.bound_draw_framebuffer.get()) {
+      // DoDrawBuffersEXT: remap GL_BACK to GL_COLOR_ATTACHMENT0.
+      if (back_buffer_read_buffer_ == GL_BACK) {
+        glReadBuffer(GL_COLOR_ATTACHMENT0);
+      }
+      if (back_buffer_draw_buffer_ == GL_BACK) {
+        GLenum buf = GL_COLOR_ATTACHMENT0;
+        glDrawBuffersARB(1, &buf);
+      }
+    }
+
+    // Trust the state struct's destructors to handle refcounting for
+    // surfaces and trigger apprpriate cleanup.
+    direct_render_state_.reset();
+  }
+#else  // !OS_ANDROID
+  LOCAL_SET_GL_ERROR(
+      GL_INVALID_OPERATION,
+      "glSetSurfaceHandleCHROMIUM",
+      "Direct render surface is unsupported on this platform");
+#endif
+}
+
 void GLES2DecoderImpl::DoInsertEventMarkerEXT(
     GLsizei length, const GLchar* marker) {
   if (!marker) {