Reland #1 of: Move WebUI ownership from the RenderFrameHostManager to the RenderFrameHost.

content/browser/frame_host/render_frame_host_impl.h

Index: content/browser/frame_host/render_frame_host_impl.h
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
index 41f0bce99c4f4556c27eae437b3f439b594f8a30..1afa1617ff45a96c4aa398849fa51be8e317fe1e 100644
--- a/content/browser/frame_host/render_frame_host_impl.h
+++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -17,6 +17,7 @@
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/site_instance_impl.h"
+#include "content/browser/webui/web_ui_impl.h"
 #include "content/common/accessibility_mode_enums.h"
 #include "content/common/ax_content_node_data.h"
 #include "content/common/content_export.h"
@@ -218,6 +219,12 @@ class CONTENT_EXPORT RenderFrameHostImpl
   RenderFrameHostDelegate* delegate() { return delegate_; }
   FrameTreeNode* frame_tree_node() { return frame_tree_node_; }
 
+  // Returns the associated WebUI or null if none applies.
+  WebUIImpl* web_ui() const { return web_ui_.get(); }
+
+  // Returns the associated WebUI type.
+  WebUI::TypeID web_ui_type() const { return web_ui_type_; }
+
   // Returns this RenderFrameHost's loading state. This method is only used by
   // FrameTreeNode. The proper way to check whether a frame is loading is to
   // call FrameTreeNode::IsLoading.
@@ -478,6 +485,15 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // addition, its associated RenderWidgetHost has to be focused.
   bool IsFocused();
 
+  // Updates the WebUI of this RenderFrameHost based on the provided |dest_url|,
+  // setting it to either none, a new instance or simply reuses the currently

[Charlie Reis, 2015/11/09 07:01:00]

This sounds like it's stale, or it should be.  A RFH's WebUI should not change
once it has been assigned.  If that were the case, this should be called
CreateWebUI and it would verify that it doesn't get called more than once.

[carlosk, 2015/11/10 15:02:30]

Changing this method so that it works like that is not a problem. The question
then is how handle the cases when that's not what should happen? Should we
create a new UMA enumeration to report those cases so that we can track them?

For the actual behavior you are suggesting I am in doubt between:
1) This method can only be ever called once and will simply decide to either
create a WebUI or not. Future calls would CHECK/DCHECK.
2) This method could be called indefinitely until a WebUI is created. From that
point on a call would CHECK/DCHECK.
3) Same as any of the above but instead of CHECK/DCHECK it would simply ignore
future calls.

Furthermore, if we do limit this method as you suggest will that be adequate for
the non-default process models we support? If indeed they should behave
differently where should the model-dependent logic exist: here or in RFHM? If
the latter is preferable then maybe this method should stay as is.

[Charlie Reis, 2015/11/11 01:11:00]

From the perspective of security, we shouldn't be loading non-WebUI URLs in a
RFH and then later upgrade it to WebUI.  Similarly, we shouldn't be loading
non-WebUI-acceptable URLs in an RFH after it's been upgraded to WebUI.  This
means the first call seems like it should be the one that determines whether
this RFH has a WebUI or not, and it shouldn't be called again (your option 1).
The other process models should be subject to the same rules.  They all swap
RFHs on WebUI-changing navigations, after Nasko's recent CL.

[carlosk, 2015/11/13 16:43:53]

Acknowledged x2.

+  // existing one. Returns true if a WebUI change occurred.
+  // If this is a history navigation its NavigationEntry bindings should be
+  // provided through |entry_bindings| to allow verifying that they are not
+  // being set differently this time around. Otherwise |entry_bindings| should
+  // be set to NavigationEntryImpl::kInvalidBindings so that no checks are done.
+  bool UpdateWebUI(const GURL& dest_url, int entry_bindings);
+
   // Returns the Mojo ImageDownloader service.
   const image_downloader::ImageDownloaderPtr& GetMojoImageDownloader();
 
@@ -648,6 +664,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   FrameTreeNode* FindAndVerifyChild(
       int32 child_frame_routing_id, bad_message::BadMessageReason reason);
 
+  // Resets all WebUI related fields.
+  void ResetWebUI();
+
   // For now, RenderFrameHosts indirectly keep RenderViewHosts alive via a
   // refcount that calls Shutdown when it reaches zero.  This allows each
   // RenderFrameHostManager to just care about RenderFrameHosts, while ensuring
@@ -823,6 +842,12 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // NavigationHandle for it is owned by the NavigationRequest.
   scoped_ptr<NavigationHandleImpl> navigation_handle_;
 
+  // The associated WebUIImpl and its type. They will be set if the current
+  // document or the one being navigated to is from WebUI source. Otherwise they
+  // will be null and WebUI::kNoWebUI, respectively.
+  scoped_ptr<WebUIImpl> web_ui_;
+  WebUI::TypeID web_ui_type_;
+
   // NOTE: This must be the last member.
   base::WeakPtrFactory<RenderFrameHostImpl> weak_ptr_factory_;