Move DidCommitProvisionalLoad code from RenderView to RenderFrame.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 8dac42d29f9d94af2feab5c61f5f5934aa6a5533..f3944fe68a39654f19a7d23cea4cda4e1bdc7b8a 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -2063,6 +2063,80 @@ void WebContentsImpl::DidRedirectProvisionalLoad(
                                       render_frame_host));
 }
 
+void WebContentsImpl::DidCommitProvisionalLoad(
+    int64 frame_id,
+    const base::string16& frame_unique_name,
+    bool is_main_frame,
+    const GURL& url,
+    PageTransition transition_type,
+    RenderFrameHostImpl* render_frame_host) {
+  // Notify observers about the commit of the provisional load.
+  FOR_EACH_OBSERVER(
+      WebContentsObserver,
+      observers_,
+      DidCommitProvisionalLoadForFrame(frame_id,
+                                       frame_unique_name,
+                                       is_main_frame,
+                                       url,
+                                       transition_type,
+                                       render_frame_host->render_view_host()));
+}
+
+void WebContentsImpl::DidNavigateMainFramePostCommit(
+    const LoadCommittedDetails& details,
+    const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
+  if (details.is_navigation_to_different_page()) {
+    // Clear the status bubble. This is a workaround for a bug where WebKit
+    // doesn't let us know that the cursor left an element during a
+    // transition (this is also why the mouse cursor remains as a hand after
+    // clicking on a link); see bugs 1184641 and 980803. We don't want to
+    // clear the bubble when a user navigates to a named anchor in the same
+    // page.
+    UpdateTargetURL(details.entry->GetPageID(), GURL());
+  }
+
+  if (!details.is_in_page) {
+    // Once the main frame is navigated, we're no longer considered to have
+    // displayed insecure content.
+    displayed_insecure_content_ = false;
+    SSLManager::NotifySSLInternalStateChanged(
+        GetController().GetBrowserContext());
+  }
+
+  // Notify observers about navigation.
+  FOR_EACH_OBSERVER(WebContentsObserver, observers_,
+                    DidNavigateMainFrame(details, params));
+
+  if (delegate_) {
+    delegate_->DidNavigateMainFramePostCommit(this);
+    view_->SetOverscrollControllerEnabled(delegate_->CanOverscrollContent());

[Charlie Reis, 2014/02/05 23:30:37]

Whew, this code is tricky.  Yes, I think moving this here is equivalent.

[nasko, 2014/02/06 01:55:13]

Done.

+  }
+}
+
+void WebContentsImpl::DidNavigateAnyFramePostCommit(
+    RenderFrameHostImpl* render_frame_host,
+    const LoadCommittedDetails& details,
+    const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
+  // If we navigate off the page, close all JavaScript dialogs.
+  if (dialog_manager_ && !details.is_in_page)
+    dialog_manager_->CancelActiveAndPendingDialogs(this);
+
+  // Notify observers about navigation.
+  FOR_EACH_OBSERVER(WebContentsObserver, observers_,
+                    DidNavigateAnyFrame(details, params));
+}
+
+void WebContentsImpl::SetMainFrameMimeType(const std::string& mime_type) {
+  contents_mime_type_ = mime_type;
+}
+
+bool WebContentsImpl::CanOverscrollContent() {
+  if (delegate_)
+    return delegate_->CanOverscrollContent();
+
+  return false;
+}
+
 void WebContentsImpl::OnDidLoadResourceFromMemoryCache(
     const GURL& url,
     const std::string& security_info,
@@ -2507,45 +2581,6 @@ void WebContentsImpl::SetIsLoading(RenderViewHost* render_view_host,
       type, Source<NavigationController>(&controller_), det);
 }
 
