cc: Drop ContextProvider references on the impl thread before quitting

cc: Drop ContextProvider references on the impl thread before quitting

When passing a TextureMailbox from the impl thread out to the cc
embedder on the main thread, we need to keep a ref on the ContextProvider
that owns the texture in the mailbox, to keep the mailbox alive while
the embedder is receiving it.

However if the compositor shutsdown and the impl thread goes away, we
must have already dropped the ref on the ContextProvider or it will be
destroyed (a) on the wrong thread and (b) after the global GPU process
objects it holds pointers to have been destroyed. Crash will occur.

So, we add a TextureMailboxDeleter class that the GLRenderer uses to
generate a TextureMailbox release callback for the main thread instead
of rolling its own thing. The TMDeleter class' lifetime is bounded by
the life of the compositor instance on the impl thread, so it is
destroyed before the impl thread is joined. And when it is destroyed it
deletes the textures it was meant to destroy, and drops its ref on the
context provider.

BUG=283625
R=piman

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222207

[danakj, 2013-09-09 14:37:43]

ping

[piman, 2013-09-09 20:51:52]

lgtm

[commit-bot: I haz the power, 2013-09-09 20:54:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23454014/9001

[commit-bot: I haz the power, 2013-09-09 20:54:27]

Failed to apply patch for cc/output/gl_renderer.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/output/gl_renderer.cc
  Hunk #2 FAILED at 127.
  Hunk #3 FAILED at 148.
  Hunk #4 succeeded at 2184 (offset 13 lines).
  Hunk #5 succeeded at 2227 (offset 13 lines).
  2 out of 5 hunks FAILED -- saving rejects to file cc/output/gl_renderer.cc.rej

Patch:       cc/output/gl_renderer.cc
Index: cc/output/gl_renderer.cc
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index
a80feac4ecc9f97f3fd5e731d99a43cc7ea98a99..b0355063d183ff87ad905586d9e209ae299f5d48
100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -33,6 +33,7 @@
 #include "cc/resources/layer_quad.h"
 #include "cc/resources/scoped_resource.h"
 #include "cc/resources/sync_point_helper.h"
+#include "cc/resources/texture_mailbox_deleter.h"
 #include "cc/trees/damage_tracker.h"
 #include "cc/trees/proxy.h"
 #include "cc/trees/single_thread_proxy.h"
@@ -126,13 +127,18 @@ struct GLRenderer::PendingAsyncReadPixels {
   DISALLOW_COPY_AND_ASSIGN(PendingAsyncReadPixels);
 };
 
-scoped_ptr<GLRenderer> GLRenderer::Create(RendererClient* client,
-                                          OutputSurface* output_surface,
-                                          ResourceProvider* resource_provider,
-                                          int highp_threshold_min,
-                                          bool use_skia_gpu_backend) {
-  scoped_ptr<GLRenderer> renderer(new GLRenderer(
-      client, output_surface, resource_provider, highp_threshold_min));
+scoped_ptr<GLRenderer> GLRenderer::Create(
+    RendererClient* client,
+    OutputSurface* output_surface,
+    ResourceProvider* resource_provider,
+    TextureMailboxDeleter* texture_mailbox_deleter,
+    int highp_threshold_min,
+    bool use_skia_gpu_backend) {
+  scoped_ptr<GLRenderer> renderer(new GLRenderer(client,
+                                                 output_surface,
+                                                 resource_provider,
+                                                 texture_mailbox_deleter,
+                                                 highp_threshold_min));
   if (!renderer->Initialize())
     return scoped_ptr<GLRenderer>();
   if (use_skia_gpu_backend) {
@@ -147,11 +153,13 @@ scoped_ptr<GLRenderer> GLRenderer::Create(RendererClient*
client,
 GLRenderer::GLRenderer(RendererClient* client,
                        OutputSurface* output_surface,
                        ResourceProvider* resource_provider,
+                       TextureMailboxDeleter* texture_mailbox_deleter,
                        int highp_threshold_min)
     : DirectRenderer(client, output_surface, resource_provider),
       offscreen_framebuffer_id_(0),
       shared_geometry_quad_(gfx::RectF(-0.5f, -0.5f, 1.0f, 1.0f)),
       context_(output_surface->context_provider()->Context3d()),
+      texture_mailbox_deleter_(texture_mailbox_deleter),
       is_backbuffer_discarded_(false),
       discard_backbuffer_when_not_visible_(false),
       is_using_bind_uniform_(false),
@@ -2170,31 +2178,6 @@ void GLRenderer::GetFramebufferPixels(void* pixels,
gfx::Rect rect) {
                          AsyncGetFramebufferPixelsCleanupCallback());
 }
 
-static void DeleteTextureReleaseCallbackOnImplThread(
-    const scoped_refptr<ContextProvider>& context_provider,
-    unsigned texture_id,
-    unsigned sync_point,
-    bool lost_resource) {
-  if (sync_point)
-    context_provider->Context3d()->waitSyncPoint(sync_point);
-  context_provider->Context3d()->deleteTexture(texture_id);
-}
-
-static void DeleteTextureReleaseCallback(
-    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-    const scoped_refptr<ContextProvider>& context_provider,
-    unsigned texture_id,
-    unsigned sync_point,
-    bool lost_resource) {
-  task_runner->PostTask(
-      FROM_HERE,
-      base::Bind(&DeleteTextureReleaseCallbackOnImplThread,
-                 context_provider,
-                 texture_id,
-                 sync_point,
-                 lost_resource));
-}
-
 void GLRenderer::GetFramebufferPixelsAsync(
     gfx::Rect rect, scoped_ptr<CopyOutputRequest> request) {
   DCHECK(!request->IsEmpty());
@@ -2238,10 +2221,8 @@ void GLRenderer::GetFramebufferPixelsAsync(
     sync_point = context_->insertSyncPoint();
     scoped_ptr<TextureMailbox> texture_mailbox = make_scoped_ptr(
         new TextureMailbox(mailbox,
-                           base::Bind(&DeleteTextureReleaseCallback,
-                                      base::MessageLoopProxy::current(),
-                                      output_surface_->context_provider(),
-                                      texture_id),
+                           texture_mailbox_deleter_->GetReleaseCallback(
+                               output_surface_->context_provider(),
texture_id),
                            GL_TEXTURE_2D,
                            sync_point));
     request->SendTextureResult(window_rect.size(), texture_mailbox.Pass());

[commit-bot: I haz the power, 2013-09-09 21:08:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23454014/18001

[commit-bot: I haz the power, 2013-09-09 21:08:56]

Failed to apply patch for cc/output/gl_renderer.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/output/gl_renderer.cc
  Hunk #2 FAILED at 127.
  Hunk #3 FAILED at 148.
  Hunk #4 succeeded at 2184 (offset 2 lines).
  Hunk #5 succeeded at 2227 (offset 2 lines).
  2 out of 5 hunks FAILED -- saving rejects to file cc/output/gl_renderer.cc.rej

Patch:       cc/output/gl_renderer.cc
Index: cc/output/gl_renderer.cc
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index
f19f4415f37de38b2f6d940aad733b9c81f77bbb..19085e2a9ce1a72717896abe42c93b194dddabae
100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -33,6 +33,7 @@
 #include "cc/resources/layer_quad.h"
 #include "cc/resources/scoped_resource.h"
 #include "cc/resources/sync_point_helper.h"
+#include "cc/resources/texture_mailbox_deleter.h"
 #include "cc/trees/damage_tracker.h"
 #include "cc/trees/proxy.h"
 #include "cc/trees/single_thread_proxy.h"
@@ -126,13 +127,18 @@ struct GLRenderer::PendingAsyncReadPixels {
   DISALLOW_COPY_AND_ASSIGN(PendingAsyncReadPixels);
 };
 
-scoped_ptr<GLRenderer> GLRenderer::Create(RendererClient* client,
-                                          OutputSurface* output_surface,
-                                          ResourceProvider* resource_provider,
-                                          int highp_threshold_min,
-                                          bool use_skia_gpu_backend) {
-  scoped_ptr<GLRenderer> renderer(new GLRenderer(
-      client, output_surface, resource_provider, highp_threshold_min));
+scoped_ptr<GLRenderer> GLRenderer::Create(
+    RendererClient* client,
+    OutputSurface* output_surface,
+    ResourceProvider* resource_provider,
+    TextureMailboxDeleter* texture_mailbox_deleter,
+    int highp_threshold_min,
+    bool use_skia_gpu_backend) {
+  scoped_ptr<GLRenderer> renderer(new GLRenderer(client,
+                                                 output_surface,
+                                                 resource_provider,
+                                                 texture_mailbox_deleter,
+                                                 highp_threshold_min));
   if (!renderer->Initialize())
     return scoped_ptr<GLRenderer>();
   if (use_skia_gpu_backend) {
@@ -147,11 +153,13 @@ scoped_ptr<GLRenderer> GLRenderer::Create(RendererClient*
client,
 GLRenderer::GLRenderer(RendererClient* client,
                        OutputSurface* output_surface,
                        ResourceProvider* resource_provider,
+                       TextureMailboxDeleter* texture_mailbox_deleter,
                        int highp_threshold_min)
     : DirectRenderer(client, output_surface, resource_provider),
       offscreen_framebuffer_id_(0),
       shared_geometry_quad_(gfx::RectF(-0.5f, -0.5f, 1.0f, 1.0f)),
       context_(output_surface->context_provider()->Context3d()),
+      texture_mailbox_deleter_(texture_mailbox_deleter),
       is_backbuffer_discarded_(false),
       discard_backbuffer_when_not_visible_(false),
       is_using_bind_uniform_(false),
@@ -2181,31 +2189,6 @@ void GLRenderer::GetFramebufferPixels(void* pixels,
gfx::Rect rect) {
                          AsyncGetFramebufferPixelsCleanupCallback());
 }
 
-static void DeleteTextureReleaseCallbackOnImplThread(
-    const scoped_refptr<ContextProvider>& context_provider,
-    unsigned texture_id,
-    unsigned sync_point,
-    bool lost_resource) {
-  if (sync_point)
-    context_provider->Context3d()->waitSyncPoint(sync_point);
-  context_provider->Context3d()->deleteTexture(texture_id);
-}
-
-static void DeleteTextureReleaseCallback(
-    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-    const scoped_refptr<ContextProvider>& context_provider,
-    unsigned texture_id,
-    unsigned sync_point,
-    bool lost_resource) {
-  task_runner->PostTask(
-      FROM_HERE,
-      base::Bind(&DeleteTextureReleaseCallbackOnImplThread,
-                 context_provider,
-                 texture_id,
-                 sync_point,
-                 lost_resource));
-}
-
 void GLRenderer::GetFramebufferPixelsAsync(
     gfx::Rect rect, scoped_ptr<CopyOutputRequest> request) {
   DCHECK(!request->IsEmpty());
@@ -2249,10 +2232,8 @@ void GLRenderer::GetFramebufferPixelsAsync(
     sync_point = context_->insertSyncPoint();
     scoped_ptr<TextureMailbox> texture_mailbox = make_scoped_ptr(
         new TextureMailbox(mailbox,
-                           base::Bind(&DeleteTextureReleaseCallback,
-                                      base::MessageLoopProxy::current(),
-                                      output_surface_->context_provider(),
-                                      texture_id),
+                           texture_mailbox_deleter_->GetReleaseCallback(
+                               output_surface_->context_provider(),
texture_id),
                            GL_TEXTURE_2D,
                            sync_point));
     request->SendTextureResult(window_rect.size(), texture_mailbox.Pass());

[commit-bot: I haz the power, 2013-09-09 22:07:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23454014/22001

[commit-bot: I haz the power, 2013-09-10 00:19:16]

Retried try job too often on win7_aura for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-09-10 04:43:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23454014/22001

[commit-bot: I haz the power, 2013-09-10 08:25:17]

Change committed as 222207