Remove frame ID from DidCommitProvisionalLoad.

content/public/browser/render_frame_host.h

Index: content/public/browser/render_frame_host.h
diff --git a/content/public/browser/render_frame_host.h b/content/public/browser/render_frame_host.h
index 0a56349141aa023cdb2e775c12362875f0d3b7e6..8125f188a72cb986e0371f0cbf047986ba9ae2e2 100644
--- a/content/public/browser/render_frame_host.h
+++ b/content/public/browser/render_frame_host.h
@@ -27,27 +27,34 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
 
   virtual ~RenderFrameHost() {}
 
-  virtual SiteInstance* GetSiteInstance() = 0;
+  // Returns the route id for this frame.
+  virtual int GetRoutingID() const = 0;

[jam, 2014/03/01 01:21:08]

nit: here and in the rest of the file, i have avoided putting const identifier
on content api interfaces. the reason is that the embedder shouldn't care about
the implementation detail of a method. it's also cumbersome when a const method
suddenly changes implementation, and as a result, embedder code has to be
changed because it was passing const objects and now all these need to be
updated.

see
http://www.chromium.org/developers/content-module/content-api

[Charlie Reis, 2014/03/01 01:39:05]

Ah, fair point.  I'll remove that.

[Charlie Reis, 2014/03/03 07:14:50]

Done.

 
-  // Returns the process for this frame.
-  virtual RenderProcessHost* GetProcess() = 0;
+  // Returns the SiteInstance grouping all RenderFrameHosts that have script
+  // access to this RenderFrameHost, and must therefore live in the same
+  // process.
+  virtual SiteInstance* GetSiteInstance() const = 0;
 
-  // Returns the route id for this frame.
-  virtual int GetRoutingID() = 0;
+  // Returns a bitwise OR of bindings types that have been enabled for this
+  // RenderFrameHost's RenderView. See BindingsPolicy for details.
+  virtual int GetEnabledBindings() const = 0;

[jam, 2014/03/01 01:21:08]

two questions:
right now this is only called inside content so it should be on
RenderFrameHostImpl until we need it externally

but more importantly, should this be a property of WebContentsImpl instead? i.e.
do we forsee that in the future we would set this on some frames only of a page,
but not the rest? if not, then i think it's simpler to keep this on WebContents,
as that way the embedder can interact with WebContents for the most part.
RenderFrameHost is really an implementation detail.

[Charlie Reis, 2014/03/01 01:39:05]

Sure, I can change it in the next patch.
Well, it couldn't live on WebContents because it at least changes over time as
we navigate from one page to another.  Having it on RenderViewHost has helped us
catch a bunch of bugs of privileges leaking after a navigation.

That said, I can imagine case where one subframe has or lacks bindings compared
to the main frame or other frames, so I think it should eventually live on
RenderFrameHost.  One example is the <webview> tag inside apps.

[Charlie Reis, 2014/03/03 07:14:50]

Done.

[jam, 2014/03/03 16:08:04]

ah, makes senses! ignore that then :)

+
+  // Returns the process for this frame.
+  virtual RenderProcessHost* GetProcess() const = 0;
 
   // Returns the current RenderFrameHost of the parent frame, or NULL if there
   // is no parent. The result may be in a different process than the current
   // RenderFrameHost.
-  virtual RenderFrameHost* GetParent() = 0;
+  virtual RenderFrameHost* GetParent() const = 0;
 
   // Returns true if the frame is out of process.
-  virtual bool IsCrossProcessSubframe() = 0;
+  virtual bool IsCrossProcessSubframe() const = 0;
 
   // Returns the last committed URL of the frame.
-  virtual GURL GetLastCommittedURL() = 0;
+  virtual GURL GetLastCommittedURL() const = 0;
 
   // Returns the associated widget's native view.
-  virtual gfx::NativeView GetNativeView() = 0;
+  virtual gfx::NativeView GetNativeView() const = 0;
 
   // Let the renderer know that the menu has been closed.
   virtual void NotifyContextMenuClosed(
@@ -58,7 +65,7 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
       int action, const CustomContextMenuContext& context) = 0;
 
   // Temporary until we get rid of RenderViewHost.
-  virtual RenderViewHost* GetRenderViewHost() = 0;
+  virtual RenderViewHost* GetRenderViewHost() const = 0;
 
  private:
   // This interface should only be implemented inside content.