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

content/browser/webui/web_ui_impl.h

Index: content/browser/webui/web_ui_impl.h
diff --git a/content/browser/webui/web_ui_impl.h b/content/browser/webui/web_ui_impl.h
index 86c70711282a2ec7c9e7aa78d9954b27387c7f57..3a50235bf1ebe5288e6c273c9e3eb6e371515c8e 100644
--- a/content/browser/webui/web_ui_impl.h
+++ b/content/browser/webui/web_ui_impl.h
@@ -25,13 +25,13 @@ class CONTENT_EXPORT WebUIImpl : public WebUI,
   WebUIImpl(WebContents* contents, const std::string& frame_name);
   ~WebUIImpl() override;
 
-  // Called by WebContentsImpl when the RenderView is first created. This is
-  // *not* called for every page load because in some cases
-  // RenderFrameHostManager will reuse RenderView instances.
+  // Should be called whenever a navigation causes a change in the
+  // WebUI/RenderView pair of instances: a WebUI is created for an existing
+  // RenderView, vice-versa or if both have just been created.

[Dan Beam, 2015/11/18 18:10:58]

revert to old comment or explain to me why this is better

[carlosk, 2015/11/18 20:02:01]

This change addresses a recent request by creis@ [1].

During the development of this refactor it seemed to me that RenderViewCreated
should not only be called when a new RenderView is created but also when it
remains the same and a new WebUI is created under it. This happens in a
same-site navigation where the RenderFrameHost/RenderViewHost is kepts but the
WebUI is changed to another type.

Neither this method's name nor its comment expressed well that logic as they
refer only to creations of the RenderView.

... (continues below)

[Dan Beam, 2015/11/20 00:07:59]

then how about:

// Called when a RenderView is created for a WebUI (i.e. opening a new
// tab) or a WebUI is created for an RenderView (i.e. navigating from
// chrome://downloads to chrome://bookmarks).

I'm still confused though: if both site instance and webui differ based on
origin, how can a site instance be re-used between webui types?

[carlosk, 2015/11/20 12:21:11]

Done (with updates).
We tried to avoid this kind of re-usage (see nasko@'s landed changes associated
with the same issue, http://crbug.com/508850) but it wasn't enough. Some of the
crashes we had were due to that still happening. See http://crbug.com/557116#c1
(restricted) for examples.

   void RenderViewCreated(RenderViewHost* render_view_host);
 
-  // Called by WebContentsImpl when the RenderView is reused. This happens on
-  // refresh or when the main page is navigated within the same SiteInstance.
+  // Should be called when a navigation reuses the exact same WebUI/RenderView
+  // pair of instances.

[Dan Beam, 2015/11/18 18:10:58]

same

[carlosk, 2015/11/18 20:02:01]

... (continued from above)

Following the same logic, RenderViewReused should work as the complement of the
previous one: it is called in a navigation where none of them change. And again
its name and comment are not clear and I would assume that if only the WebUI
would have changed I should still call this method.

In other words: if the navigating WebUI has never interacted with the RenderView
before the former method should be called. And if it has already seen it then we
should call the latter.

[1]
https://codereview.chromium.org/1426403006/diff/1/content/browser/frame_host/...

[Dan Beam, 2015/11/20 00:07:59]

// Called when a renderer is reused for the same WebUI type (i.e. refresh).

[carlosk, 2015/11/20 12:21:10]

Done (with minor updates).

   void RenderViewReused(RenderViewHost* render_view_host, bool was_main_frame);
 
   // WebUI implementation: