SiteInstance unittest improvements

content/browser/site_instance_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_SITE_INSTANCE_IMPL_H_
 #define CONTENT_BROWSER_SITE_INSTANCE_IMPL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/site_instance.h"
 #include "url/gurl.h"
 
 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:
     // Called when this SiteInstance transitions to having no active frames,
     // as measured by active_frame_count().
     virtual void ActiveFrameCountIsZero(SiteInstanceImpl* site_instance) {}
 
     // Called when the renderer process of this SiteInstance has exited.
     virtual void RenderProcessGone(SiteInstanceImpl* site_instance) = 0;
   };
@@ skipping 87 matching lines @@
 
   // Returns true if pages loaded from |effective_url| ought to be handled only
   // by a renderer process isolated from other sites. If --site-per-process is
   // on the command line, this is true for all sites. In other site isolation
   // modes, only a subset of sites will require dedicated processes.
   //
   // |effective_url| must be an effective URL.
   static bool DoesSiteRequireDedicatedProcess(BrowserContext* browser_context,
                                               const GURL& effective_url);
 
  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.
   void set_is_default_subframe_site_instance() {
     is_default_subframe_site_instance_ = true;
   }
 
  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;
   void RenderProcessExited(RenderProcessHost* host,
                            base::TerminationStatus status,
                            int exit_code) override;
 
   // Used to restrict a process' origin access rights.
   void LockToOrigin();
 
@@ skipping 30 matching lines @@
   bool is_default_subframe_site_instance_;
 
   base::ObserverList<Observer, true> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(SiteInstanceImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SITE_INSTANCE_IMPL_H_