Implement ProcessReusePolicy for SiteInstances

content/browser/renderer_host/render_process_host_impl.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_BROWSER_RENDERER_HOST_RENDER_PROCESS_HOST_IMPL_H_
 #define CONTENT_BROWSER_RENDERER_HOST_RENDER_PROCESS_HOST_IMPL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 53 matching lines @@
 class IndexedDBDispatcherHost;
 class InProcessChildThreadParams;
 class NotificationMessageFilter;
 #if BUILDFLAG(ENABLE_WEBRTC)
 class P2PSocketDispatcherHost;
 #endif
 class PermissionServiceContext;
 class PeerConnectionTrackerHost;
 class PushMessagingManager;
 class RenderFrameMessageFilter;
+class RenderProcessHostFactory;
 class RenderWidgetHelper;
 class RenderWidgetHost;
 class RenderWidgetHostImpl;
 class ResourceMessageFilter;
+class SiteInstanceImpl;
 class StoragePartition;
 class StoragePartitionImpl;
 
 namespace mojom {
 class StoragePartitionService;
 }  // namespace mojom
 
 typedef base::Thread* (*RendererMainThreadFactoryFunction)(
     const InProcessChildThreadParams& params);
 
@@ skipping 161 matching lines @@
   // Registers the given |process| to be used for any instance of |url|
   // within |browser_context|.
   // This should only be used for process-per-site mode, which can be enabled
   // globally with a command line flag or per-site, as determined by
   // SiteInstanceImpl::ShouldUseProcessPerSite.
   static void RegisterProcessHostForSite(
       BrowserContext* browser_context,
       RenderProcessHost* process,
       const GURL& url);
 
+  // The policy to apply when selecting a RenderProcessHost for a site.

[Charlie Reis, 2017/05/03 23:28:00]

I'm unclear if this is just about process-per-site style reuse or if we'll
extend it for all the types of reuse in
https://docs.google.com/a/chromium.org/document/d/1UrrmlcqwM5qIpqBM3ud3gYY5uG....

For example, normal SiteInstances don't proactively reuse processes, but when
over the limit, they reuse randomly, not based on site.  In contrast,
process-per-site and ServiceWorkers proactively join a process that already
contains the site.

The "for a site" part of this comment seems to imply it might be specifically
for the cases where affinity is based on site, but then again, it's handling
TDI's default subframe process already.  Maybe we should rephrase the comment
about how much we expect this will cover in the future?

[clamy, 2017/05/04 15:56:19]

I have rephrased the comment.

+  enum class ProcessReusePolicy {

[Charlie Reis, 2017/05/03 23:28:00]

It's a bit odd that this enum is not used in any APIs here.  Should we declare
it in SiteInstance(Impl?) instead?

[clamy, 2017/05/04 15:56:19]

Done.

+    // Do not attempt to reuse an existing RenderProcessHost.
+    DONT,

[Charlie Reis, 2017/05/03 23:28:00]

Let's clarify if this really means "never reuse, even when over the process
limit," since at first I thought it might mean "don't proactively reuse, unless
we're over the process limit" (until I saw DEFAULT).

Is there a case we know we need this for?  I see that it's easy to implement,
but I think there's still some open questions about whether it's needed.  For
example, even a site that requires a dedicated process could still share with
other instances of that site when over the limit.  Same for non-tab features
that use WebContents.  I could imagine implementing that more as pools (e.g.,
defined by site), rather than saying "never reuse under any circumstances."

In other words, we might want to leave this out until there's a clear need for
it.

[nasko, 2017/05/03 23:46:17]

nit: DO_NOT

[clamy, 2017/05/04 15:56:19]

I initially thought we might want to change
RenderProcessHostImpl::ShouldTryToUseExistingProcessHost so that when
SiteIsolationPolicy::UseDedicatedProcessesForAllSites() is true (and not in
single process mode), we intialize the SiteInstance with this value instead of
default. But this is probably out-of-scope for this CL, so I'm removing it.

+
+    // In this mode, all instances of the site will be rendered in the same

[Charlie Reis, 2017/05/03 23:28:00]

Is the idea to also use this value for ServiceWorker SiteInstances, to make them
join one (of possibly many) existing processes?  And for process-per-site sites
(e.g., WebUI), we use it for all SiteInstances in the site?

If so, we should rephrase this-- in the ServiceWorker case, we'll set this value
for the ServiceWorker SiteInstance but not "all instances of the site."  The
normal tabs for the site will still get the default value.

Since we're choosing whether to apply this value to one or all SiteInstances for
a site to define different policies, that's a second reason to move this enum to
SiteInstance instead of RenderProcessHost.

[nasko, 2017/05/03 23:46:17]

nit: s/rendererd/hosted/?

[clamy, 2017/05/04 15:56:19]

Done.

For the naming question, I'm planning a different value for ServiceWorkers,
since we also have to take into account pending navigations (I've called it
REUSE_CHECKING_FRAMES_AND_NAVIGATIONS).

If we want, we can eventually name this one REUSE_CHECKING_FRAMES. Note that in
this CL I'm trying not to make changes to the functionality, and in particular
I'm not changing the implementation of process-per-site. I'd rather leave it
named as it is, and change the name once we change the underlying implemetation.
Wdyt?

[Charlie Reis, 2017/05/04 23:12:23]

Works for me, thanks.

+    // RenderProcessHost.
+    PROCESS_PER_SITE,
+
+    // In this mode, subframes will be rendered in a designated
+    // RenderProcessHost.
+    USE_DEFAULT_SUBFRAME_INSTANCE,

[Charlie Reis, 2017/05/03 23:28:00]

s/INSTANCE/PROCESS/, perhaps?

There are multiple default subframe SiteInstances (one per BrowsingInstance),
but only one RenderProcessHost.

[clamy, 2017/05/04 15:56:19]

Done.

+
+    // By default, a new RenderProcessHost will be created unless the process
+    // limit has been reached.

[Charlie Reis, 2017/05/03 23:28:00]

Let's mention that reuse after the limit is random, not based on site.

[clamy, 2017/05/04 15:56:19]

Done.

+    DEFAULT,
+  };
+
+  // Returns a suitable RenderProcessHost to use for |site_instance|. Depending
+  // on the SiteInstance ProcessReusePolicy and its url, this may be an existing

