"Top Document Isolation" mode

content/browser/site_instance_impl.h

Index: content/browser/site_instance_impl.h
diff --git a/content/browser/site_instance_impl.h b/content/browser/site_instance_impl.h
index 81967cfffa12d827d3e8c35687ee63d9c6ac4fed..1922656de45ab8a9b6f788dfe2d5697ce1aa5f12 100644
--- a/content/browser/site_instance_impl.h
+++ b/content/browser/site_instance_impl.h
@@ -44,6 +44,12 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
   size_t GetRelatedActiveContentsCount() override;
   bool RequiresDedicatedProcess() override;
 
+  // Returns the SiteInstance, related to this one, that should be used
+  // for subframes when an oopif is required, but a dedicated process is not.
+  // This SiteInstance will be created if it doesn't already exist. There is
+  // at most one of these per BrowsingInstance.
+  SiteInstance* GetDefaultSubframeSiteInstance();
+
   // Set the web site that this SiteInstance is rendering pages for.
   // This includes the scheme and registered domain, but not the port.  If the
   // URL does not have a valid registered domain, then the full hostname is
@@ -88,6 +94,10 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
+  bool is_default_subframe_site_instance() {
+    return is_default_subframe_site_instance_;
+  }
+
   // Sets the global factory used to create new RenderProcessHosts.  It may be
   // NULL, 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
@@ -125,6 +135,11 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
   // instead.
   explicit SiteInstanceImpl(BrowsingInstance* browsing_instance);
 
+  // Only BrowsingInstance should call this.
+  void set_is_default_subframe_site_instance() {
+    is_default_subframe_site_instance_ = true;
+  }
+
  private:
   // RenderProcessHostObserver implementation.
   void RenderProcessHostDestroyed(RenderProcessHost* host) override;
@@ -146,7 +161,7 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
   int32_t id_;
 
   // The number of active frames in this SiteInstance.
-  size_t active_frame_count_;
+  size_t active_frame_count_ = 0;

[Charlie Reis, 2016/03/25 19:47:13]

Style question: Is this a new practice that we're moving to in general?  I
haven't seen inline initialization in other code yet, and I kind of like knowing
where to look (i.e., the constructor(s)).  I won't object if this style has
already been discussed, though.

[ncarter (slow), 2016/03/28 22:00:29]

It's been allowed for over a year:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/zqB-DySA4V0

(It's also on http://chromium-cpp.appspot.com/ as "non-static Non-Static Class
Member Initializer").

Personally I prefer it if it's used consistently within a class, and it's
particularly beneficial when there is more than one ctor. But I would be happy
to retract this part of the change if you want. It didn't occur to me that it
would be controversial.

[Charlie Reis, 2016/03/29 17:17:12]

Ah, interesting.  Thanks for letting me know.
Let's back it out of this CL, anyway, just to avoid derailing the conversation. 
Maybe we can do it separately, but this seems like one of the cases people
weren't happy with on the discussion list: initializers split between here and
in the constructor, requiring you to look in both places.  (That seems a little
harder to read and harder to verify that all of the members get initialized,
since it's easy to assume that anything missing must be handled in the other
file.)

 
   // BrowsingInstance to which this SiteInstance belongs.
   scoped_refptr<BrowsingInstance> browsing_instance_;
@@ -155,13 +170,14 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
   // This pointer will only change once the RenderProcessHost is destructed.  It
   // will still remain the same even if the process crashes, since in that
   // scenario the RenderProcessHost remains the same.
-  RenderProcessHost* process_;
+  RenderProcessHost* process_ = nullptr;
 
   // The web site that this SiteInstance is rendering pages for.
   GURL site_;
 
   // Whether SetSite has been called.
-  bool has_site_;
+  bool has_site_ = false;
+  bool is_default_subframe_site_instance_ = false;

[Charlie Reis, 2016/03/25 19:47:13]

nit: Add a blank line before and a comment about this.

[ncarter (slow), 2016/03/28 22:00:29]

Done.

 
   base::ObserverList<Observer, true> observers_;