Add HostSharedBitmapManagerClient to organize bitmaps coming from renderers.

PeerHandle() is not necessarily unique, so have a HostSharedBitmapManagerClient per renderer channel that's responsible for deleting bitmaps from renderers that die.

Review URL: https://codereview.chromium.org/955523005

Cr-Commit-Position: refs/heads/master@{#319001}

content/browser/renderer_host/render_message_filter.cc

@@ -306,11 +306,12 @@ RenderMessageFilter::RenderMessageFilter(
     media::AudioManager* audio_manager,
     MediaInternals* media_internals,
     DOMStorageContextWrapper* dom_storage_context)
-    : BrowserMessageFilter(
-          kFilteredMessageClasses, arraysize(kFilteredMessageClasses)),
+    : BrowserMessageFilter(kFilteredMessageClasses,
+                           arraysize(kFilteredMessageClasses)),
       resource_dispatcher_host_(ResourceDispatcherHostImpl::Get()),
       plugin_service_(plugin_service),
       profile_data_directory_(browser_context->GetPath()),
+      bitmap_manager_client_(HostSharedBitmapManager::current()),
       request_context_(request_context),
       resource_context_(browser_context->GetResourceContext()),
       render_widget_helper_(render_widget_helper),
@@ -329,7 +330,6 @@ RenderMessageFilter::~RenderMessageFilter() {
   // This function should be called on the IO thread.
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(plugin_host_clients_.empty());
-  HostSharedBitmapManager::current()->ProcessRemoved(PeerHandle());
   BrowserGpuMemoryBufferManager* gpu_memory_buffer_manager =
       BrowserGpuMemoryBufferManager::current();
   if (gpu_memory_buffer_manager)
@@ -920,8 +920,8 @@ void RenderMessageFilter::AllocateSharedBitmapOnFileThread(
     const cc::SharedBitmapId& id,
     IPC::Message* reply_msg) {
   base::SharedMemoryHandle handle;
-  HostSharedBitmapManager::current()->AllocateSharedBitmapForChild(
-      PeerHandle(), buffer_size, id, &handle);
+  bitmap_manager_client_.AllocateSharedBitmapForChild(PeerHandle(), buffer_size,
+                                                      id, &handle);
   ChildProcessHostMsg_SyncAllocateSharedBitmap::WriteReplyParams(reply_msg,
                                                                  handle);
   Send(reply_msg);
@@ -944,12 +944,12 @@ void RenderMessageFilter::OnAllocatedSharedBitmap(
     size_t buffer_size,
     const base::SharedMemoryHandle& handle,
     const cc::SharedBitmapId& id) {
-  HostSharedBitmapManager::current()->ChildAllocatedSharedBitmap(
-      buffer_size, handle, PeerHandle(), id);
+  bitmap_manager_client_.ChildAllocatedSharedBitmap(buffer_size, handle,
+                                                    PeerHandle(), id);
 }
 
 void RenderMessageFilter::OnDeletedSharedBitmap(const cc::SharedBitmapId& id) {
-  HostSharedBitmapManager::current()->ChildDeletedSharedBitmap(id);
+  bitmap_manager_client_.ChildDeletedSharedBitmap(id);
 }
 
 void RenderMessageFilter::OnAllocateLockedDiscardableSharedMemory(

content/browser/renderer_host/render_message_filter.h

@@ -20,6 +20,7 @@
 #include "base/strings/string16.h"
 #include "build/build_config.h"
 #include "cc/resources/shared_bitmap_manager.h"
+#include "content/common/host_shared_bitmap_manager.h"
 #include "content/public/browser/browser_message_filter.h"
 #include "content/public/common/three_d_api_types.h"
 #include "ipc/message_filter.h"
@@ -310,6 +311,8 @@ class CONTENT_EXPORT RenderMessageFilter : public BrowserMessageFilter {
   PluginServiceImpl* plugin_service_;
   base::FilePath profile_data_directory_;
 
+  HostSharedBitmapManagerClient bitmap_manager_client_;
+
   // Contextual information to be used for requests created here.
   scoped_refptr<net::URLRequestContextGetter> request_context_;
 

content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

@@ -1867,11 +1867,12 @@ TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFrames) {
   int handles_per_frame = 5;
   RendererFrameManager::GetInstance()->set_max_handles(handles_per_frame * 2);
 
+  HostSharedBitmapManagerClient bitmap_client(
+      HostSharedBitmapManager::current());
+
   for (size_t i = 0; i < (renderer_count - 1) * handles_per_frame; i++) {
-    HostSharedBitmapManager::current()->ChildAllocatedSharedBitmap(
-        1,
-        base::SharedMemory::NULLHandle(),
-        base::GetCurrentProcessHandle(),
+    bitmap_client.ChildAllocatedSharedBitmap(
+        1, base::SharedMemory::NULLHandle(), base::GetCurrentProcessHandle(),
         cc::SharedBitmap::GenerateId());
   }
 
@@ -1883,8 +1884,6 @@ TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFrames) {
     else
       EXPECT_TRUE(views[i]->HasFrameData());
   }
-  HostSharedBitmapManager::current()->ProcessRemoved(
-      base::GetCurrentProcessHandle());
   RendererFrameManager::GetInstance()->set_max_handles(
       base::SharedMemory::GetHandleLimit());
 

content/common/host_shared_bitmap_manager.cc

@@ -55,6 +55,41 @@ class HostSharedBitmap : public cc::SharedBitmap {
 base::LazyInstance<HostSharedBitmapManager> g_shared_memory_manager =
     LAZY_INSTANCE_INITIALIZER;
 
+HostSharedBitmapManagerClient::HostSharedBitmapManagerClient(
+    HostSharedBitmapManager* manager)
+    : manager_(manager) {
+}
+
+HostSharedBitmapManagerClient::~HostSharedBitmapManagerClient() {
+  for (const auto& id : owned_bitmaps_)
+    manager_->ChildDeletedSharedBitmap(id);
+}
+
+void HostSharedBitmapManagerClient::AllocateSharedBitmapForChild(
+    base::ProcessHandle process_handle,
+    size_t buffer_size,
+    const cc::SharedBitmapId& id,
+    base::SharedMemoryHandle* shared_memory_handle) {
+  manager_->AllocateSharedBitmapForChild(process_handle, buffer_size, id,
+                                         shared_memory_handle);
+  owned_bitmaps_.insert(id);
+}
+
+void HostSharedBitmapManagerClient::ChildAllocatedSharedBitmap(
+    size_t buffer_size,
+    const base::SharedMemoryHandle& handle,
+    base::ProcessHandle process_handle,
+    const cc::SharedBitmapId& id) {
+  manager_->ChildAllocatedSharedBitmap(buffer_size, handle, process_handle, id);
+  owned_bitmaps_.insert(id);
+}
+
+void HostSharedBitmapManagerClient::ChildDeletedSharedBitmap(
+    const cc::SharedBitmapId& id) {
+  manager_->ChildDeletedSharedBitmap(id);
+  owned_bitmaps_.erase(id);
+}
+
 HostSharedBitmapManager::HostSharedBitmapManager() {}
 HostSharedBitmapManager::~HostSharedBitmapManager() {
   DCHECK(handle_map_.empty());
@@ -123,7 +158,6 @@ void HostSharedBitmapManager::ChildAllocatedSharedBitmap(
       new BitmapData(process_handle, buffer_size));
 
   handle_map_[id] = data;
-  process_map_[process_handle].insert(id);
 #if defined(OS_WIN)
   data->memory = make_scoped_ptr(
       new base::SharedMemory(handle, false, data->process_handle));
@@ -157,7 +191,6 @@ void HostSharedBitmapManager::AllocateSharedBitmapForChild(
   data->memory = shared_memory.Pass();
 
   handle_map_[id] = data;
-  process_map_[process_handle].insert(id);
   if (!data->memory->ShareToProcess(process_handle, shared_memory_handle)) {
     LOG(ERROR) << "Cannot share shared memory buffer";
     *shared_memory_handle = base::SharedMemory::NULLHandle();
@@ -169,29 +202,7 @@ void HostSharedBitmapManager::AllocateSharedBitmapForChild(
 void HostSharedBitmapManager::ChildDeletedSharedBitmap(
     const cc::SharedBitmapId& id) {
   base::AutoLock lock(lock_);
-  BitmapMap::iterator it = handle_map_.find(id);
-  if (it == handle_map_.end())
-    return;
-  base::hash_set<cc::SharedBitmapId>& res =
-      process_map_[it->second->process_handle];
-  res.erase(id);
-  handle_map_.erase(it);
-}
-
-void HostSharedBitmapManager::ProcessRemoved(
-    base::ProcessHandle process_handle) {
-  base::AutoLock lock(lock_);
-  ProcessMap::iterator proc_it = process_map_.find(process_handle);
-  if (proc_it == process_map_.end())
-    return;
-  base::hash_set<cc::SharedBitmapId>& res = proc_it->second;
-
-  for (base::hash_set<cc::SharedBitmapId>::iterator it = res.begin();
-       it != res.end();
-       ++it) {
-    handle_map_.erase(*it);
-  }
-  process_map_.erase(proc_it);
+  handle_map_.erase(id);
 }
 
 size_t HostSharedBitmapManager::AllocatedBitmapCount() const {

content/common/host_shared_bitmap_manager.h

@@ -29,6 +29,31 @@ struct hash<cc::SharedBitmapId> {
 
 namespace content {
 class BitmapData;
+class HostSharedBitmapManager;
+
+class CONTENT_EXPORT HostSharedBitmapManagerClient {
+ public:
+  explicit HostSharedBitmapManagerClient(HostSharedBitmapManager* manager);
+
+  ~HostSharedBitmapManagerClient();
+
+  void AllocateSharedBitmapForChild(
+      base::ProcessHandle process_handle,
+      size_t buffer_size,
+      const cc::SharedBitmapId& id,
+      base::SharedMemoryHandle* shared_memory_handle);
+  void ChildAllocatedSharedBitmap(size_t buffer_size,
+                                  const base::SharedMemoryHandle& handle,
+                                  base::ProcessHandle process_handle,
+                                  const cc::SharedBitmapId& id);
+  void ChildDeletedSharedBitmap(const cc::SharedBitmapId& id);
+
+ private:
+  HostSharedBitmapManager* manager_;
+  base::hash_set<cc::SharedBitmapId> owned_bitmaps_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostSharedBitmapManagerClient);
+};
 
 class CONTENT_EXPORT HostSharedBitmapManager : public cc::SharedBitmapManager {
  public:
@@ -44,6 +69,13 @@ class CONTENT_EXPORT HostSharedBitmapManager : public cc::SharedBitmapManager {
       const gfx::Size& size,
       const cc::SharedBitmapId&) override;
 
+  size_t AllocatedBitmapCount() const;
+
+  void FreeSharedMemoryFromMap(const cc::SharedBitmapId& id);
+
+ private:
+  friend class HostSharedBitmapManagerClient;
+
   void AllocateSharedBitmapForChild(
       base::ProcessHandle process_handle,
       size_t buffer_size,
@@ -54,22 +86,14 @@ class CONTENT_EXPORT HostSharedBitmapManager : public cc::SharedBitmapManager {
                                   base::ProcessHandle process_handle,
                                   const cc::SharedBitmapId& id);
   void ChildDeletedSharedBitmap(const cc::SharedBitmapId& id);
-  void ProcessRemoved(base::ProcessHandle process_handle);
-
-  size_t AllocatedBitmapCount() const;
 
-  void FreeSharedMemoryFromMap(const cc::SharedBitmapId& id);
-
- private:
   mutable base::Lock lock_;
 
   typedef base::hash_map<cc::SharedBitmapId, scoped_refptr<BitmapData> >
       BitmapMap;
   BitmapMap handle_map_;
 
-  typedef base::hash_map<base::ProcessHandle,
-                         base::hash_set<cc::SharedBitmapId> > ProcessMap;
-  ProcessMap process_map_;
+  DISALLOW_COPY_AND_ASSIGN(HostSharedBitmapManager);
 };
 
 }  // namespace content

content/common/host_shared_bitmap_manager_unittest.cc

@@ -23,10 +23,11 @@ TEST_F(HostSharedBitmapManagerTest, TestCreate) {
   memset(bitmap->memory(), 0xff, size_in_bytes);
   cc::SharedBitmapId id = cc::SharedBitmap::GenerateId();
 
+  HostSharedBitmapManagerClient client(manager_.get());
   base::SharedMemoryHandle handle;
   bitmap->ShareToProcess(base::GetCurrentProcessHandle(), &handle);
-  manager_->ChildAllocatedSharedBitmap(
-      size_in_bytes, handle, base::GetCurrentProcessHandle(), id);
+  client.ChildAllocatedSharedBitmap(size_in_bytes, handle,
+                                    base::GetCurrentProcessHandle(), id);
 
   scoped_ptr<cc::SharedBitmap> large_bitmap;
   large_bitmap = manager_->GetSharedBitmapFromId(gfx::Size(1024, 1024), id);
@@ -63,7 +64,7 @@ TEST_F(HostSharedBitmapManagerTest, TestCreate) {
   EXPECT_EQ(memcmp(shared_bitmap->pixels(), bitmap->memory(), size_in_bytes),
             0);
 
-  manager_->ChildDeletedSharedBitmap(id);
+  client.ChildDeletedSharedBitmap(id);
 
   memset(bitmap->memory(), 0, size_in_bytes);
 
@@ -78,9 +79,10 @@ TEST_F(HostSharedBitmapManagerTest, TestCreateForChild) {
   size_t size_in_bytes;
   EXPECT_TRUE(cc::SharedBitmap::SizeInBytes(bitmap_size, &size_in_bytes));
   cc::SharedBitmapId id = cc::SharedBitmap::GenerateId();
+  HostSharedBitmapManagerClient client(manager_.get());
   base::SharedMemoryHandle handle;
-  manager_->AllocateSharedBitmapForChild(
-      base::GetCurrentProcessHandle(), size_in_bytes, id, &handle);
+  client.AllocateSharedBitmapForChild(base::GetCurrentProcessHandle(),
+                                      size_in_bytes, id, &handle);
 
   EXPECT_TRUE(base::SharedMemory::IsHandleValid(handle));
   scoped_ptr<base::SharedMemory> bitmap(new base::SharedMemory(handle, false));
@@ -93,7 +95,7 @@ TEST_F(HostSharedBitmapManagerTest, TestCreateForChild) {
   EXPECT_TRUE(
       memcmp(bitmap->memory(), shared_bitmap->pixels(), size_in_bytes) == 0);
 
-  manager_->ChildDeletedSharedBitmap(id);
+  client.ChildDeletedSharedBitmap(id);
 }
 
 TEST_F(HostSharedBitmapManagerTest, RemoveProcess) {
@@ -106,17 +108,19 @@ TEST_F(HostSharedBitmapManagerTest, RemoveProcess) {
   cc::SharedBitmapId id = cc::SharedBitmap::GenerateId();
 
   base::SharedMemoryHandle handle;
+  scoped_ptr<HostSharedBitmapManagerClient> client(
+      new HostSharedBitmapManagerClient(manager_.get()));
   bitmap->ShareToProcess(base::GetCurrentProcessHandle(), &handle);
-  manager_->ChildAllocatedSharedBitmap(
-      size_in_bytes, handle, base::GetCurrentProcessHandle(), id);
-
-  manager_->ProcessRemoved(base::kNullProcessHandle);
+  client->ChildAllocatedSharedBitmap(size_in_bytes, handle,
+                                     base::GetCurrentProcessHandle(), id);
 
   scoped_ptr<cc::SharedBitmap> shared_bitmap;
   shared_bitmap = manager_->GetSharedBitmapFromId(bitmap_size, id);
   ASSERT_TRUE(shared_bitmap.get() != NULL);
 
-  manager_->ProcessRemoved(base::GetCurrentProcessHandle());
+  EXPECT_EQ(1u, manager_->AllocatedBitmapCount());
+  client.reset();
+  EXPECT_EQ(0u, manager_->AllocatedBitmapCount());
 
   scoped_ptr<cc::SharedBitmap> shared_bitmap2;
   shared_bitmap2 = manager_->GetSharedBitmapFromId(bitmap_size, id);
@@ -125,9 +129,6 @@ TEST_F(HostSharedBitmapManagerTest, RemoveProcess) {
             0);
 
   shared_bitmap.reset();
-
-  // Should no-op.
-  manager_->ChildDeletedSharedBitmap(id);
 }
 
 TEST_F(HostSharedBitmapManagerTest, AddDuplicate) {
@@ -138,25 +139,26 @@ TEST_F(HostSharedBitmapManagerTest, AddDuplicate) {
   bitmap->CreateAndMapAnonymous(size_in_bytes);
   memset(bitmap->memory(), 0xff, size_in_bytes);
   cc::SharedBitmapId id = cc::SharedBitmap::GenerateId();
+  HostSharedBitmapManagerClient client(manager_.get());
 
   base::SharedMemoryHandle handle;
   bitmap->ShareToProcess(base::GetCurrentProcessHandle(), &handle);
-  manager_->ChildAllocatedSharedBitmap(
-      size_in_bytes, handle, base::GetCurrentProcessHandle(), id);
+  client.ChildAllocatedSharedBitmap(size_in_bytes, handle,
+                                    base::GetCurrentProcessHandle(), id);
 
   scoped_ptr<base::SharedMemory> bitmap2(new base::SharedMemory());
   bitmap2->CreateAndMapAnonymous(size_in_bytes);
   memset(bitmap2->memory(), 0x00, size_in_bytes);
 
-  manager_->ChildAllocatedSharedBitmap(
-      size_in_bytes, bitmap2->handle(), base::GetCurrentProcessHandle(), id);
+  client.ChildAllocatedSharedBitmap(size_in_bytes, bitmap2->handle(),
+                                    base::GetCurrentProcessHandle(), id);
 
   scoped_ptr<cc::SharedBitmap> shared_bitmap;
   shared_bitmap = manager_->GetSharedBitmapFromId(bitmap_size, id);
   ASSERT_TRUE(shared_bitmap.get() != NULL);
   EXPECT_EQ(memcmp(shared_bitmap->pixels(), bitmap->memory(), size_in_bytes),
             0);
-  manager_->ChildDeletedSharedBitmap(id);
+  client.ChildDeletedSharedBitmap(id);
 }
 
 }  // namespace