Remove RFHM::GetNavigatingWebUI() and RFHM::web_ui().

content/browser/frame_host/render_frame_host_manager.cc

Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 42134daee6d15133221653119436803f1268a8cc..fd177759a874b8b72d641699bb418fe519316b73 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -283,21 +283,6 @@ RenderViewHostImpl* RenderFrameHostManager::pending_render_view_host() const {
   return pending_render_frame_host_->render_view_host();
 }
 
-WebUIImpl* RenderFrameHostManager::GetNavigatingWebUI() const {
-  if (current_web_ui_is_navigating_)
-    return render_frame_host_->web_ui();
-
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableBrowserSideNavigation)) {
-    if (speculative_render_frame_host_)
-      return speculative_render_frame_host_->web_ui();
-  } else {
-    if (pending_render_frame_host_)
-      return pending_render_frame_host_->web_ui();
-  }
-  return nullptr;
-}
-
 RenderWidgetHostView* RenderFrameHostManager::GetRenderWidgetHostView() const {
   if (interstitial_page_)
     return interstitial_page_->GetView();
@@ -414,8 +399,8 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate(
     if (!InitRenderView(dest_render_frame_host->render_view_host(), nullptr))
       return nullptr;
 
-    if (GetNavigatingWebUI()) {
-      GetNavigatingWebUI()->RenderViewCreated(
+    if (dest_render_frame_host->web_ui()) {
+      dest_render_frame_host->web_ui()->RenderViewCreated(
           dest_render_frame_host->render_view_host());
     }
 
@@ -674,9 +659,9 @@ void RenderFrameHostManager::CommitPendingIfNecessary(
     // We should only hear this from our current renderer.
     DCHECK_EQ(render_frame_host_, render_frame_host);
 
-    // A commit is required if there is a navigating WebUI, even without a
-    // pending or speculative RenderFrameHost.
-    if (GetNavigatingWebUI())
+    // If the current RenderFrameHost has a WebUI it must be committed (for
+    // proper focus setting).

[nasko, 2015/11/04 17:23:26]

Is the detail about focus setting important?

[carlosk, 2015/11/04 21:50:38]

I chose to mention it here because I spent some time on a request by creis@ [1]
to make this method care *only* about cross-site navigations and move all
current RFH commit logic to its own RFHI::OnDidCommitProvisionalLoad. But that
effort failed precisely because I didn't notice this detail until later on.

Finally, I added this note to facilitate a next effort to execute that change.

[1]
https://codereview.chromium.org/1352813006/diff/460001/content/browser/frame_...

[nasko, 2015/11/05 00:46:43]

Let's put that as a separate sentence then explaining what the pitfall might be.
As it is, it doesn't seem important and is easily confusing.

[carlosk, 2015/11/05 13:40:41]

Done.

+    if (render_frame_host_->web_ui())
       CommitPending();
     return;
   }
@@ -1037,17 +1022,17 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
 
     // If there is a WebUI in the current RenderFrameHost, it will navigate.
     current_web_ui_is_navigating_ = !!render_frame_host_->web_ui();
-    DCHECK_EQ(GetNavigatingWebUI(), render_frame_host_->web_ui());
 
     // If the current RenderFrameHost has a WebUI and the associated RenderFrame
     // is alive, notify to the WebUI that the RenderView is being created or
     // reused depending on whether the WebUI was changed or not.
-    if (GetNavigatingWebUI() && render_frame_host_->IsRenderFrameLive()) {
+    if (render_frame_host_->web_ui() &&
+        render_frame_host_->IsRenderFrameLive()) {
       if (changed_web_ui) {
-        GetNavigatingWebUI()->RenderViewCreated(
+        render_frame_host_->web_ui()->RenderViewCreated(
             render_frame_host_->render_view_host());
       } else {
-        GetNavigatingWebUI()->RenderViewReused(
+        render_frame_host_->web_ui()->RenderViewReused(
             render_frame_host_->render_view_host(),
             frame_tree_node_->IsMainFrame());
       }
@@ -1084,7 +1069,6 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
       }
     }
     DCHECK(speculative_render_frame_host_);
-    DCHECK_EQ(GetNavigatingWebUI(), speculative_render_frame_host_->web_ui());
 
     navigation_rfh = speculative_render_frame_host_.get();
 
@@ -1111,8 +1095,8 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
     if (!InitRenderView(navigation_rfh->render_view_host(), nullptr))
       return nullptr;
 
-    if (GetNavigatingWebUI()) {
-      GetNavigatingWebUI()->RenderViewCreated(
+    if (navigation_rfh->web_ui()) {
+      navigation_rfh->web_ui()->RenderViewCreated(
           navigation_rfh->render_view_host());
     }
 
@@ -2118,7 +2102,7 @@ void RenderFrameHostManager::CommitPending() {
   bool will_focus_location_bar = delegate_->FocusLocationBarByDefault();
 
   if (!pending_render_frame_host_ && !speculative_render_frame_host_) {
-    DCHECK_EQ(current_web_ui_is_navigating_, !!render_frame_host_->web_ui());
+    DCHECK(!current_web_ui_is_navigating_ || render_frame_host_->web_ui());

[nasko, 2015/11/04 17:23:26]

I thought this variable is only needed to support GetNavigatingWebUI.

[carlosk, 2015/11/04 21:50:38]

Oops! My bad! I forgot to remove it but it was already not doing anything really
useful.

Done now.

     current_web_ui_is_navigating_ = false;
     // If there's no pending/speculative RenderFrameHost then the current
     // RenderFrameHost is committing.
@@ -2333,9 +2317,6 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
           pending_render_frame_host_->render_view_host());
     }
 
-    // We now have a pending RFH and possibly an associated pending WebUI.
-    DCHECK_EQ(GetNavigatingWebUI(), pending_render_frame_host_->web_ui());
-
     // Check if our current RFH is live before we set up a transition.
     if (!render_frame_host_->IsRenderFrameLive()) {
       // The current RFH is not live.  There's no reason to sit around with a
@@ -2398,17 +2379,16 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
 
   // If there is a WebUI in the current RenderFrameHost, it will navigate.
   current_web_ui_is_navigating_ = !!render_frame_host_->web_ui();
-  DCHECK_EQ(GetNavigatingWebUI(), render_frame_host_->web_ui());
 
   // If the current RenderFrameHost has a WebUI and the associated RenderFrame
   // is alive, notify to the WebUI that the RenderView is being created or
   // reused depending on whether the WebUI was changed or not.
-  if (GetNavigatingWebUI() && render_frame_host_->IsRenderFrameLive()) {
+  if (render_frame_host_->web_ui() && render_frame_host_->IsRenderFrameLive()) {
     if (changed_web_ui) {
-      GetNavigatingWebUI()->RenderViewCreated(
+      render_frame_host_->web_ui()->RenderViewCreated(
           render_frame_host_->render_view_host());
     } else {
-      GetNavigatingWebUI()->RenderViewReused(
+      render_frame_host_->web_ui()->RenderViewReused(
           render_frame_host_->render_view_host(),
           frame_tree_node_->IsMainFrame());
     }