Reland #1 Remove the is_loading_ field from WebContentsImpl

content/browser/frame_host/render_frame_host_impl.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_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/containers/hash_tables.h"
@@ skipping 196 matching lines @@
       pending_web_ui_type_(WebUI::kNoWebUI),
       should_reuse_web_ui_(false),
       weak_ptr_factory_(this) {
   bool is_swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
   bool hidden = !!(flags & CREATE_RF_HIDDEN);
   frame_tree_->AddRenderViewHostRef(render_view_host_);
   GetProcess()->AddRoute(routing_id_, this);
   g_routing_id_frame_map.Get().insert(std::make_pair(
       RenderFrameHostID(GetProcess()->GetID(), routing_id_),
       this));
+  site_instance_->AddObserver(this);
 
   if (is_swapped_out) {
     rfh_state_ = STATE_SWAPPED_OUT;
   } else {
     rfh_state_ = STATE_DEFAULT;
     GetSiteInstance()->IncrementActiveFrameCount();
   }
 
   // New child frames should inherit the nav_entry_id of their parent.
   if (frame_tree_node_->parent()) {
@@ skipping 28 matching lines @@
 
 RenderFrameHostImpl::~RenderFrameHostImpl() {
   // Release the WebUI instances before all else as the WebUI may accesses the
   // RenderFrameHost during cleanup.
   ClearAllWebUI();
 
   GetProcess()->RemoveRoute(routing_id_);
   g_routing_id_frame_map.Get().erase(
       RenderFrameHostID(GetProcess()->GetID(), routing_id_));
 
+  site_instance_->RemoveObserver(this);
+
   if (delegate_ && render_frame_created_)
     delegate_->RenderFrameDeleted(this);
 
   bool is_active = IsRFHStateActive(rfh_state_);
 
   // If this RenderFrameHost is swapped out, it already decremented the active
   // frame count of the SiteInstance it belongs to.
   if (is_active)
     GetSiteInstance()->DecrementActiveFrameCount();
 
@@ skipping 235 matching lines @@
     IPC_MESSAGE_HANDLER(FrameHostMsg_Detach, OnDetach)
     IPC_MESSAGE_HANDLER(FrameHostMsg_FrameFocused, OnFrameFocused)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidStartProvisionalLoad,
                         OnDidStartProvisionalLoad)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidFailProvisionalLoadWithError,
                         OnDidFailProvisionalLoadWithError)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidFailLoadWithError,
                         OnDidFailLoadWithError)
     IPC_MESSAGE_HANDLER_GENERIC(FrameHostMsg_DidCommitProvisionalLoad,
                                 OnDidCommitProvisionalLoad(msg))
-    IPC_MESSAGE_HANDLER(FrameHostMsg_DidDropNavigation, OnDidDropNavigation)
     IPC_MESSAGE_HANDLER(FrameHostMsg_UpdateState, OnUpdateState)
     IPC_MESSAGE_HANDLER(FrameHostMsg_OpenURL, OnOpenURL)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DocumentOnLoadCompleted,
                         OnDocumentOnLoadCompleted)
     IPC_MESSAGE_HANDLER(FrameHostMsg_BeforeUnload_ACK, OnBeforeUnloadACK)
     IPC_MESSAGE_HANDLER(FrameHostMsg_SwapOut_ACK, OnSwapOutACK)
     IPC_MESSAGE_HANDLER(FrameHostMsg_ContextMenu, OnContextMenu)
     IPC_MESSAGE_HANDLER(FrameHostMsg_JavaScriptExecuteResponse,
                         OnJavaScriptExecuteResponse)
     IPC_MESSAGE_HANDLER(FrameHostMsg_VisualStateResponse,
@@ skipping 152 matching lines @@
 
 gfx::NativeViewAccessible
     RenderFrameHostImpl::AccessibilityGetNativeViewAccessible() {
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetWidget()->GetView());
   if (view)
     return view->AccessibilityGetNativeViewAccessible();
   return NULL;
 }
 
+void RenderFrameHostImpl::RenderProcessGone(SiteInstanceImpl* site_instance) {
+  DCHECK(site_instance == site_instance_.get());
+
+  // The render process is gone, this frame can no longer be loading.
+  ResetLoadingState();
+}
+
 bool RenderFrameHostImpl::CreateRenderFrame(int proxy_routing_id,
                                             int opener_routing_id,
                                             int parent_routing_id,
                                             int previous_sibling_routing_id) {
   TRACE_EVENT0("navigation", "RenderFrameHostImpl::CreateRenderFrame");
   DCHECK(!IsRenderFrameLive()) << "Creating frame twice";
 
   // The process may (if we're sharing a process with another host that already
   // initialized it) or may not (we have our own process or the old process
   // crashed) have been initialized. Calling Init multiple times will be
@@ skipping 332 matching lines @@
       !validated_params.was_within_same_page) {
     RenderWidgetHostImpl::From(GetView()->GetRenderWidgetHost())
         ->StartNewContentRenderingTimeout();
   }
 
   // PlzNavigate
   if (IsBrowserSideNavigationEnabled())
     pending_commit_ = false;
 }
 
