Ensure at most one AppCacheBackend per RenderProcessHost.

AppCacheService maintains a map from RenderProcessHost id to the
corresponding AppCacheBackend. The id can be reused if the renderer
process is shutdown and a new renderer process is created using
the same RenderProcessHost. This may lead to a race where the old
backend is not yet unregistered and a new backend is trying to
register for the same process id.

This patch fixed the race by eagerly unregistering the previous
backend before the new backend is registered.

Bug: 850921

Change-Id: Ie3c7576e17697ad87968b84a1e438a6f3c55a13e
Reviewed-on: https://chromium-review.googlesource.com/1135545
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575238}
diff --git a/chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper_browsertest.cc b/chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper_browsertest.cc
index ba41057..52e5a64 100644
--- a/chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper_browsertest.cc
+++ b/chrome/browser/ui/bloated_renderer/bloated_renderer_tab_helper_browsertest.cc
@@ -17,9 +17,7 @@
 
 using BloatedRendererTabHelperBrowserTest = InProcessBrowserTest;
 
-// TODO(ulan): Enable the test after fixing crbug.com/850921.
-IN_PROC_BROWSER_TEST_F(BloatedRendererTabHelperBrowserTest,
-                       DISABLED_ReloadBloatedTab) {
+IN_PROC_BROWSER_TEST_F(BloatedRendererTabHelperBrowserTest, ReloadBloatedTab) {
   content::WindowedNotificationObserver load(
       content::NOTIFICATION_NAV_ENTRY_COMMITTED,
       content::NotificationService::AllSources());
@@ -38,7 +36,11 @@
       InfoBarService::FromWebContents(web_contents);
   EXPECT_EQ(0u, infobar_service->infobar_count());
 
+  auto* page_signal_receiver =
+      resource_coordinator::PageSignalReceiver::GetInstance();
   resource_coordinator::PageNavigationIdentity page_id;
+  page_id.navigation_id =
+      page_signal_receiver->GetNavigationIDForWebContents(web_contents);
   BloatedRendererTabHelper::FromWebContents(web_contents)
       ->OnRendererIsBloated(web_contents, page_id);
   reload.Wait();
diff --git a/content/browser/appcache/appcache_dispatcher_host.cc b/content/browser/appcache/appcache_dispatcher_host.cc
index 774dbf4..966ab0e 100644
--- a/content/browser/appcache/appcache_dispatcher_host.cc
+++ b/content/browser/appcache/appcache_dispatcher_host.cc
@@ -32,9 +32,17 @@
 void AppCacheDispatcherHost::Create(ChromeAppCacheService* appcache_service,
                                     int process_id,
                                     mojom::AppCacheBackendRequest request) {
+  // The process_id is the id of the RenderProcessHost, which can be reused for
+  // a new renderer process if the previous renderer process was shutdown.
+  // It can take some time after shutdown for the pipe error to propagate
+  // and unregister the previous backend. Since the AppCacheService assumes
+  // that there is one backend per process_id, we need to ensure that the
+  // previous backend is unregistered by eagerly unbinding the pipe.
+  appcache_service->Unbind(process_id);
+
   appcache_service->Bind(
       std::make_unique<AppCacheDispatcherHost>(appcache_service, process_id),
-      std::move(request));
+      std::move(request), process_id);
 }
 
 void AppCacheDispatcherHost::RegisterHost(int32_t host_id) {
diff --git a/content/browser/appcache/appcache_service_impl.h b/content/browser/appcache/appcache_service_impl.h
index ff3bcd5..bf6fabe 100644
--- a/content/browser/appcache/appcache_service_impl.h
+++ b/content/browser/appcache/appcache_service_impl.h
@@ -153,7 +153,7 @@
   // Each child process in chrome uses a distinct backend instance.
   // See chrome/browser/AppCacheDispatcherHost.
   void RegisterBackend(AppCacheBackendImpl* backend_impl);
-  void UnregisterBackend(AppCacheBackendImpl* backend_impl);
+  virtual void UnregisterBackend(AppCacheBackendImpl* backend_impl);
   AppCacheBackendImpl* GetBackend(int id) const {
     BackendMap::const_iterator it = backends_.find(id);
     return (it != backends_.end()) ? it->second : NULL;
diff --git a/content/browser/appcache/chrome_appcache_service.cc b/content/browser/appcache/chrome_appcache_service.cc
index 89ccb9a..5c95cb9 100644
--- a/content/browser/appcache/chrome_appcache_service.cc
+++ b/content/browser/appcache/chrome_appcache_service.cc
@@ -45,8 +45,26 @@
 
 void ChromeAppCacheService::Bind(
     std::unique_ptr<mojom::AppCacheBackend> backend,
-    mojom::AppCacheBackendRequest request) {
-  bindings_.AddBinding(std::move(backend), std::move(request));
+    mojom::AppCacheBackendRequest request,
+    int process_id) {
+  DCHECK(process_bindings_.find(process_id) == process_bindings_.end());
+  process_bindings_[process_id] =
+      bindings_.AddBinding(std::move(backend), std::move(request));
+}
+
+void ChromeAppCacheService::Unbind(int process_id) {
+  auto it = process_bindings_.find(process_id);
+  if (it != process_bindings_.end()) {
+    bindings_.RemoveBinding(it->second);
+    DCHECK(process_bindings_.find(process_id) == process_bindings_.end());
+  }
+}
+
+void ChromeAppCacheService::UnregisterBackend(
+    AppCacheBackendImpl* backend_impl) {
+  int process_id = backend_impl->process_id();
+  process_bindings_.erase(process_bindings_.find(process_id));
+  AppCacheServiceImpl::UnregisterBackend(backend_impl);
 }
 
 void ChromeAppCacheService::Shutdown() {
diff --git a/content/browser/appcache/chrome_appcache_service.h b/content/browser/appcache/chrome_appcache_service.h
index 7845936..6c2ca7e 100644
--- a/content/browser/appcache/chrome_appcache_service.h
+++ b/content/browser/appcache/chrome_appcache_service.h
@@ -9,6 +9,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner_helpers.h"
+#include "content/browser/appcache/appcache_backend_impl.h"
 #include "content/browser/appcache/appcache_policy.h"
 #include "content/browser/appcache/appcache_service_impl.h"
 #include "content/common/appcache.mojom.h"
@@ -56,7 +57,11 @@
       scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy);
 
   void Bind(std::unique_ptr<mojom::AppCacheBackend> backend,
-            mojom::AppCacheBackendRequest request);
+            mojom::AppCacheBackendRequest request,
+            int process_id);
+  // Unbinds the pipe corresponding the to the given process_id.
+  // It does nothing if no pipe was bound.
+  void Unbind(int process_id);
 
   void Shutdown();
 
@@ -66,6 +71,9 @@
   bool CanCreateAppCache(const GURL& manifest_url,
                          const GURL& first_party) override;
 
+  // AppCacheServiceImpl override
+  void UnregisterBackend(AppCacheBackendImpl* backend_impl) override;
+
  protected:
   ~ChromeAppCacheService() override;
 
@@ -81,6 +89,9 @@
   base::FilePath cache_path_;
   mojo::StrongBindingSet<mojom::AppCacheBackend> bindings_;
 
+  // A map from a process_id to a binding_id.
+  std::map<int, mojo::BindingId> process_bindings_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeAppCacheService);
 };