Fix process reuse for dedicated processes when over process limit.

content/public/browser/render_process_host.h

 // Copyright (c) 2012 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 CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_H_
 #define CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 334 matching lines @@
   // Acquires the |mojom::Renderer| interface to the render process. This is for
   // internal use only, and is only exposed here to support
   // MockRenderProcessHost usage in tests.
   virtual mojom::Renderer* GetRendererInterface() = 0;
 
   // Whether this process is locked out from ever being reused for sites other
   // than the ones it currently has.
   virtual void SetIsNeverSuitableForReuse() = 0;
   virtual bool MayReuseHost() = 0;
 
+  // Indicates whether this RenderProcessHost can be used to host a URL that
+  // requires a dedicated process.  This starts out as true for new processes
+  // and will become false after a call to UnsetCanBecomeDedicatedProcess(),
+  // such as after the process has been used to host web content.
+  virtual bool CanBecomeDedicatedProcess() = 0;

[alexmos, 2017/06/12 17:35:07]

These are public because of MockRenderProcessHost. :(  I struggled with finding
good names here.  What I really wanted this to mean was that this is true up
until the first LockToOrigin call (which might come either when the first
navigation commits, or when a SiteInstance with a site that's already set calls
LockToOrigin).  I.e., this intends to be true for
uninitialized/untainted/site-less SiteInstances so that IsSuitableHost allows
their process to be used as a dedicated process.  Without this, the very first
navigation in a new tab caused a process swap from the initial SiteInstance,
causing lots of test failures.

[Charlie Reis, 2017/06/12 23:08:17]

Sigh.  Some day we'll fix that.

[Charlie Reis, 2017/06/12 23:08:17]

Yeah, I wonder if there's a better concept to be tracking here.  Would something
like IsUnused work, with documentation that it gets cleared in any of the
following cases?
1) Commits a page.
2) Given to a SiteInstance that already has a site assigned.
3) Hosts a ServiceWorker or SharedWorker.

That makes it potentially more generally useful, without being specific to the
dedicated process case.

[alexmos, 2017/06/14 22:39:05]

I like this, especially given the potential for this to be used elsewhere. 
Changed.

(Note that I didn't add (3) to the comment - see my explanation in
https://codereview.chromium.org/2921063003/diff/80001/content/browser/rendere....
 Though I guess I could say that ServiceWorkers are implicitly handled via (2),
and SharedWorkers are implicitly handled via (1).)

[Charlie Reis, 2017/06/17 23:13:53]

Yes, saying it's implicit in those cases might be worthwhile, just to show that
those cases are covered.

[alexmos, 2017/06/19 20:03:58]

Done.

+  virtual void UnsetCanBecomeDedicatedProcess() = 0;
+
   // Returns the current number of active views in this process.  Excludes
   // any RenderViewHosts that are swapped out.
   size_t GetActiveViewCount();
 
   // Posts |task|, if this RenderProcessHost is ready or when it becomes ready
   // (see RenderProcessHost::IsReady method).  The |task| might not run at all
   // (e.g. if |render_process_host| is destroyed before becoming ready).  This
   // function can only be called on the browser's UI thread (and the |task| will
   // be posted back on the UI thread).
   void PostTaskWhenProcessIsReady(base::OnceClosure task);
@@ skipping 49 matching lines @@
   static void SetMaxRendererProcessCount(size_t count);
 
   // Returns the current maximum number of renderer process hosts kept by the
   // content module.
   static size_t GetMaxRendererProcessCount();
 };
 
 }  // namespace content.
 
 #endif  // CONTENT_PUBLIC_BROWSER_RENDER_PROCESS_HOST_H_