Defend against RPHI::ForceReleaseWorkerRefCount called twice.

content/browser/renderer_host/render_process_host_impl.cc

Index: content/browser/renderer_host/render_process_host_impl.cc
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index a9db38c4e80d175983907baad30eec1fc6eea026..5f59494f19bd78e81b17ed23e6842458015905ad 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -1399,7 +1399,12 @@ void RenderProcessHostImpl::DecrementSharedWorkerRefCount() {
 
 void RenderProcessHostImpl::ForceReleaseWorkerRefCounts() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK(!is_worker_ref_count_disabled_);
+  // ForceReleaseWorkerRefCounts can possibly be called twice if a new
+  // BrowserContext is created at the same address as an one that was just
+  // destroyed, and the new one is also soon destroyed.
+  // https://crbug.com/661843.

[horo, 2016/11/28 07:26:51]

I think we should introduce RenderProcessHostImpl::OnContextWillBeDestroyed()
which sets browser_context_ to null and calls ForceReleaseWorkerRefCounts().

[falken, 2016/11/28 07:34:21]

Hm, I considered this (see CL description) but Cleanup() uses browser_context_
and also sometimes doesn't to run synchronously ("delayed_cleanup_needed"). So
I'm not sure it's safe to clear browser_context_ even after calling Cleanup().

[horo, 2016/11/28 08:54:40]

After BrowserContext::NotifyWillBeDestroyed() is called, we should not refer the
browser_context.
So we should set browser_context_ to null in the end of
RenderProcessHostImpl::OnContextWillBeDestroyed().
 RenderProcessHostImpl::OnContextWillBeDestroyed() {
   ForceReleaseWorkerRefCounts();
   browser_context_ = nullptr;
 }

The browser_context may be alive even after
BrowserContext::NotifyWillBeDestroyed().
But this is a bug that MUST BE FIXED.
https://chromium.googlesource.com/chromium/src/+/b15e190/chrome/browser/profi...

[falken, 2016/11/29 04:19:36]

Hm yea I'm no longer sure of either solution. //content assumes that
BrowserContext outlives its RPHI. So any tricks to prevent a stale
browser_context_ pointer are just working around the fact that somehow this
expectation is violated, and a DCHECK like ProfileDestroyer above would be
better to make the violation explicitly known.

+  if (is_worker_ref_count_disabled_)
+    return;
   is_worker_ref_count_disabled_ = true;
   if (!worker_ref_count())
     return;