gfx: Fix sending native ozone pixmaps from InProcessCommandBuffer.

gpu/ipc/client/command_buffer_proxy_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/ipc/client/command_buffer_proxy_impl.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/callback.h"
@@ skipping 444 matching lines @@
 
   gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager =
       channel_->gpu_memory_buffer_manager();
   gfx::GpuMemoryBuffer* gpu_memory_buffer =
       reinterpret_cast<gfx::GpuMemoryBuffer*>(buffer);
   DCHECK(gpu_memory_buffer);
 
   // This handle is owned by the GPU process and must be passed to it or it
   // will leak. In otherwords, do not early out on error between here and the
   // sending of the CreateImage IPC below.
-  bool requires_sync_token = false;
   gfx::GpuMemoryBufferHandle handle =
-      channel_->ShareGpuMemoryBufferToGpuProcess(gpu_memory_buffer->GetHandle(),
-                                                 &requires_sync_token);
+      channel_->IsLost()

[piman, 2016/11/28 23:42:36]

Actually this is not necessary anymore, since now handle ownership is passed to
the IPC system on all platforms (including Windows and Mac). This check is racy
anyway (IsLost can change on the IO thread)... previously the code was
duplicating handles directly into the GPU process, and there could be ways in
which they were leaked there if we couldn't send the message. But now the handle
is always passed/brokered/freed whether the message is sent or not, so I think
we should just unconditionally clone. If the channel is lost, we're most likely
going to early out on l.451, the only few remaining cases (channel is lost on
the IO thread but we haven't yet executed the task that tells us so) are not
interesting to "optimize".

[sadrul, 2016/11/29 01:53:51]

Oh cool. Thanks! Removed the IsLost() check from here.

+          ? gfx::GpuMemoryBufferHandle()
+          : gfx::CloneHandleForIPC(gpu_memory_buffer->GetHandle());

[sadrul, 2016/11/27 03:59:06]

This may actually change behaviour. Right now, 'IsLost()' is checked only when
the GMBHandle's type is SHARED_MEMORY_HANDLE, and only the SharedMemoryHandle is
null'ed out. The rest of the fields are copied/cloned normally. With this
change, all of the fields are left invalid. I think this ought to be OK?

+  bool requires_sync_token = handle.type == gfx::IO_SURFACE_BUFFER;
 
   uint64_t image_fence_sync = 0;
   if (requires_sync_token) {
     image_fence_sync = GenerateFenceSyncRelease();
 
     // Make sure fence syncs were flushed before CreateImage() was called.
     DCHECK_EQ(image_fence_sync, flushed_fence_sync_release_ + 1);
   }
 
   DCHECK(gpu::IsGpuMemoryBufferFormatSupported(gpu_memory_buffer->GetFormat(),
@@ skipping 387 matching lines @@
     return;
   channel_->FlushPendingStream(stream_id_);
   channel_->Send(new GpuChannelMsg_DestroyCommandBuffer(route_id_));
   channel_->RemoveRoute(route_id_);
   channel_ = nullptr;
   if (gpu_control_client_)
     gpu_control_client_->OnGpuControlLostContext();
 }
 
 }  // namespace gpu