mus: Get GpuMemoryBuffer from gpu process when possible.

services/ui/ws/mus_gpu_memory_buffer_manager.h

Index: services/ui/ws/mus_gpu_memory_buffer_manager.h
diff --git a/services/ui/ws/mus_gpu_memory_buffer_manager.h b/services/ui/ws/mus_gpu_memory_buffer_manager.h
index a40a7438d1335deb2e84ae31e4629dc0f4df1b12..d098bb1b1fd6e27bab3cca48ba12111fd73f9295 100644
--- a/services/ui/ws/mus_gpu_memory_buffer_manager.h
+++ b/services/ui/ws/mus_gpu_memory_buffer_manager.h
@@ -27,6 +27,18 @@ class MusGpuMemoryBufferManager : public gpu::GpuMemoryBufferManager {
                             int client_id);
   ~MusGpuMemoryBufferManager() override;
 
+  void DestroyGpuMemoryBuffer(gfx::GpuMemoryBufferId id,
+                              int client_id,
+                              const gpu::SyncToken& sync_token);
+
+  gfx::GpuMemoryBufferHandle AllocateGpuMemoryBufferHandle(

[reveman, 2016/11/22 08:18:56]

Hm, I'm not a fan of a Handle suffix. It's allocating a buffer and not a handle.
The result is just returned in the form of a handle.

I think the problem is that the AllocateGpuMemoryBuffer function below should be
named CreateGpuMemoryBuffer as it returns an instance. We might want a quick
rename before we land this.

+      gfx::GpuMemoryBufferId id,
+      int client_id,
+      const gfx::Size& size,
+      gfx::BufferFormat format,
+      gfx::BufferUsage usage,
+      gpu::SurfaceHandle surface_handle);
+
   // Overridden from gpu::GpuMemoryBufferManager:
   std::unique_ptr<gfx::GpuMemoryBuffer> AllocateGpuMemoryBuffer(
       const gfx::Size& size,
@@ -43,13 +55,14 @@ class MusGpuMemoryBufferManager : public gpu::GpuMemoryBufferManager {
  private:
   DISALLOW_COPY_AND_ASSIGN(MusGpuMemoryBufferManager);
 
-  void DestroyGpuMemoryBuffer(gfx::GpuMemoryBufferId id,
-                              int client_id,
-                              bool is_native,
-                              const gpu::SyncToken& sync_token);
-
   mojom::GpuServiceInternal* gpu_service_;
   const int client_id_;
+
+  using NativeBuffers =
+      std::unordered_set<gfx::GpuMemoryBufferId,
+                         BASE_HASH_NAMESPACE::hash<gfx::GpuMemoryBufferId>>;
+  std::unordered_map<int, NativeBuffers> native_buffers_;

[reveman, 2016/11/22 08:18:56]

what is this used for? It seems to keep track of what are native buffers but I'm
not seeing it actually being used for anything.

[sadrul, 2016/11/22 18:45:41]

This is somewhat equivalent to BrowserGpuMemoryManager::clients_ map [1]. This
is used to decide when to notify mus-gpu about GMB destruction.

[1]
https://cs.chromium.org/chromium/src/content/browser/gpu/browser_gpu_memory_b...

[reveman, 2016/11/22 23:25:04]

That's only needed for for Mac because IOSurface sharing is currently broken
there (it doesn't survide GPU process crashes like other platforms). I created a
bug some time ago for fixing it (crbug.com/649721) and that would remove the
need for this map. Let's not add it for MUS as it's broken be design. The
destroy notification for GMBs shouldn't exist. We should fix the Mac issue asap
as that will also guarantee that GMBs don't cause drawing artifacts when the GPU
process crashes. You only need this for MUS if you're planning to support Mac
with native IOSurface backed GMBs before we fixed the above mentioned issue.

[sadrul, 2016/11/24 02:02:41]

Ok. If I understand correctly, you are suggesting that I don't actually need to
do this, and I should remove it from the CL?

[sadrul, 2016/11/24 03:34:04]

It looks like ozone may be doing something as well? [1] Or can that be removed
too?

[1]
https://cs.chromium.org/chromium/src/gpu/ipc/service/gpu_memory_buffer_factor...

[reveman, 2016/11/30 17:32:45]

Yes, I forgot that we unfortunately have a workaround in place that requires
this (crbug.com/628334). This should not be needed in the future but I guess we
have to deal with it even in the mus case for now.

+
   base::WeakPtrFactory<MusGpuMemoryBufferManager> weak_factory_;
 };