Move WebUI ownership from the RenderFrameHostManager to the RenderFrameHost.

content/browser/frame_host/render_frame_host_manager.h

 // Copyright 2013 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_BROWSER_FRAME_HOST_RENDER_FRAME_HOST_MANAGER_H_
 #define CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_HOST_MANAGER_H_
 
 #include <list>
 #include <map>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/common/referrer.h"
 #include "ui/base/page_transition_types.h"
 #include "url/origin.h"
 
 namespace content {
 class BrowserContext;
 class CrossProcessFrameConnector;
 class CrossSiteTransferringRequest;
 class FrameNavigationEntry;
 class FrameTreeNode;
 class InterstitialPageImpl;
 class NavigationControllerImpl;
 class NavigationEntry;
 class NavigationEntryImpl;
 class NavigationHandleImpl;
 class NavigationRequest;
 class NavigatorTestWithBrowserSideNavigation;
-class RenderFrameHost;
 class RenderFrameHostDelegate;
-class RenderFrameHostImpl;
 class RenderFrameHostManagerTest;
 class RenderFrameProxyHost;
 class RenderViewHost;
 class RenderViewHostImpl;
 class RenderWidgetHostDelegate;
 class RenderWidgetHostView;
 class TestWebContents;
 class WebUIImpl;
 struct CommonNavigationParams;
 struct FrameReplicationState;
@@ skipping 92 matching lines @@
                                                 RenderFrameHost* new_host,
                                                 bool is_main_frame) = 0;
     // TODO(nasko): This should be removed once extensions no longer use
     // NotificationService. See https://crbug.com/462682.
     virtual void NotifyMainFrameSwappedFromRenderManager(
         RenderViewHost* old_host,
         RenderViewHost* new_host) = 0;
     virtual NavigationControllerImpl&
         GetControllerForRenderManager() = 0;
 
-    // Creates a WebUI object for the given URL if one applies. Ownership of the
-    // returned pointer will be passed to the caller. If no WebUI applies,
-    // returns NULL.
-    virtual scoped_ptr<WebUIImpl> CreateWebUIForRenderManager(
-        const GURL& url) = 0;
-
     // Returns the navigation entry of the current navigation, or NULL if there
     // is none.
     virtual NavigationEntry*
         GetLastCommittedNavigationEntryForRenderManager() = 0;
 
     // Returns true if the location bar should be focused by default rather than
     // the page contents. The view calls this function when the tab is focused
     // to see what it should do.
     virtual bool FocusLocationBarByDefault() = 0;
 
@@ skipping 88 matching lines @@
   // Returns the speculative RenderFrameHost, or null if there is no speculative
   // one.
   RenderFrameHostImpl* speculative_frame_host() const {
     return speculative_render_frame_host_.get();
   }
 
   // TODO(creis): Remove this when we no longer use RVH for navigation.
   RenderViewHostImpl* pending_render_view_host() const;
 
   // Returns the current committed Web UI or NULL if none applies.
-  WebUIImpl* web_ui() const { return web_ui_.get(); }
+  WebUIImpl* web_ui() const { return render_frame_host_->web_ui(); }

[clamy, 2015/09/17 17:04:38]

If we move WebUIs out of the RFHM then I think these accessors should move as
well. The goal here is to remove the concept of WebUIs from the RFHM.

[carlosk, 2015/09/30 19:37:27]

I partially agree. I did this initially to make this change simpler as these 3
getters are used in almost 40 different places.

The problem is that if we eliminate them from there we have to replicate the
what-RFH-to-get-the-speculative/pending-webui-from-logic from the new
implementations for every caller. And this seems the correct place for that. I
added a future work entry in the design doc to go over this point in a future
change.

