Define the public interface for content browser SiteInstance. This interface is implemented by th...

content/public/browser/site_instance.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_SITE_INSTANCE_H_
+#define CONTENT_PUBLIC_BROWSER_SITE_INSTANCE_H_
+#pragma once
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "content/common/content_export.h"
+#include "googleurl/src/gurl.h"
+
+class BrowsingInstance;
+
+namespace content {
+class BrowserContext;

[jam, 2012/01/24 03:29:33]

nit: no need for two "namespace content", just move this below

[ananta, 2012/01/24 23:46:26]

Done.

+}
+
+namespace content {
+
+// Interface that represents a site instance, which is a data structure that

[jam, 2012/01/24 03:29:33]

we usually move the entire comment from the impl class to the header (except the
parts that discuss implementation details). i think in this case it's also
useful to move the entire comment

[ananta, 2012/01/24 23:46:26]

Done.

+// is associated with all pages in a given instance of a web site.
+class CONTENT_EXPORT SiteInstance : public base::RefCounted<SiteInstance> {
+ public:
+  // Returns a unique ID for this SiteInstance.
+  virtual int32 GetId() = 0;
+
+  // Whether this SiteInstance has a running process associated with it.
+  virtual bool HasProcess() const = 0;
+
+  // Returns the current process being used to render pages in this
+  // SiteInstance.  If the process has crashed or otherwise gone away, then
+  // this method will create a new process and update our host ID accordingly.
+  virtual content::RenderProcessHost* GetProcess() = 0;
+
+  // Set / Get 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
+  // stored.
+  virtual void SetSite(const GURL& url) = 0;

[jam, 2012/01/24 03:29:33]

this is only called by code in content. so it shouldn't be in the interface,
since that should only contain functions that are called by chrome.

i haven't checked each function here. can you double check that they're all used
by chrome, and move out the ones that aren't?

[ananta, 2012/01/24 23:46:26]

Done.

+  virtual const GURL& GetSite() const = 0;
+
+  virtual bool HasSite() const = 0;
+
+  // Returns whether there is currently a related SiteInstance (registered with
+  // BrowsingInstance) for the site of the given url.  If so, we should try to
+  // avoid dedicating an unused SiteInstance to it (e.g., in a new tab).
+  virtual bool HasRelatedSiteInstance(const GURL& url) = 0;

[jam, 2012/01/24 03:29:33]

ditto

[ananta, 2012/01/24 23:46:26]

Done.

+
+  // Gets a SiteInstance for the given URL that shares the current
+  // BrowsingInstance, creating a new SiteInstance if necessary.  This ensures
+  // that a BrowsingInstance only has one SiteInstance per site, so that pages
+  // in a BrowsingInstance have the ability to script each other.  Callers
+  // should ensure that this SiteInstance becomes ref counted, by storing it in
+  // a scoped_refptr.  (By having this method, we can hide the BrowsingInstance
+  // class from the rest of the codebase.)
+  // TODO(creis): This may be an argument to build a pass_refptr<T> class, as
+  // Darin suggests.
+  virtual SiteInstance* GetRelatedSiteInstance(const GURL& url) = 0;
+
+  // Returns whether this SiteInstance has a process that is the wrong type for
+  // the given URL.  If so, the browser should force a process swap when
+  // navigating to the URL.
+  virtual bool HasWrongProcessForURL(const GURL& url) const = 0;

[jam, 2012/01/24 03:29:33]

ditto

[ananta, 2012/01/24 23:46:26]

Done.

+
+  // Browser context to which this SiteInstance (and all related
+  // SiteInstances) belongs.
+  virtual content::BrowserContext* GetBrowserContext() const = 0;
+
+  // Returns the BrowsingInstance associated with this SiteInstance.
+  virtual BrowsingInstance* GetBrowsingInstance() const = 0;

[jam, 2012/01/24 03:29:33]

ditto. we're supposed to be hiding BrowsingInstance from chrome, see Joi's
recent change  http://codereview.chromium.org/9221017/

[ananta, 2012/01/24 23:46:26]

Done.

+
+  // Factory method to create a new SiteInstance.  This will create a new
+  // new BrowsingInstance, so it should only be used when creating a new tab
+  // from scratch (or similar circumstances).  Callers should ensure that
+  // this SiteInstance becomes ref counted, by storing it in a scoped_refptr.
+  //
+  // The render process host factory may be NULL. See SiteInstance constructor.
+  //
+  // TODO(creis): This may be an argument to build a pass_refptr<T> class, as
+  // Darin suggests.
+  static SiteInstance* CreateSiteInstance(

[jam, 2012/01/24 03:29:33]

nit: I realize this was already called that, but for content api by convention
we call these methods simply "Create" or "CreateForURL" below. this matches the
WebKit API, and removes redundancy (i.e. SiteInstance::CreateSiteInstance).

[ananta, 2012/01/24 23:46:26]

Done.

+      content::BrowserContext* browser_context);
+
+  // Factory method to get the appropriate SiteInstance for the given URL, in
+  // a new BrowsingInstance.  Use this instead of CreateSiteInstance when you
+  // know the URL, since it allows special site grouping rules to be applied
+  // (for example, to group chrome-ui pages into the same instance).
+  static SiteInstance* CreateSiteInstanceForURL(
+      content::BrowserContext* browser_context, const GURL& url);
+
+  // Returns the site for the given URL, which includes only the scheme and
+  // registered domain.  Returns an empty GURL if the URL has no host.
+  static GURL GetSiteForURL(content::BrowserContext* context, const GURL& url);

[jam, 2012/01/24 03:29:33]

ditto (only called from content)

[ananta, 2012/01/24 23:46:26]

Done.

[jam, 2012/01/25 02:02:17]

it's still here?

[ananta, 2012/01/25 02:16:29]

Sorry. This one missed out :(. Done.

+
+  // Return whether both URLs are part of the same web site, for the purpose of
+  // assigning them to processes accordingly.  The decision is currently based
+  // on the registered domain of the URLs (google.com, bbc.co.uk), as well as
+  // the scheme (https, http).  This ensures that two pages will be in
+  // the same process if they can communicate with other via JavaScript.
+  // (e.g., docs.google.com and mail.google.com have DOM access to each other
+  // if they both set their document.domain properties to google.com.)
+  static bool IsSameWebSite(content::BrowserContext* browser_context,
+                            const GURL& url1, const GURL& url2);
+
+ protected:
+  friend class base::RefCounted<SiteInstance>;
+
+  SiteInstance() {}
+  ~SiteInstance() {}
+
+  // Get the effective URL for the given actual URL.
+  static GURL GetEffectiveURL(content::BrowserContext* browser_context,

[jam, 2012/01/24 03:29:33]

why is this in the interface?

[ananta, 2012/01/24 23:46:26]

Done.

+                              const GURL& url);
+};
+
+}  // namespace content.
+
+#endif  // CONTENT_PUBLIC_BROWSER_SITE_INSTANCE_H_