SiteInstance unittest improvements

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 935fc2982c2f685a9a05543f3e15db64b27a1746..ca0460ecf5019b51372b3052b1e8d4cbd639b5d5 100644
--- a/content/browser/site_instance_impl.h
+++ b/content/browser/site_instance_impl.h
@@ -20,8 +20,8 @@ namespace content {
 class BrowsingInstance;
 class RenderProcessHostFactory;
 
-class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
-                                        public RenderProcessHostObserver {
+class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
+                                              public RenderProcessHostObserver {
  public:
   class CONTENT_EXPORT Observer {
    public:
@@ -132,12 +132,10 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
  protected:

[ncarter (slow), 2016/03/31 16:53:47]

Do we still need a protected section?

I think this is one of those cases where we're pretending that 'protected
friends' exist -- ie. friends that can only access protected methods. But C++
doesn't work that way.

I understand the idiom, and I'm happy to leave it in place, but in general my
rule is 'protected is for inheritance', and friends should be in the private:
section.

Let me know what you think.

[Charlie Reis, 2016/04/01 21:50:41]

Ah yes, we should reflect reality.  Putting the friend line in the private
section makes sense.  And yes, now that we're final, we don't need the protected
section.

[ncarter (slow), 2016/04/04 21:47:33]

Done.

   friend class BrowsingInstance;
 
-  // Virtual to allow tests to extend it.
   ~SiteInstanceImpl() override;
 
-  // Create a new SiteInstance.  Protected to give access to BrowsingInstance
-  // and tests; most callers should use Create or GetRelatedSiteInstance
-  // instead.
+  // Create a new SiteInstance.  Protected to give access to BrowsingInstance;

[Charlie Reis, 2016/04/01 21:50:41]

Guess this will be stale if we remove protected.  Let's update.

[ncarter (slow), 2016/04/04 21:47:33]

Done.

+  // use Create or GetRelatedSiteInstance instead.
   explicit SiteInstanceImpl(BrowsingInstance* browsing_instance);
 
   // Only BrowsingInstance should call this.
@@ -146,6 +144,7 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance,
   }
 
  private:
+  friend class SiteInstanceTestBrowserClient;

[Charlie Reis, 2016/04/01 21:50:41]

nit: Add blank line after.

[ncarter (slow), 2016/04/04 21:47:33]

Done.

   // RenderProcessHostObserver implementation.
   void RenderProcessHostDestroyed(RenderProcessHost* host) override;
   void RenderProcessWillExit(RenderProcessHost* host) override;