One thing we might want to consider anyway is unifying the pending/speculative
getters as they are basically all used behind a is-plz-navigate-enabled check
that could be moved into the unified method.

 
   // Returns the Web UI for the pending navigation, or NULL of none applies.
   WebUIImpl* pending_web_ui() const {
-    return pending_web_ui_.get() ? pending_web_ui_.get() :
-                                   pending_and_current_web_ui_.get();
+    return pending_render_frame_host_ ? pending_render_frame_host_->web_ui()
+                                      : render_frame_host_->pending_web_ui();
   }
 
   // PlzNavigate
   // Returns the speculative WebUI for the navigation (a newly created one or
   // the current one if it should be reused). If none is set returns nullptr.
   WebUIImpl* speculative_web_ui() const {
-    return should_reuse_web_ui_ ? web_ui_.get() : speculative_web_ui_.get();
+    return speculative_render_frame_host_
+               ? speculative_render_frame_host_->web_ui()
+               : render_frame_host_->pending_web_ui();
   }
 
   // Called when we want to instruct the renderer to navigate to the given
   // navigation entry. It may create a new RenderFrameHost or re-use an existing
   // one. The RenderFrameHost to navigate will be returned. Returns NULL if one
   // could not be created. |dest_url| takes precedence over the |frame_entry|'s
   // url (this is necessary because ReloadOriginalRequest navigates to a
   // different URL than the last committed entry, without modifying it).
   RenderFrameHostImpl* Navigate(const GURL& dest_url,
                                 const FrameNavigationEntry& frame_entry,
@@ skipping 42 matching lines @@
 
   // Called when this frame's opener is changed to the frame specified by
   // |opener_routing_id| in |source_site_instance|'s process.  This change
   // could come from either the current RenderFrameHost or one of the
   // proxies (e.g., window.open that targets a RemoteFrame by name).  The
   // updated opener will be forwarded to any other RenderFrameProxies and
   // RenderFrames for this FrameTreeNode.
   void DidChangeOpener(int opener_routing_id,
                        SiteInstance* source_site_instance);
 
-  // Sets the pending Web UI for the pending navigation, ensuring that the
-  // bindings are appropriate compared to |bindings|.
-  void SetPendingWebUI(const GURL& url, int bindings);
-
-  // Creates and initializes a RenderFrameHost. The |web_ui| is an optional
-  // input parameter used to double check bindings when swapping back in a
-  // previously existing RenderFrameHost. If |flags| has the
+  // Creates and initializes a RenderFrameHost. If |flags| has the
   // CREATE_RF_SWAPPED_OUT bit set from the CreateRenderFrameFlags enum, it will
   // initially be placed on the swapped out hosts list. If |view_routing_id_ptr|
   // is not nullptr it will be set to the routing id of the view associated with
   // the frame.
   scoped_ptr<RenderFrameHostImpl> CreateRenderFrame(SiteInstance* instance,
-                                                    WebUIImpl* web_ui,
                                                     int flags,
                                                     int* view_routing_id_ptr);
 
   // Helper method to create and initialize a RenderFrameProxyHost and return
   // its routing id.
   int CreateRenderFrameProxy(SiteInstance* instance);
 
   // Creates proxies for a new child frame at FrameTreeNode |child| in all
   // SiteInstances for which the current frame has proxies.  This method is
   // called on the parent of a new child frame before the child leaves the
@@ skipping 182 matching lines @@
   // SiteInstance's site and when we later call IsSameWebSite.  If there is no
   // current NavigationEntry, check the current SiteInstance's site, which might
   // already be committed to a Web UI URL (such as the NTP).
   bool ShouldSwapBrowsingInstancesForNavigation(
       const GURL& current_effective_url,
       bool current_is_view_source_mode,
       SiteInstance* new_site_instance,
       const GURL& new_effective_url,
       bool new_is_view_source_mode) const;
 
-  // Creates a new Web UI, ensuring that the bindings are appropriate compared
-  // to |bindings|.
-  scoped_ptr<WebUIImpl> CreateWebUI(const GURL& url, int bindings);
-
-  // Returns true if it is safe to reuse the current WebUI when navigating from
-  // |current_entry| to |new_url|.
-  bool ShouldReuseWebUI(
-      const NavigationEntry* current_entry,
-      const GURL& new_url) const;
-
   // Returns the SiteInstance to use for the navigation.
   SiteInstance* GetSiteInstanceForNavigation(const GURL& dest_url,
                                              SiteInstance* source_instance,
                                              SiteInstance* dest_instance,
                                              SiteInstance* candidate_instance,
                                              ui::PageTransition transition,
                                              bool dest_is_restore,
                                              bool dest_is_view_source_mode);
 
   // Returns a descriptor of the appropriate SiteInstance object for the given
@@ skipping 25 matching lines @@
   // description, it is returned as is.
   SiteInstance* ConvertToSiteInstance(const SiteInstanceDescriptor& descriptor,
                                       SiteInstance* candidate_instance);
 
   // Determines the appropriate url to use as the current url for SiteInstance
   // selection.
   const GURL& GetCurrentURLForSiteInstance(
       SiteInstance* current_instance,
       NavigationEntry* current_entry);
 
+  // This is the private internal version of CreateRenderFrame that also allows
+  // the caller to specify a URL and bindings for the potential setup of a
+  // WebUI.
+  scoped_ptr<RenderFrameHostImpl> CreateRenderFrameInternal(
+      SiteInstance* instance,
+      const GURL& url,
+      int bindings,
+      int flags,
+      int* view_routing_id_ptr);
+
   // Creates a new RenderFrameHostImpl for the |new_instance| and assign it to
   // |pending_render_frame_host_| while respecting the opener route if needed
   // and stores it in pending_render_frame_host_.
   void CreatePendingRenderFrameHost(SiteInstance* old_instance,
-                                    SiteInstance* new_instance);
+                                    SiteInstance* new_instance,
+                                    const GURL& url,
+                                    int bindings);
 
   // Ensure that we have created all needed proxies for a new RFH with
   // SiteInstance |new_instance|: (1) create swapped-out RVHs and proxies for
   // the new RFH's opener chain if we are staying in the same BrowsingInstance;
   // (2) Create proxies for the new RFH's SiteInstance in its own frame tree.
   void CreateProxiesForNewRenderFrameHost(SiteInstance* old_instance,
                                           SiteInstance* new_instance);
 
   // Traverse the opener chain and populate |opener_frame_trees| with
   // all FrameTrees accessible by following frame openers of nodes in the
