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 84c579d8a494dc03e44a932f6250579806aacbf8..3ec7fda43390f65674c5766188b8bce768bf23be 100644
--- a/content/browser/frame_host/render_frame_host_impl.h
+++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -16,6 +16,7 @@
 #include "base/time/time.h"
 #include "content/browser/accessibility/browser_accessibility_manager.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"
@@ -213,6 +214,14 @@ class CONTENT_EXPORT RenderFrameHostImpl
   RenderFrameHostDelegate* delegate() { return delegate_; }
   FrameTreeNode* frame_tree_node() { return frame_tree_node_; }
 
+  // Returns the associated active WebUI or null if none applies.
+  WebUIImpl* web_ui() const { return web_ui_.get(); }
+
+  // Returns the pending WebUI, or null of none applies.

[Charlie Reis, 2015/10/23 06:39:25]

nit: s/of/if/

[carlosk, 2015/10/27 14:35:44]

Done.

+  WebUIImpl* pending_web_ui() const {
+    return should_reuse_web_ui_ ? web_ui_.get() : pending_web_ui_.get();
+  }
+
   // 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.
@@ -468,6 +477,24 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // addition, its associated RenderWidgetHost has to be focused.
   bool IsFocused();
 
+  // Updates the pending WebUI data of this RenderFrameHost based on the
+  // provided |dest_url|, setting to either none, a new instance or to reuse the
+  // current active one. The WebUI chosen here will be made active upon the next
+  // call to CommitPendingWebUI.
+  // 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.
+  void UpdatePendingWebUI(const GURL& dest_url, int entry_bindings);
+
+  // Updates the active WebUI with the pending one set by the last call to
+  // UpdatePendingWebUI and then clears any pending data. If UpdatePendingWebUI
+  // was not called the active WebUI will simply be cleared.
+  void CommitPendingWebUI();
+
+  // Destroys the pending WebUI and cleans up any related data.
+  void DiscardPendingWebUI();
+
   // Returns the Mojo ImageDownloader service.
   const image_downloader::ImageDownloaderPtr& GetMojoImageDownloader();
 
@@ -622,6 +649,14 @@ class CONTENT_EXPORT RenderFrameHostImpl
       const content::StartNavigationParams& start_params,
       const content::RequestNavigationParams& request_params);
 
+  // Creates a WebUI object for the given URL if one applies and sets
+  // |web_ui_type| to its type, also ensuring that its bindings match
+  // |entry_bindings|. If no WebUI applies, returns null and resets web_ui_type

[Charlie Reis, 2015/10/23 06:39:25]

(Mention that we only check for a bindings match if entry_bindings is not
kInvalidBindings.)

[carlosk, 2015/10/27 14:35:44]

Done.

+  // to WebUI::kNoWebUI.
+  scoped_ptr<WebUIImpl> CreateWebUI(const GURL& dest_url,
+                                    int entry_bindings,
+                                    WebUI::TypeID* web_ui_type);

[Charlie Reis, 2015/10/23 06:39:25]

Hmm, it's unfortunate that WebUI doesn't know its own type, since that requires
us to have an out parameter here (and store the type separately below).  Ah
well; perhaps not something to tackle here.

[carlosk, 2015/10/27 14:35:44]

Yes. It was already brought up before and it was agreed it should be done
separately.

+
   // 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
@@ -790,6 +825,21 @@ 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 is from WebUI source. Otherwise they will be null and
+  // WebUI::kNoWebUI, respectively.
+  scoped_ptr<WebUIImpl> web_ui_;
+  WebUI::TypeID web_ui_type_;
+
+  // Stores a pending WebUIImpl and its type. These will only be set during a
+  // same-site navigation to a WebUI page (reusing this RenderFrameHost).

[Charlie Reis, 2015/10/23 06:39:25]

Nasko, Dan, and I haven't been able to think of a case where this happens.  When
does a navigation change the WebUI object without changing the RFH?

[carlosk, 2015/10/27 14:35:44]

(this behavior was changed before I sent out this reply but I'll leave my
original explanation here)

In terms of the code path for that it would happen when all of these are true:
* A non-default process model is in use (single-process or process-per-tab) [A]
so that ShouldTransitionCrossSite returns false [1]
* We navigate between WebUI URLs that use the same bindings so that
ShouldSwapBrowsingInstancesForNavigation doesn't return an early true [2]
* And finally the two WebUIs should be considered of different types [3]

I had the impression before I was able to reproduce this with the default
process model but I couldn't do that now. With --process-per-tab it was simple
to repro this navigating from chrome://version to chrome://history.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

[2]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

[3]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

+  scoped_ptr<WebUIImpl> pending_web_ui_;
+  WebUI::TypeID pending_web_ui_type_;
+
+  // If true at commit time the current WebUI will simply be reused (no pending
+  // one will exist to be committed).
+  bool should_reuse_web_ui_;
+
   // NOTE: This must be the last member.
   base::WeakPtrFactory<RenderFrameHostImpl> weak_ptr_factory_;