Aura: Don't create GL context for CreateSharedSurfaceHandle()

content/browser/renderer_host/gpu_message_filter.cc

Index: content/browser/renderer_host/gpu_message_filter.cc
diff --git a/content/browser/renderer_host/gpu_message_filter.cc b/content/browser/renderer_host/gpu_message_filter.cc
index 9068a4072ecf5f2b982360cfe2b1f007bac0e7b2..5de0e4f6742c8143a6cbe610c6d4a8cc75f3e4c9 100644
--- a/content/browser/renderer_host/gpu_message_filter.cc
+++ b/content/browser/renderer_host/gpu_message_filter.cc
@@ -94,21 +94,17 @@ bool GpuMessageFilter::OnMessageReceived(
   return handled;
 }
 
-void GpuMessageFilter::SurfaceUpdated(int32 surface_id) {
+void GpuMessageFilter::OnParentChannelEstablished() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   typedef std::vector<linked_ptr<CreateViewCommandBufferRequest> > RequestList;
   RequestList retry_requests;
   retry_requests.swap(pending_requests_);
   for (RequestList::iterator it = retry_requests.begin();
-      it != retry_requests.end(); ++it) {
-    if ((*it)->surface_id != surface_id) {

[no sievers, 2013/12/10 21:17:20]

Would it be ok to remove this since we notify all observers at the same time
anyway?

[piman, 2013/12/11 01:14:03]

So, as the code was before, the GLSurfaceHandle was updated for each surface id
separately, each operation under a lock, on the UI thread. Similarly,
OnCreateViewCommandBuffer grabs it separately from the surface id, under the
lock, on the IO thread.
Because we don't atomically update all the surfaces, this was needed to make
sure we only look at surfaces that are known to be updated already.

In this version of the code, we don't update the GLSurfaceHandle for the
surface_id anymore. If we can keep this property, then we indeed don't need this
logic any more.

-      pending_requests_.push_back(*it);
-    } else {
-      linked_ptr<CreateViewCommandBufferRequest> request = *it;
-      OnCreateViewCommandBuffer(request->surface_id,
-                                request->init_params,
-                                request->reply.release());
-    }
+       it != retry_requests.end();
+       ++it) {
+    linked_ptr<CreateViewCommandBufferRequest> request = *it;
+    OnCreateViewCommandBuffer(
+        request->surface_id, request->init_params, request->reply.release());
   }
 }
 
@@ -187,23 +183,27 @@ void GpuMessageFilter::OnCreateViewCommandBuffer(
       surface_id, &renderer_id, &render_widget_id);
   if (result && renderer_id == render_process_id_) {
     compositing_surface = surface_tracker->GetSurfaceHandle(surface_id);
+    if (compositing_surface.parent_client_id) {
+      DCHECK(gpu_process_id_);
+      DCHECK(compositing_surface.is_transport());
+      DCHECK(BrowserGpuChannelHostFactory::instance());
+      // Note: The browser channel id might be 0 (not established yet), or the
+      // GPU process might have crashed. In either case we have to wait so that
+      // child and parent end up using the same GPU process.
+      if (BrowserGpuChannelHostFactory::instance()->GpuProcessHostId() !=

[piman, 2013/12/11 01:14:03]

This doesn't guarantee the renderer uses the same channel as the UI is currently
using: after a gpu crash, the renderer may:
1- EstablishGpuChannel, which will bump
BrowserGpuChannelHostFactory::instance()->GpuProcessHostId(), and
GpuMessageFilter will use that as gpu_process_id_
2- CreateViewCommandBuffer, at which point this test will be false

all that before the UI recreated its channel.

So it really means the code to wait on the UI to recreate the channel doesn't do
anything, really.

[no sievers, 2013/12/11 01:49:24]

Doh, right. So it'd have to get it from the image factory or so.

So the main reason for blocking the renderer was for RWHV to delete the
transport texture before new frames are swapped? And this might not be terribly
important for TextureImageTransportSurface with mailboxes and ubercomp anymore?
Or if don't currently, we should maybe handle it better?

And another reason you mentioned is throttling for when the renderer becomes
accelerated but the browser is still in software. The null GLSurfaceHandle()
then causes it in GpuMessageFilter to not delay the task, but return in ' if
(!host || compositing_surface.is_null()) {'. Is that correct?

Are there any other causes I'd have to worry about wrt comparing the gpu process
id for renderer and browser?

+          gpu_process_id_) {
+        linked_ptr<CreateViewCommandBufferRequest> request(
+            new CreateViewCommandBufferRequest(
+                surface_id, init_params, reply.Pass()));
+        pending_requests_.push_back(request);
+        return;
+      }
+    }
   } else {
     DLOG(ERROR) << "Renderer " << render_process_id_
                 << " tried to access a surface for renderer " << renderer_id;
   }
 
-  if (compositing_surface.parent_gpu_process_id &&
-      compositing_surface.parent_gpu_process_id != gpu_process_id_) {
-    // If the current handle for the surface is using a different (older) gpu
-    // host, it means the GPU process died and we need to wait until the UI
-    // re-allocates the surface in the new process.
-    linked_ptr<CreateViewCommandBufferRequest> request(
-        new CreateViewCommandBufferRequest(
-            surface_id, init_params, reply.Pass()));
-    pending_requests_.push_back(request);
-    return;
-  }
-
   GpuProcessHost* host = GpuProcessHost::FromID(gpu_process_id_);
   if (!host || compositing_surface.is_null()) {
     // TODO(apatrick): Eventually, this IPC message will be routed to a