@@ skipping 17 matching lines @@
 
   // Creates a RenderFrameHost and corresponding RenderViewHost if necessary.
   scoped_ptr<RenderFrameHostImpl> CreateRenderFrameHost(SiteInstance* instance,
                                                         int32 view_routing_id,
                                                         int32 frame_routing_id,
                                                         int32 widget_routing_id,
                                                         int32 surface_id,
                                                         int flags);
 
   // PlzNavigate
-  // Creates and initializes a speculative RenderFrameHost and/or WebUI for an
-  // ongoing navigation. They might be destroyed and re-created later if the
-  // navigation is redirected to a different SiteInstance.
+  // Creates and initializes a speculative RenderFrameHost for an ongoing
+  // navigation. They might be destroyed and re-created later if the navigation
+  // is redirected to a different SiteInstance.
   bool CreateSpeculativeRenderFrameHost(const GURL& url,
                                         SiteInstance* old_instance,
                                         SiteInstance* new_instance,
                                         int bindings);
 
   // Sets up the necessary state for a new RenderViewHost.  Creates a
   // RenderFrameProxy in the target renderer process with the given
   // |proxy_routing_id|, which is used to route IPC messages when in swapped
   // out state.  Returns early if the RenderViewHost has already been
   // initialized for another RenderFrameHost.
   bool InitRenderView(RenderViewHostImpl* render_view_host,
                       int proxy_routing_id);
 
   // Initialization for RenderFrameHost uses the same sequence as InitRenderView
   // above.
   bool InitRenderFrame(RenderFrameHostImpl* render_frame_host);
 
-  // Sets the pending RenderFrameHost/WebUI to be the active one. Note that this
-  // doesn't require the pending render_frame_host_ pointer to be non-NULL,
-  // since there could be Web UI switching as well. Call this for every commit.
+  // Sets the pending RenderFrameHost to be the active one. Note that if the
+  // current one is being reused this method might switch its WebUI if there is
+  // a pending one. Call this for every commit.
   // If PlzNavigate is enabled the method will set the speculative (not pending)
   // RenderFrameHost to be the active one.
   void CommitPending();
 
   // Helper to call CommitPending() in all necessary cases.
   void CommitPendingIfNecessary(RenderFrameHostImpl* render_frame_host,
                                 bool was_caused_by_user_gesture);
 
   // Commits any pending sandbox flag updates when the renderer's frame
   // navigates.
@@ skipping 51 matching lines @@
 
   // Our delegate, not owned by us. Guaranteed non-NULL.
   Delegate* delegate_;
 
   // Implemented by the owner of this class.  These delegates are installed into
   // all the RenderFrameHosts that we create.
   RenderFrameHostDelegate* render_frame_delegate_;
   RenderViewHostDelegate* render_view_delegate_;
   RenderWidgetHostDelegate* render_widget_delegate_;
 
