mus: Get GpuMemoryBuffer from gpu process when possible.

services/ui/ws/mus_gpu_memory_buffer_manager.h

 // Copyright 2016 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.
 
 #ifndef SERVICES_UI_WS_MUS_GPU_MEMORY_BUFFER_MANAGER_H_
 #define SERVICES_UI_WS_MUS_GPU_MEMORY_BUFFER_MANAGER_H_
 
 #include <memory>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "gpu/command_buffer/client/gpu_memory_buffer_manager.h"
 
 namespace ui {
 
 namespace mojom {
 class GpuServiceInternal;
 }
 
 namespace ws {
 
 // This GpuMemoryBufferManager is for establishing a GpuChannelHost used by
 // mus locally.
 class MusGpuMemoryBufferManager : public gpu::GpuMemoryBufferManager {
  public:
   MusGpuMemoryBufferManager(mojom::GpuServiceInternal* gpu_service,
                             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,
       gfx::BufferFormat format,
       gfx::BufferUsage usage,
       gpu::SurfaceHandle surface_handle) override;
   std::unique_ptr<gfx::GpuMemoryBuffer> CreateGpuMemoryBufferFromHandle(
       const gfx::GpuMemoryBufferHandle& handle,
       const gfx::Size& size,
       gfx::BufferFormat format) override;
   void SetDestructionSyncToken(gfx::GpuMemoryBuffer* buffer,
                                const gpu::SyncToken& sync_token) override;
 
  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_;
 };
 
 }  // namespace ws
 }  // namespace ui
 
 #endif  // SERVICES_UI_WS_MUS_GPU_MEMORY_BUFFER_MANAGER_H_