Remove the is_loading_ field from WebContentsImpl

content/browser/frame_host/render_frame_host_manager.cc

 // 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.
 
 #include "content/browser/frame_host/render_frame_host_manager.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 898 matching lines @@
   if (notify_webui_of_rv_creation && GetNavigatingWebUI())
     GetNavigatingWebUI()->RenderViewCreated(navigation_rfh->render_view_host());
 
   return navigation_rfh;
 }
 
 // PlzNavigate
 void RenderFrameHostManager::CleanUpNavigation() {
   CHECK(IsBrowserSideNavigationEnabled());
   render_frame_host_->ClearPendingWebUI();
-  if (speculative_render_frame_host_)
+  if (speculative_render_frame_host_) {
+    bool was_loading = speculative_render_frame_host_->is_loading();
     DiscardUnusedFrame(UnsetSpeculativeRenderFrameHost());
+    if (was_loading)
+      frame_tree_node_->DidStopLoading();

[nasko, 2016/01/20 20:58:34]

Shouldn't loading state be independent of speculative RFH in PlzNavigate? It
should key off of the NavigationRequest, shouldn't it? In general, the
speculative RFH is an optimization and PlzNavigate should function without it
just fine.

[clamy, 2016/01/22 13:08:15]

It has to be dependent of it during the short time where we are committing the
navigation. The NavigationRequest no longer exist, so we rely on the fact that
the speculative RFH is loading to know that we are still loading a page.

[nasko, 2016/01/22 17:52:59]

Looking at the callers of CleanUpNavigation, it seems that this one might be
problematic:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr....
In the case where same process navigation commits while having a cross-process
one, cancelling the cross-process navigation should not reset the loading state.
As it is, it will, wouldn't it?

We should have a browser test for this type of navigation, let's make sure we
add checks for the loading state if possible. Otherwise it might be good to have
an explicit test for it to avoid regressing it.

+  }
 }
 
 // PlzNavigate
 scoped_ptr<RenderFrameHostImpl>
 RenderFrameHostManager::UnsetSpeculativeRenderFrameHost() {
   CHECK(IsBrowserSideNavigationEnabled());
   speculative_render_frame_host_->GetProcess()->RemovePendingView();
   return std::move(speculative_render_frame_host_);
 }
 
@@ skipping 1042 matching lines @@
   if (!delegate_->IsHidden() && new_rfh_has_view) {
     // In most cases, we need to show the new view.
     render_frame_host_->GetView()->Show();
   }
   if (!new_rfh_has_view) {
     // If the view is gone, then this RenderViewHost died while it was hidden.
     // We ignored the RenderProcessGone call at the time, so we should send it
     // now to make sure the sad tab shows up, etc.
     DCHECK(!render_frame_host_->IsRenderFrameLive());
     DCHECK(!render_frame_host_->render_view_host()->IsRenderViewLive());
+    render_frame_host_->ResetLoadingState();
     delegate_->RenderProcessGoneFromRenderManager(
         render_frame_host_->render_view_host());
   }
 
   // For top-level frames, also hide the old RenderViewHost's view.
   // TODO(creis): As long as show/hide are on RVH, we don't want to hide on
   // subframe navigations or we will interfere with the top-level frame.
   if (is_main_frame &&
       old_render_frame_host->render_view_host()->GetWidget()->GetView()) {
     old_render_frame_host->render_view_host()->GetWidget()->GetView()->Hide();
@@ skipping 231 matching lines @@
       render_frame_host_->pending_web_ui()->RenderViewCreated(
           render_frame_host_->render_view_host());
     }
   }
 }
 
 void RenderFrameHostManager::CancelPending() {
   TRACE_EVENT1("navigation", "RenderFrameHostManager::CancelPending",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
   render_frame_host_->ClearPendingWebUI();
+
+  bool pending_was_loading = pending_render_frame_host_->is_loading();
   DiscardUnusedFrame(UnsetPendingRenderFrameHost());
+  if (pending_was_loading)
+    frame_tree_node_->DidStopLoading();
 }
 
 scoped_ptr<RenderFrameHostImpl>
 RenderFrameHostManager::UnsetPendingRenderFrameHost() {
   scoped_ptr<RenderFrameHostImpl> pending_render_frame_host =
       std::move(pending_render_frame_host_);
 
   RenderFrameDevToolsAgentHost::OnCancelPendingNavigation(
       pending_render_frame_host.get(),
       render_frame_host_.get());
@@ skipping 213 matching lines @@
 int RenderFrameHostManager::GetOpenerRoutingID(SiteInstance* instance) {
   if (!frame_tree_node_->opener())
     return MSG_ROUTING_NONE;
 
   return frame_tree_node_->opener()
       ->render_manager()
       ->GetRoutingIdForSiteInstance(instance);
 }
 
 }  // namespace content