-  // Our RenderFrameHost and its associated Web UI (if any, will be NULL for
-  // non-WebUI pages). This object is responsible for all communication with
-  // a child RenderFrame instance.
+  // Our RenderFrameHost which is responsible for all communication with a child
+  // RenderFrame instance.
   // For now, RenderFrameHost keeps a RenderViewHost in its SiteInstance alive.
   // Eventually, RenderViewHost will be replaced with a page context.
   scoped_ptr<RenderFrameHostImpl> render_frame_host_;
-  scoped_ptr<WebUIImpl> web_ui_;
 
   // A RenderFrameHost used to load a cross-site page. This remains hidden
-  // while a cross-site request is pending until it calls DidNavigate. It may
-  // have an associated Web UI, in which case the Web UI pointer will be non-
-  // NULL.
-  //
-  // The |pending_web_ui_| may be non-NULL even when the
-  // |pending_render_frame_host_| is NULL. This will happen when we're
-  // transitioning between two Web UI pages: the RFH won't be swapped, so the
-  // pending pointer will be unused, but there will be a pending Web UI
-  // associated with the navigation.
-  // Note: This is not used in PlzNavigate.
+  // while a cross-site request is pending until it calls DidNavigate.
+  // Note: This member is not used in PlzNavigate.
   scoped_ptr<RenderFrameHostImpl> pending_render_frame_host_;
 
   // If a pending request needs to be transferred to another process, this
   // owns the request until it's transferred to the new process, so it will be
   // cleaned up if the navigation is cancelled.  Otherwise, this is NULL.
   scoped_ptr<CrossSiteTransferringRequest> cross_site_transferring_request_;
 
   // This is used to temporarily store the NavigationHandle during
   // transferring navigations. The handle needs to be stored because the old
   // RenderFrameHost may be discarded before a new RenderFrameHost is created
   // for the navigation.
   // PlzNavigate: this will never be set since there are no transferring
   // navigations in PlzNavigate.
   scoped_ptr<NavigationHandleImpl> transfer_navigation_handle_;
 
-  // If either of these is non-NULL, the pending navigation is to a chrome:
-  // page. The scoped_ptr is used if pending_web_ui_ != web_ui_, the WeakPtr is
-  // used for when they reference the same object. If either is non-NULL, the
-  // other should be NULL.
-  // Note: These are not used in PlzNavigate.
-  scoped_ptr<WebUIImpl> pending_web_ui_;
-  base::WeakPtr<WebUIImpl> pending_and_current_web_ui_;
-
   class RenderFrameProxyHostMap;
   scoped_ptr<RenderFrameProxyHostMap> proxy_hosts_;
 
   // A list of RenderFrameHosts waiting to shut down after swapping out.  We use
   // a linked list since we expect frequent deletes and no indexed access, and
   // because sets don't appear to support linked_ptrs.
   typedef std::list<linked_ptr<RenderFrameHostImpl> > RFHPendingDeleteList;
   RFHPendingDeleteList pending_delete_hosts_;
 
   // The intersitial page currently shown if any, not own by this class
   // (the InterstitialPage is self-owned, it deletes itself when hidden).
   InterstitialPageImpl* interstitial_page_;
 
   // PlzNavigate
-  // These members store a speculative RenderFrameHost and WebUI. They are
-  // created early in a navigation so a renderer process can be started in
-  // parallel, if needed. This is purely a performance optimization and is not
-  // required for correct behavior. The created RenderFrameHost might be
-  // discarded later on if the final URL's SiteInstance isn't compatible with
-  // what was used to create it.
-  // Note: PlzNavigate only uses speculative RenderFrameHost and WebUI, not
-  // the pending ones.
+  // Stores a speculative RenderFrameHost which is created early in a navigation
+  // so a renderer process can be started in parallel, if needed.
+  // This is purely a performance optimization and is not required for correct
+  // behavior. The speculative RenderFrameHost might be discarded later on if
+  // the final URL's SiteInstance isn't compatible with the one used to create
+  // it.
+  // Note: PlzNavigate only uses the speculative RenderFrameHost, not the
+  // pending one.
   scoped_ptr<RenderFrameHostImpl> speculative_render_frame_host_;
-  scoped_ptr<WebUIImpl> speculative_web_ui_;
-
-  // PlzNavigate
-  // If true at navigation commit time the current WebUI will be kept instead of
-  // creating a new one.
-  bool should_reuse_web_ui_;
 
   base::WeakPtrFactory<RenderFrameHostManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderFrameHostManager);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_HOST_MANAGER_H_