ui::ContextFactory should not create SurfaceIdAllocators

content/browser/compositor/gpu_process_transport_factory.cc

 // Copyright 2014 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 "content/browser/compositor/gpu_process_transport_factory.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 669 matching lines @@
 }
 
 cc::TaskGraphRunner* GpuProcessTransportFactory::GetTaskGraphRunner() {
   return task_graph_runner_.get();
 }
 
 ui::ContextFactory* GpuProcessTransportFactory::GetContextFactory() {
   return this;
 }
 
-std::unique_ptr<cc::SurfaceIdAllocator>
-GpuProcessTransportFactory::CreateSurfaceIdAllocator() {
-  std::unique_ptr<cc::SurfaceIdAllocator> allocator =
-      base::WrapUnique(new cc::SurfaceIdAllocator(next_surface_client_id_++));
+uint32_t GpuProcessTransportFactory::AllocateSurfaceClientId() {
+  uint32_t client_id = next_surface_client_id_++;
   if (GetSurfaceManager())
-    allocator->RegisterSurfaceClientId(GetSurfaceManager());
-  return allocator;
+    GetSurfaceManager()->RegisterSurfaceClientId(client_id);

[piman, 2016/07/21 17:19:47]

So, the previous code also relied on SurfaceIdAllocator::RegisterSurfaceClientId
setting up the manager_ field on the the SurfaceIdAllocator, so that in
~SurfaceIdAllocator, SurfaceManager::InvalidateSurfaceClientId is called.

The latter doesn't happen any more. Is it not needed?

[Fady Samuel, 2016/07/21 17:22:23]

Well, it is still needed, but SurfaceIdAllocator will take care of that on
destruction so the code shouldn't be any different here. In the end state, this
won't be needed though because SurfaceFactory will take in a client ID and do
all the necessary set up and tear down work.

[piman, 2016/07/21 17:28:40]

With this patch, the SurfaceIdAllocator doesn't know about the SurfaceManager
any more, so skips InvalidateSurfaceClientId in the destructor (manager_ would
be null - unless I missed something?)
Got it.

[Fady Samuel, 2016/07/21 20:43:33]

I've made all the clients that create SurfaceIdAllocators Register and
invalidate the client IDs themselves. This takes us in the right direction for
bundling this into SurfaceFactory in a subsequent CL.

+  return client_id;
 }
 
 void GpuProcessTransportFactory::ResizeDisplay(ui::Compositor* compositor,
                                                const gfx::Size& size) {
   PerCompositorDataMap::iterator it = per_compositor_data_.find(compositor);
   if (it == per_compositor_data_.end())
     return;
   PerCompositorData* data = it->second;
   DCHECK(data);
   if (data->display)
@@ skipping 184 matching lines @@
       shared_vulkan_context_provider_ =
           cc::VulkanInProcessContextProvider::Create();
     }
 
     shared_vulkan_context_provider_initialized_ = true;
   }
   return shared_vulkan_context_provider_;
 }
 
 }  // namespace content