[Charlie Reis, 2017/05/03 23:28:00]

nit: SiteInstance's

[clamy, 2017/05/04 15:56:19]

Done.

+  // RenderProcessHost or a new one.
+  static RenderProcessHost* GetProcessHostForSiteInstance(
+      BrowserContext* browser_context,
+      SiteInstanceImpl* site_instance);
+
   static base::MessageLoop* GetInProcessRendererThreadForTesting();
 
   // This forces a renderer that is running "in process" to shut down.
   static void ShutDownInProcessRenderer();
 
   static void RegisterRendererMainThreadFactory(
       RendererMainThreadFactoryFunction create);
 
   RenderFrameMessageFilter* render_frame_message_filter_for_testing() const {
     return render_frame_message_filter_.get();
@@ skipping 19 matching lines @@
 #endif  // defined(OS_POSIX) && !defined(OS_ANDROID) && !defined(OS_MACOSX)
 
   void RecomputeAndUpdateWebKitPreferences();
 
   // Called when an audio stream is added or removed and used to determine if
   // the process should be backgrounded or not.
   void OnAudioStreamAdded() override;
   void OnAudioStreamRemoved() override;
   int get_audio_stream_count_for_testing() const { return audio_stream_count_; }
 
+  // Sets the global factory used to create new RenderProcessHosts.  It may be
+  // nullptr, in which case the default RenderProcessHost will be created (this
+  // is the behavior if you don't call this function).  The factory must be set
+  // back to nullptr before it's destroyed; ownership is not transferred.
+  static void set_render_process_host_factory(

[Charlie Reis, 2017/05/03 23:28:00]

Thanks-- this makes more sense here.

+      const RenderProcessHostFactory* rph_factory);
+
  protected:
   // A proxy for our IPC::Channel that lives on the IO thread.
   std::unique_ptr<IPC::ChannelProxy> channel_;
 
   // True if fast shutdown has been performed on this RPH.
   bool fast_shutdown_started_;
 
   // True if we've posted a DeleteTask and will be deleted soon.
   bool deleting_soon_;
 
@@ skipping 73 matching lines @@
   // available for extraction by a SubprocesMetricsProvider in order to
   // report those histograms to UMA.
   void CreateSharedRendererHistogramAllocator();
 
   // Handle termination of our process.
   void ProcessDied(bool already_dead, RendererClosedDetails* known_details);
 
   // GpuSwitchingObserver implementation.
   void OnGpuSwitched() override;
 
+  // Returns the default subframe RenderProcessHost to use for |site_instance|.
+  static RenderProcessHost* GetDefaultSubframeProcessHost(
+      BrowserContext* browser_context,
+      SiteInstanceImpl* site_instance,
+      bool is_for_guests_only);
+
 #if BUILDFLAG(ENABLE_WEBRTC)
   void OnRegisterAecDumpConsumer(int id);
   void OnUnregisterAecDumpConsumer(int id);
   void RegisterAecDumpConsumerOnUIThread(int id);
   void UnregisterAecDumpConsumerOnUIThread(int id);
   void EnableAecDumpForId(const base::FilePath& file, int id);
   // Sends |file_for_transit| to the render process.
   void SendAecDumpFileToRenderer(int id,
                                  IPC::PlatformFileForTransit file_for_transit);
   void SendDisableAecDumpToRenderer();
@@ skipping 228 matching lines @@
   FrameSinkProviderImpl frame_sink_provider_;
 
   base::WeakPtrFactory<RenderProcessHostImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderProcessHostImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_RENDER_PROCESS_HOST_IMPL_H_