Add gfx::SurfaceFactoryWebview

android_webview/browser/in_process_view_renderer.cc

Index: android_webview/browser/in_process_view_renderer.cc
diff --git a/android_webview/browser/in_process_view_renderer.cc b/android_webview/browser/in_process_view_renderer.cc
index a714956e08064795b9da15890fab5f8cddc411f6..260fb6c5ecd789777a62db63a576a24952d5195f 100644
--- a/android_webview/browser/in_process_view_renderer.cc
+++ b/android_webview/browser/in_process_view_renderer.cc
@@ -6,6 +6,7 @@
 
 #include <android/bitmap.h>
 
+#include "android_webview/browser/aw_gl_surface.h"
 #include "android_webview/browser/scoped_app_gl_state_restore.h"
 #include "android_webview/common/aw_switches.h"
 #include "android_webview/public/browser/draw_gl.h"
@@ -160,6 +161,23 @@ bool ScopedAllowGL::allow_gl = false;
 
 base::LazyInstance<GLViewRendererManager>::Leaky g_view_renderer_manager;
 
+class ScopedSetSurfaceFactory {
+ public:
+  ScopedSetSurfaceFactory(InProcessViewRenderer* factory);

[piman, 2013/08/06 21:41:55]

nit: explicit

+  ~ScopedSetSurfaceFactory();
+};
+
+ScopedSetSurfaceFactory::ScopedSetSurfaceFactory(
+    InProcessViewRenderer* factory) {
+  DCHECK(factory);
+  DCHECK(!gfx::GLFactoryAndroid::GetCurrent());
+  gfx::GLFactoryAndroid::SetCurrent(factory);
+}
+
+ScopedSetSurfaceFactory::~ScopedSetSurfaceFactory() {
+  gfx::GLFactoryAndroid::SetCurrent(NULL);
+}
+
 }  // namespace
 
 // Called from different threads!
@@ -289,6 +307,30 @@ bool InProcessViewRenderer::OnDraw(jobject java_canvas,
   return result;
 }
 
+scoped_refptr<gfx::GLSurface>
+InProcessViewRenderer::CreateNonOwnedViewSurface() {
+  DCHECK(!hardware_initialized_);
+  DCHECK(!gl_surface_);
+  gl_surface_ = new AwGLSurface;
+  return gl_surface_;
+}
+
+bool InProcessViewRenderer::InitializeHwDraw() {
+  TRACE_EVENT0("android_webview", "InitializeHwDraw");
+
+  {
+    ScopedSetSurfaceFactory setter(this);

[piman, 2013/08/06 21:41:55]

So, essentially, you're passing |this| all the way to CreateNonOwnedViewSurface
through a global variable.

Can we fix this? E.g. by creating the GLSurface here, and passing it to
InitializeHwDraw that can then give it to the
WebGraphicsContext3DInProcessCommandBufferImpl?

[no sievers, 2013/08/06 21:44:50]

Just chatted with Bo and realized (a little too late) that having a clean
failure path that does not create a GLSurfaceStub for WebView might be worth the
extra layer of indirection he had earlier. (Esp. since a GLSurfaceStub will
somewhat work except for not binding the right FBO which is only sometimes
needed.)

[boliu, 2013/08/06 21:51:16]

That's actually attempt 1: https://codereview.chromium.org/22066002/

Could you take a look at the gpu and ui changes there and see if you prefer that
over this factory approach.

Concern there was GLSurface is not thread safe so passing it from the client to
server side using scoped_refptr is wrong.

+    hardware_failed_ = !compositor_->InitializeHwDraw();
+  }
+
+  hardware_initialized_ = true;
+  if (!hardware_failed_) {
+    DCHECK(gl_surface_);
+  }
+
+  return !hardware_failed_;
+}
+
 void InProcessViewRenderer::DrawGL(AwDrawGLInfo* draw_info) {
   TRACE_EVENT0("android_webview", "InProcessViewRenderer::DrawGL");
   DCHECK(visible_);
@@ -309,12 +351,9 @@ void InProcessViewRenderer::DrawGL(AwDrawGLInfo* draw_info) {
   ScopedAllowGL allow_gl;
 
   if (attached_to_window_ && compositor_ && !hardware_initialized_) {
-    TRACE_EVENT0("android_webview", "InitializeHwDraw");
-    hardware_failed_ = !compositor_->InitializeHwDraw();
-    hardware_initialized_ = true;
-    last_egl_context_ = current_context;
-
-    if (hardware_failed_)
+    if (InitializeHwDraw())
+      last_egl_context_ = current_context;
+    else
       return;
   }
 
@@ -330,7 +369,6 @@ void InProcessViewRenderer::DrawGL(AwDrawGLInfo* draw_info) {
     TRACE_EVENT_INSTANT0(
         "android_webview", "EGLContextChanged", TRACE_EVENT_SCOPE_THREAD);
   }
-  last_egl_context_ = current_context;
 
   if (!compositor_) {
     TRACE_EVENT_INSTANT0(
@@ -338,21 +376,26 @@ void InProcessViewRenderer::DrawGL(AwDrawGLInfo* draw_info) {
     return;
   }
 
+  DCHECK(gl_surface_);
+  gl_surface_->SetBackingFrameBufferObject(
+      state_restore.framebuffer_binding_ext());
+
   gfx::Transform transform;
   transform.matrix().setColMajorf(draw_info->transform);
   transform.Translate(scroll_at_start_of_frame_.x(),
                       scroll_at_start_of_frame_.y());
-  // TODO(joth): Check return value.
-  block_invalidates_ = true;
   gfx::Rect clip_rect(draw_info->clip_left,
                       draw_info->clip_top,
                       draw_info->clip_right - draw_info->clip_left,
                       draw_info->clip_bottom - draw_info->clip_top);
+  block_invalidates_ = true;
+  // TODO(joth): Check return value.
   compositor_->DemandDrawHw(gfx::Size(draw_info->width, draw_info->height),
                             transform,
                             clip_rect,
                             state_restore.stencil_enabled());
   block_invalidates_ = false;
+  gl_surface_->ResetBackingFrameBufferObject();
 
   UpdateCachedGlobalVisibleRect();
   bool drew_full_visible_rect = clip_rect.Contains(cached_global_visible_rect_);
@@ -552,6 +595,7 @@ void InProcessViewRenderer::OnDetachedFromWindow() {
     ScopedAllowGL allow_gl;
     compositor_->ReleaseHwDraw();
     hardware_initialized_ = false;
+    gl_surface_ = NULL;
   }
 
   attached_to_window_ = false;