-void RenderFrameHostImpl::OnDidDropNavigation() {
-  // At the end of Navigate(), the FrameTreeNode's DidStartLoading is called to
-  // force the spinner to start, even if the renderer didn't yet begin the load.
-  // If it turns out that the renderer dropped the navigation, the spinner needs
-  // to be turned off.
-  frame_tree_node_->DidStopLoading();
-  navigation_handle_.reset();
-}
-
 void RenderFrameHostImpl::OnUpdateState(const PageState& state) {
   // TODO(creis): Verify the state's ISN matches the last committed FNE.
 
   // Without this check, the renderer can trick the browser into using
   // filenames it can't access in a future session restore.
   // TODO(creis): Move CanAccessFilesOfPageState to RenderFrameHostImpl.
   if (!render_view_host_->CanAccessFilesOfPageState(state)) {
     bad_message::ReceivedBadMessage(
         GetProcess(), bad_message::RFH_CAN_ACCESS_FILES_OF_PAGE_STATE);
     return;
@@ skipping 245 matching lines @@
       frame_tree_node_->render_manager()->IsPendingDeletion(this)) {
     render_view_host_->set_is_active(false);
     render_view_host_->set_is_swapped_out(true);
   }
 
   if (frame_tree_node_->render_manager()->DeleteFromPendingList(this)) {
     // We are now deleted.
     return;
   }
 
+  ResetLoadingState();
+
   // If this RFH wasn't pending deletion, then it is now swapped out.
   SetState(RenderFrameHostImpl::STATE_SWAPPED_OUT);
 }
 
 void RenderFrameHostImpl::OnContextMenu(const ContextMenuParams& params) {
   // Validate the URLs in |params|.  If the renderer can't request the URLs
   // directly, don't show them in the context menu.
   ContextMenuParams validated_params(params);
   RenderProcessHost* process = GetProcess();
 
@@ skipping 365 matching lines @@
     delegate_->EnterFullscreenMode(GetLastCommittedURL().GetOrigin());
   else
     delegate_->ExitFullscreenMode(/* will_cause_resize */ true);
 
   // The previous call might change the fullscreen state. We need to make sure
   // the renderer is aware of that, which is done via the resize message.
   render_view_host_->GetWidget()->WasResized();
 }
 
 void RenderFrameHostImpl::OnDidStartLoading(bool to_different_document) {
-  // Any main frame load to a new document should reset the load since it will
-  // replace the current page and any frames.
-  if (to_different_document && !GetParent())
-    is_loading_ = false;
+  bool was_previously_loading = frame_tree_node_->frame_tree()->IsLoading();
+  is_loading_ = true;
 
-  // This method should never be called when the frame is loading.
-  // Unfortunately, it can happen if a history navigation happens during a
-  // BeforeUnload or Unload event.
-  // TODO(fdegans): Change this to a DCHECK after LoadEventProgress has been
-  // refactored in Blink. See crbug.com/466089
-  if (is_loading_) {
-    LOG(WARNING) << "OnDidStartLoading was called twice.";
-    return;
+  // Only inform the FrameTreeNode of a change in load state if the load state
+  // of this RenderFrameHost is being tracked.
+  bool is_load_tracked =
+      this == frame_tree_node_->render_manager()->current_frame_host() ||
+      (IsBrowserSideNavigationEnabled()
+           ? this ==
+                 frame_tree_node_->render_manager()->speculative_frame_host()
+           : this == frame_tree_node_->render_manager()->pending_frame_host());

[nasko, 2016/02/11 16:33:45]

This seems rather hard to read. Can we find a simpler way to write it?
Alternatively, why can't we consult the RFH state? If I got it right, the issue
was receiving IPC while in pending swap/deletion state, right?

[clamy, 2016/02/12 12:25:42]

Done.

+  if (is_load_tracked) {
+    frame_tree_node_->DidStartLoading(to_different_document,
+                                      was_previously_loading);
   }
-
-  frame_tree_node_->DidStartLoading(to_different_document);
-  is_loading_ = true;
 }
 
 void RenderFrameHostImpl::OnDidStopLoading() {
   // This method should never be called when the frame is not loading.
   // Unfortunately, it can happen if a history navigation happens during a
   // BeforeUnload or Unload event.
   // TODO(fdegans): Change this to a DCHECK after LoadEventProgress has been
   // refactored in Blink. See crbug.com/466089
   if (!is_loading_) {
     LOG(WARNING) << "OnDidStopLoading was called twice.";
     return;
   }
 
   is_loading_ = false;
-  frame_tree_node_->DidStopLoading();
   navigation_handle_.reset();
+
+  // Only inform the FrameTreeNode of a change in load state if the load state
+  // of this RenderFrameHost is being tracked.
+  bool is_load_tracked =
+      this == frame_tree_node_->render_manager()->current_frame_host() ||
+      (IsBrowserSideNavigationEnabled()
+           ? this ==
+                 frame_tree_node_->render_manager()->speculative_frame_host()
+           : this == frame_tree_node_->render_manager()->pending_frame_host());
+  if (is_load_tracked)
+    frame_tree_node_->DidStopLoading();
 }
 
 void RenderFrameHostImpl::OnDidChangeLoadProgress(double load_progress) {
   frame_tree_node_->DidChangeLoadProgress(load_progress);
 }
 
 void RenderFrameHostImpl::OnSerializeAsMHTMLResponse(
     int job_id,
     bool success,
     const std::set<std::string>& digests_of_uris_of_serialized_resources) {
@@ skipping 159 matching lines @@
   // throbber needs to be kept in sync with what's happening in the UI. For
   // example, the throbber will start immediately when the user navigates even
   // if the renderer is delayed. There is also an issue with the throbber
   // starting because the WebUI (which controls whether the favicon is
   // displayed) happens synchronously. If the start loading messages was
   // asynchronous, then the default favicon would flash in.
   //
   // Blink doesn't send throb notifications for JavaScript URLs, so it is not
   // done here either.
   if (!common_params.url.SchemeIs(url::kJavaScriptScheme))
-    frame_tree_node_->DidStartLoading(true);
+    OnDidStartLoading(true);
 }
 
 void RenderFrameHostImpl::NavigateToInterstitialURL(const GURL& data_url) {
   DCHECK(data_url.SchemeIs(url::kDataScheme));
   CommonNavigationParams common_params(
       data_url, Referrer(), ui::PAGE_TRANSITION_LINK,
       FrameMsg_Navigate_Type::NORMAL, false, false, base::TimeTicks::Now(),
       FrameMsg_UILoadMetricsReportType::NO_REPORT, GURL(), GURL(), LOFI_OFF,
       base::TimeTicks::Now());
   if (IsBrowserSideNavigationEnabled()) {
@@ skipping 318 matching lines @@
 
 const image_downloader::ImageDownloaderPtr&
 RenderFrameHostImpl::GetMojoImageDownloader() {
   if (!mojo_image_downloader_.get() && GetServiceRegistry()) {
     GetServiceRegistry()->ConnectToRemoteService(
         mojo::GetProxy(&mojo_image_downloader_));
   }
   return mojo_image_downloader_;
 }
 
+void RenderFrameHostImpl::ResetLoadingState() {
+  if (is_loading()) {
+    // When pending deletion, just set the loading state to not loading.
+    // Otherwise, OnDidStopLoading will take care of that, as well as sending
+    // notification to the FrameTreeNode about the change in loading state.
+    if (rfh_state_ != STATE_DEFAULT)
+      is_loading_ = false;
+    else
+      OnDidStopLoading();
+  }
+}
+
 bool RenderFrameHostImpl::IsSameSiteInstance(
     RenderFrameHostImpl* other_render_frame_host) {
   // As a sanity check, make sure the frame belongs to the same BrowserContext.
   CHECK_EQ(GetSiteInstance()->GetBrowserContext(),
            other_render_frame_host->GetSiteInstance()->GetBrowserContext());
   return GetSiteInstance() == other_render_frame_host->GetSiteInstance();
 }
 
 void RenderFrameHostImpl::SetAccessibilityMode(AccessibilityMode mode) {
   Send(new FrameMsg_SetAccessibilityMode(routing_id_, mode));
@@ skipping 300 matching lines @@
   *dst = src;
 
   if (src.routing_id != -1)
     dst->tree_id = RoutingIDToAXTreeID(src.routing_id);
 
   if (src.parent_routing_id != -1)
     dst->parent_tree_id = RoutingIDToAXTreeID(src.parent_routing_id);
 }
 
 }  // namespace content