Fix gpu command buffer use after free by GrContext

ContextProviderCommandBuffer owns a WebGraphicsContext3DCommandBufferImpl and a
GrContextForWebGraphicsContext3D via scoped_ptr. The problem was
that the GrContext object held by GrContextForWebGraphicsContext3D
depended on interface pointers that reference an interface that is owned
by WebGraphicsContext3DCommandBufferImpl, so whenever the
GrContext outlived the ContextProviderCommandBuffer, we ended up in a
state where the interface function pointers are deallocated, but still
referenced. Then, attempts to use the GrContext would result in using
deallocated function pointers. Because the GrContext is a ref counted
object, it can easily outlive the ContextProviderCommandBuffer. This led to
a dangerous situation where we had to be careful about object destruction
order.

This CL fixes the problem for good by wrapping the ownership of the
WebGraphicsContext3DCommandBufferImpl into a subclass of
GrGLInterface, which is a ref counted object that can be owned jointly by
the GrContext and the ContextProviderCommandBuffer, thus guaranteeing
that the command buffer interface will remain valid for the lifetimes of the
GrContext and of the ContextProviderCommandBuffer.

BUG=551143
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1414683003

Cr-Commit-Position: refs/heads/master@{#359493}

2 files changed, 35 insertions, 20 deletions

diff --git a/content/browser/android/in_process/context_provider_in_process.cc b/content/browser/android/in_process/context_provider_in_process.cc
index fda13aa..71f8010 100644
--- a/content/browser/android/in_process/context_provider_in_process.cc
+++ b/content/browser/android/in_process/context_provider_in_process.cc
@@ -22,11 +22,11 @@ class ContextProviderInProcess::LostContextCallbackProxy
  public:
   explicit LostContextCallbackProxy(ContextProviderInProcess* provider)
       : provider_(provider) {
-    provider_->context3d_->setContextLostCallback(this);
+    provider_->WebContext3DImpl()->setContextLostCallback(this);
   }
 
   virtual ~LostContextCallbackProxy() {
-    provider_->context3d_->setContextLostCallback(NULL);
+    provider_->WebContext3DImpl()->setContextLostCallback(NULL);
   }
 
   virtual void onContextLost() {
@@ -49,10 +49,12 @@ scoped_refptr<ContextProviderInProcess> ContextProviderInProcess::Create(
 ContextProviderInProcess::ContextProviderInProcess(
     scoped_ptr<WebGraphicsContext3DInProcessCommandBufferImpl> context3d,
     const std::string& debug_name)
-    : context3d_(context3d.Pass()),
-      debug_name_(debug_name) {
+    : debug_name_(debug_name) {
   DCHECK(main_thread_checker_.CalledOnValidThread());
-  DCHECK(context3d_);
+  DCHECK(context3d);
+  gr_interface_ = skia::AdoptRef(new GrGLInterfaceForWebGraphicsContext3D(
+      context3d.Pass()));
+  DCHECK(gr_interface_->WebContext3D());
   context_thread_checker_.DetachFromThread();
 }
 
@@ -65,11 +67,20 @@ blink::WebGraphicsContext3D* ContextProviderInProcess::WebContext3D() {
   DCHECK(lost_context_callback_proxy_);  // Is bound to thread.
   DCHECK(context_thread_checker_.CalledOnValidThread());
 
-  return context3d_.get();
+  return WebContext3DImpl();
+}
+
+gpu_blink::WebGraphicsContext3DInProcessCommandBufferImpl*
+    ContextProviderInProcess::WebContext3DImpl() {
+  DCHECK(gr_interface_->WebContext3D());
+
+  return
+      static_cast<gpu_blink::WebGraphicsContext3DInProcessCommandBufferImpl*>(
+          gr_interface_->WebContext3D());
 }
 
 bool ContextProviderInProcess::BindToCurrentThread() {
-  DCHECK(context3d_);
+  DCHECK(WebContext3DImpl());
 
   // This is called on the thread the context will be used.
   DCHECK(context_thread_checker_.CalledOnValidThread());
@@ -77,14 +88,15 @@ bool ContextProviderInProcess::BindToCurrentThread() {
   if (lost_context_callback_proxy_)
     return true;
 
-  if (!context3d_->InitializeOnCurrentThread())
+  if (!WebContext3DImpl()->InitializeOnCurrentThread())
     return false;
 
+  gr_interface_->BindToCurrentThread();
   InitializeCapabilities();
 
   const std::string unique_context_name =
-      base::StringPrintf("%s-%p", debug_name_.c_str(), context3d_.get());
-  context3d_->traceBeginCHROMIUM("gpu_toplevel",
+      base::StringPrintf("%s-%p", debug_name_.c_str(), WebContext3DImpl());
+  WebContext3DImpl()->traceBeginCHROMIUM("gpu_toplevel",
                                  unique_context_name.c_str());
 
   lost_context_callback_proxy_.reset(new LostContextCallbackProxy(this));
@@ -96,9 +108,9 @@ void ContextProviderInProcess::DetachFromThread() {
 }
 
 void ContextProviderInProcess::InitializeCapabilities() {
-  capabilities_.gpu = context3d_->GetImplementation()->capabilities();
+  capabilities_.gpu = WebContext3DImpl()->GetImplementation()->capabilities();
 
-  size_t mapped_memory_limit = context3d_->GetMappedMemoryLimit();
+  size_t mapped_memory_limit = WebContext3DImpl()->GetMappedMemoryLimit();
   capabilities_.max_transfer_buffer_usage_bytes =
       mapped_memory_limit ==
               WebGraphicsContext3DInProcessCommandBufferImpl::kNoLimit
@@ -114,21 +126,21 @@ ContextProviderInProcess::ContextCapabilities() {
 }
 
 ::gpu::gles2::GLES2Interface* ContextProviderInProcess::ContextGL() {
-  DCHECK(context3d_);
+  DCHECK(WebContext3DImpl());
   DCHECK(lost_context_callback_proxy_);  // Is bound to thread.
   DCHECK(context_thread_checker_.CalledOnValidThread());
 
-  return context3d_->GetGLInterface();
+  return WebContext3DImpl()->GetGLInterface();
 }
 
 ::gpu::ContextSupport* ContextProviderInProcess::ContextSupport() {
-  DCHECK(context3d_);
+  DCHECK(WebContext3DImpl());
   if (!lost_context_callback_proxy_)
     return NULL;  // Not bound to anything.
 
   DCHECK(context_thread_checker_.CalledOnValidThread());
 
-  return context3d_->GetContextSupport();
+  return WebContext3DImpl()->GetContextSupport();
 }
 
 class GrContext* ContextProviderInProcess::GrContext() {
@@ -138,7 +150,7 @@ class GrContext* ContextProviderInProcess::GrContext() {
   if (gr_context_)
     return gr_context_->get();
 
-  gr_context_.reset(new GrContextForWebGraphicsContext3D(context3d_.get()));
+  gr_context_.reset(new GrContextForWebGraphicsContext3D(gr_interface_));
   return gr_context_->get();
 }
 
@@ -151,7 +163,7 @@ void ContextProviderInProcess::InvalidateGrContext(uint32_t state) {
 }
 
 void ContextProviderInProcess::SetupLock() {
-  context3d_->SetLock(&context_lock_);
+  WebContext3DImpl()->SetLock(&context_lock_);
 }
 
 base::Lock* ContextProviderInProcess::GetLock() {
diff --git a/content/browser/android/in_process/context_provider_in_process.h b/content/browser/android/in_process/context_provider_in_process.h
index 761caf0..81726d6 100644
--- a/content/browser/android/in_process/context_provider_in_process.h
+++ b/content/browser/android/in_process/context_provider_in_process.h
@@ -12,6 +12,7 @@
 #include "base/synchronization/lock.h"
 #include "base/threading/thread_checker.h"
 #include "cc/blink/context_provider_web_context.h"
+#include "skia/ext/refptr.h"
 
 namespace blink { class WebGraphicsContext3D; }
 
@@ -22,6 +23,7 @@ class WebGraphicsContext3DInProcessCommandBufferImpl;
 namespace content {
 
 class GrContextForWebGraphicsContext3D;
+class GrGLInterfaceForWebGraphicsContext3D;
 
 class ContextProviderInProcess
     : NON_EXPORTED_BASE(public cc_blink::ContextProviderWebContext) {
@@ -41,6 +43,8 @@ class ContextProviderInProcess
   // cc_blink::ContextProviderWebContext:
   blink::WebGraphicsContext3D* WebContext3D() override;
 
+  gpu_blink::WebGraphicsContext3DInProcessCommandBufferImpl* WebContext3DImpl();
+
   // cc::ContextProvider:
   bool BindToCurrentThread() override;
   void DetachFromThread() override;
@@ -61,8 +65,7 @@ class ContextProviderInProcess
   base::ThreadChecker main_thread_checker_;
   base::ThreadChecker context_thread_checker_;
 
-  scoped_ptr<gpu_blink::WebGraphicsContext3DInProcessCommandBufferImpl>
-      context3d_;
+  skia::RefPtr<GrGLInterfaceForWebGraphicsContext3D> gr_interface_;
   scoped_ptr<GrContextForWebGraphicsContext3D> gr_context_;
 
   LostContextCallback lost_context_callback_;