aura: Fix raciness in context loss and recreation.

When the context is lost, we post a task to notify observers about
the loss. They should be able to use the old context to clean up
after themselves. However, if anyone came and asked for the shared
context in between the post task and it executing, the shared context
would be destroyed immediately since we looked in the getter if
the context was lost.

Instead, always return the same context3d from the shared context
getter, and only destroy it in the execution of the post task.

Also, the OwnedTexture class holds a raw pointer to the shared
context, but the OwnedTexture is not destroyed by the lost
context notifications (contrary to a misleading comment). So
when the context is lost, the OwnedTexture needs to drop its
raw pointer to the old context 3d, which will be deleted after
the notification. This ended up incorporating exactly the changes
made by cpu@ in https://codereview.chromium.org/23364002/.

Finally, the compositor thread shared context does not have a lost
context listener in the GpuProcessContextFactory, but we want it to
always be in the same share group as the main thread context. So
when we lost the main thread context and drop the reference on the
context3d, we will also do the same for the compositor thread's
context. This way both contexts are replaced on the main thread at
the same time. The next commit by the compositor will use a new
context for both threads, and a commit between the loss and the
post task executing will use the old contexts on both threads.

R=piman
BUG=275775

Review URL: https://chromiumcodereview.appspot.com/23055007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@219119 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 25 insertions, 9 deletions

diff --git a/content/browser/aura/gpu_process_transport_factory.cc b/content/browser/aura/gpu_process_transport_factory.cc
index 382c6a6..3c55282 100644
--- a/content/browser/aura/gpu_process_transport_factory.cc
+++ b/content/browser/aura/gpu_process_transport_factory.cc
@@ -74,6 +74,7 @@ class OwnedTexture : public ui::Texture, ImageTransportFactoryObserver {
   // ImageTransportFactory overrides:
   virtual void OnLostResources() OVERRIDE {
     DeleteTexture();
+    host_context_ = NULL;
   }
 
  protected:
@@ -90,9 +91,8 @@ class OwnedTexture : public ui::Texture, ImageTransportFactoryObserver {
     }
   }
 
-  // A raw pointer. This |ImageTransportClientTexture| will be destroyed
-  // before the |host_context_| via
-  // |ImageTransportFactoryObserver::OnLostContext()| handlers.
+  // The OnLostResources() callback will happen before this context
+  // pointer is destroyed.
   WebKit::WebGraphicsContext3D* host_context_;
   unsigned texture_id_;
 
@@ -387,8 +387,11 @@ void GpuProcessTransportFactory::RemoveObserver(
 
 scoped_refptr<cc::ContextProvider>
 GpuProcessTransportFactory::OffscreenContextProviderForMainThread() {
-  if (!shared_contexts_main_thread_.get() ||
-      shared_contexts_main_thread_->DestroyedOnMainThread()) {
+  // Don't check for DestroyedOnMainThread() here. We hear about context
+  // loss for this context through the lost context callback. If the context
+  // is lost, we want to leave this ContextProvider available until the lost
+  // context notification is sent to the ImageTransportFactoryObserver clients.
+  if (!shared_contexts_main_thread_.get()) {
     shared_contexts_main_thread_ = ContextProviderCommandBuffer::Create(
         GpuProcessTransportFactory::CreateOffscreenCommandBufferContext());
     if (shared_contexts_main_thread_) {
@@ -406,8 +409,10 @@ GpuProcessTransportFactory::OffscreenContextProviderForMainThread() {
 
 scoped_refptr<cc::ContextProvider>
 GpuProcessTransportFactory::OffscreenContextProviderForCompositorThread() {
-  if (!shared_contexts_compositor_thread_.get() ||
-      shared_contexts_compositor_thread_->DestroyedOnMainThread()) {
+  // The lifetime of this context is tied to the main thread context so that
+  // they are always in the same share group. So do not check for
+  // DestroyedOnMainThread().
+  if (!shared_contexts_compositor_thread_.get()) {
     shared_contexts_compositor_thread_ = ContextProviderCommandBuffer::Create(
         GpuProcessTransportFactory::CreateOffscreenCommandBufferContext());
   }
@@ -493,18 +498,29 @@ void GpuProcessTransportFactory::OnLostMainThreadSharedContextInsideCallback() {
 
 void GpuProcessTransportFactory::OnLostMainThreadSharedContext() {
   LOG(ERROR) << "Lost UI shared context.";
+
   // Keep old resources around while we call the observers, but ensure that
   // new resources are created if needed.
+  // Kill shared contexts for both threads in tandem so they are always in
+  // the same share group.
 
-  scoped_refptr<ContextProviderCommandBuffer> old_contexts_main_thread =
+  scoped_refptr<cc::ContextProvider> lost_shared_contexts_main_thread =
       shared_contexts_main_thread_;
+  scoped_refptr<cc::ContextProvider> lost_shared_contexts_compositor_thread =
+      shared_contexts_compositor_thread_;
   shared_contexts_main_thread_ = NULL;
+  shared_contexts_compositor_thread_ = NULL;
 
-  scoped_ptr<GLHelper> old_helper(gl_helper_.release());
+  scoped_ptr<GLHelper> lost_gl_helper = gl_helper_.Pass();
 
   FOR_EACH_OBSERVER(ImageTransportFactoryObserver,
                     observer_list_,
                     OnLostResources());
+
+  // Kill things that use the shared context before killing the shared context.
+  lost_gl_helper.reset();
+  lost_shared_contexts_main_thread = NULL;
+  lost_shared_contexts_compositor_thread = NULL;
 }
 
 }  // namespace content