-void WebContentsImpl::DidNavigateMainFramePostCommit(
-    const LoadCommittedDetails& details,
-    const ViewHostMsg_FrameNavigate_Params& params) {
-  if (details.is_navigation_to_different_page()) {
-    // Clear the status bubble. This is a workaround for a bug where WebKit
-    // doesn't let us know that the cursor left an element during a
-    // transition (this is also why the mouse cursor remains as a hand after
-    // clicking on a link); see bugs 1184641 and 980803. We don't want to
-    // clear the bubble when a user navigates to a named anchor in the same
-    // page.
-    UpdateTargetURL(details.entry->GetPageID(), GURL());
-  }
-
-  if (!details.is_in_page) {
-    // Once the main frame is navigated, we're no longer considered to have
-    // displayed insecure content.
-    displayed_insecure_content_ = false;
-    SSLManager::NotifySSLInternalStateChanged(
-        GetController().GetBrowserContext());
-  }
-
-  // Notify observers about navigation.
-  FOR_EACH_OBSERVER(WebContentsObserver, observers_,
-                    DidNavigateMainFrame(details, params));
-}
-
-void WebContentsImpl::DidNavigateAnyFramePostCommit(
-    RenderViewHost* render_view_host,
-    const LoadCommittedDetails& details,
-    const ViewHostMsg_FrameNavigate_Params& params) {
-  // If we navigate off the page, close all JavaScript dialogs.
-  if (dialog_manager_ && !details.is_in_page)
-    dialog_manager_->CancelActiveAndPendingDialogs(this);
-
-  // Notify observers about navigation.
-  FOR_EACH_OBSERVER(WebContentsObserver, observers_,
-                    DidNavigateAnyFrame(details, params));
-}
-
 bool WebContentsImpl::ShouldAssignSiteForURL(const GURL& url) {

[Charlie Reis, 2014/02/05 23:30:37]

Let's delete this now that you've moved it to NavigatorImpl.

[nasko, 2014/02/06 01:55:13]

Done.

   // about:blank should not "use up" a new SiteInstance.  The SiteInstance can
   // still be used for a normal web site.
@@ -2816,145 +2851,11 @@ void WebContentsImpl::RenderViewDeleted(RenderViewHost* rvh) {
 
 void WebContentsImpl::DidNavigate(
     RenderViewHost* rvh,
-    const ViewHostMsg_FrameNavigate_Params& orig_params) {
-  ViewHostMsg_FrameNavigate_Params params(orig_params);
-  bool use_site_per_process =
-      CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess);
-  if (frame_tree_.IsFirstNavigationAfterSwap()) {
-    // First navigation should be a main frame navigation.
-    // TODO(creis): This DCHECK is currently disabled for --site-per-process
-    // because cross-process subframe navigations still have a main frame
-    // PageTransition.
-    if (!use_site_per_process)
-      DCHECK(PageTransitionIsMainFrame(params.transition));
-    frame_tree_.OnFirstNavigationAfterSwap(params.frame_id);
-  }
-
-  // When using --site-per-process, look up the FrameTreeNode ID that the
-  // renderer-specific frame ID corresponds to.
-  int64 frame_tree_node_id = frame_tree_.root()->frame_tree_node_id();
-  if (use_site_per_process) {
-    FrameTreeNode* source_node = frame_tree_.FindByFrameID(params.frame_id);
-    if (source_node)
-      frame_tree_node_id = source_node->frame_tree_node_id();
-
-    // TODO(creis): In the short term, cross-process subframe navigations are
-    // happening in the pending RenderViewHost's top-level frame.  (We need to
-    // both mirror the frame tree and get the navigation to occur in the correct
-    // subframe to fix this.)  Until then, we should check whether we have a
-    // pending NavigationEntry with a frame ID and if so, treat the
-    // cross-process "main frame" navigation as a subframe navigation.  This
-    // limits us to a single cross-process subframe per RVH, and it affects
-    // NavigateToEntry, NavigatorImpl::DidStartProvisionalLoad, and
-    // OnDidFinishLoad.
-    NavigationEntryImpl* pending_entry =
-        NavigationEntryImpl::FromNavigationEntry(controller_.GetPendingEntry());
-    int root_ftn_id = frame_tree_.root()->frame_tree_node_id();
-    if (pending_entry &&
-        pending_entry->frame_tree_node_id() != -1 &&
-        pending_entry->frame_tree_node_id() != root_ftn_id) {
-      params.transition = PAGE_TRANSITION_AUTO_SUBFRAME;
-      frame_tree_node_id = pending_entry->frame_tree_node_id();
-    }
-  }
-
-  if (PageTransitionIsMainFrame(params.transition)) {
-    // When overscroll navigation gesture is enabled, a screenshot of the page
-    // in its current state is taken so that it can be used during the
-    // nav-gesture. It is necessary to take the screenshot here, before calling
-    // RenderFrameHostManager::DidNavigateMainFrame, because that can change
-    // WebContents::GetRenderViewHost to return the new host, instead of the one
-    // that may have just been swapped out.
-    if (delegate_ && delegate_->CanOverscrollContent())
-      controller_.TakeScreenshot();
-
-    if (!use_site_per_process)
-      GetRenderManager()->DidNavigateMainFrame(rvh);
-  }
-
-  // When using --site-per-process, we notify the RFHM for all navigations,
-  // not just main frame navigations.
-  if (use_site_per_process) {
-    FrameTreeNode* frame = frame_tree_.FindByID(frame_tree_node_id);
-    // TODO(creis): Rename to DidNavigateFrame.
-    frame->render_manager()->DidNavigateMainFrame(rvh);
-  }
-
-  // Update the site of the SiteInstance if it doesn't have one yet, unless
-  // assigning a site is not necessary for this URL.  In that case, the
-  // SiteInstance can still be considered unused until a navigation to a real
-  // page.
-  if (!static_cast<SiteInstanceImpl*>(GetSiteInstance())->HasSite() &&
-      ShouldAssignSiteForURL(params.url)) {
-    static_cast<SiteInstanceImpl*>(GetSiteInstance())->SetSite(params.url);
-  }
-
-  // Need to update MIME type here because it's referred to in
-  // UpdateNavigationCommands() called by RendererDidNavigate() to
-  // determine whether or not to enable the encoding menu.
-  // It's updated only for the main frame. For a subframe,
-  // RenderView::UpdateURL does not set params.contents_mime_type.
-  // (see http://code.google.com/p/chromium/issues/detail?id=2929 )
-  // TODO(jungshik): Add a test for the encoding menu to avoid
-  // regressing it again.
-  if (PageTransitionIsMainFrame(params.transition))
-    contents_mime_type_ = params.contents_mime_type;
-
-  LoadCommittedDetails details;
-  bool did_navigate = controller_.RendererDidNavigate(rvh, params, &details);
-
-  // For now, keep track of each frame's URL in its FrameTreeNode.  This lets
-  // us estimate our process count for implementing OOP iframes.
-  // TODO(creis): Remove this when we track which pages commit in each frame.
-  frame_tree_.SetFrameUrl(params.frame_id, params.url);
-
-  // Send notification about committed provisional loads. This notification is
-  // different from the NAV_ENTRY_COMMITTED notification which doesn't include
-  // the actual URL navigated to and isn't sent for AUTO_SUBFRAME navigations.
-  if (details.type != NAVIGATION_TYPE_NAV_IGNORE) {
-    // For AUTO_SUBFRAME navigations, an event for the main frame is generated
-    // that is not recorded in the navigation history. For the purpose of
-    // tracking navigation events, we treat this event as a sub frame navigation
-    // event.
-    bool is_main_frame = did_navigate ? details.is_main_frame : false;
-    PageTransition transition_type = params.transition;
-    // Whether or not a page transition was triggered by going backward or
-    // forward in the history is only stored in the navigation controller's
-    // entry list.
-    if (did_navigate &&
-        (controller_.GetLastCommittedEntry()->GetTransitionType() &
-            PAGE_TRANSITION_FORWARD_BACK)) {
-      transition_type = PageTransitionFromInt(
-          params.transition | PAGE_TRANSITION_FORWARD_BACK);
-    }
-    // Notify observers about the commit of the provisional load.
-    FOR_EACH_OBSERVER(WebContentsObserver, observers_,
-                      DidCommitProvisionalLoadForFrame(
-                          params.frame_id,
-                          params.frame_unique_name,
-                          is_main_frame,
-                          params.url,
-                          transition_type,
-                          rvh));
-  }
-
-  if (!did_navigate)
-    return;  // No navigation happened.
-
-  // DO NOT ADD MORE STUFF TO THIS FUNCTION! Your component should either listen
-  // for the appropriate notification (best) or you can add it to
-  // DidNavigateMainFramePostCommit / DidNavigateAnyFramePostCommit (only if
-  // necessary, please).
-
-  // Run post-commit tasks.
-  if (details.is_main_frame) {
-    DidNavigateMainFramePostCommit(details, params);
-    if (delegate_) {
-      delegate_->DidNavigateMainFramePostCommit(this);
-      view_->SetOverscrollControllerEnabled(delegate_->CanOverscrollContent());
-    }
-  }
-  DidNavigateAnyFramePostCommit(rvh, details, params);
+    const FrameHostMsg_DidCommitProvisionalLoad_Params& orig_params) {
+  RenderViewHostImpl* render_view_host = static_cast<RenderViewHostImpl*>(rvh);
+  RenderFrameHostImpl* rfh = RenderFrameHostImpl::FromID(
+      rvh->GetProcess()->GetID(), render_view_host->main_frame_routing_id());
+  frame_tree_.root()->navigator()->DidNavigate(rfh, orig_params);

[Charlie Reis, 2014/02/05 23:30:37]

If we're just keeping this for TestWebContents::TestDidNavigateWithReferrer, can
we move this code there and remove WebContentsImpl::DidNavigate entirely?

[nasko, 2014/02/06 20:08:00]

Done.

 }
 
 void WebContentsImpl::UpdateState(RenderViewHost* rvh,