Move load progress tracking logic to the frame tree.

content/browser/web_contents/web_contents_impl.cc

 // Copyright (c) 2012 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/web_contents/web_contents_impl.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
@@ skipping 147 matching lines @@
 }
 
 // Helper function for retrieving all the sites in a frame tree.
 bool CollectSites(BrowserContext* context,
                   std::set<GURL>* sites,
                   FrameTreeNode* node) {
   sites->insert(SiteInstance::GetSiteForURL(context, node->current_url()));
   return true;
 }
 
-// Helper function used with FrameTree::ForEach() for retrieving the total
-// loading progress and number of frames in a frame tree.
-bool CollectLoadProgress(double* progress,
-                         int* frame_count,
-                         FrameTreeNode* node) {
-  // Ignore the current frame if it has not started loading.
-  double frame_progress = node->GetLoadingProgress();
-  if (frame_progress == RenderFrameHostImpl::kLoadingProgressNotStarted)
-    return true;
-
-  // Collect progress.
-  *progress += node->GetLoadingProgress();
-  (*frame_count)++;
-  return true;
-}
-
-// Helper function used with FrameTree::ForEach() to check if at least one of
-// the nodes is loading.
-bool IsNodeLoading(bool* is_loading, FrameTreeNode* node) {
-  if (node->IsLoading()) {
-    // There is at least one node loading, so abort traversal.
-    *is_loading = true;
-    return false;
-  }
-  return true;
-}
-
-// Helper function used with FrameTree::ForEach() to reset the load progress.
-bool ResetLoadProgress(FrameTreeNode* node) {
-  node->current_frame_host()->set_loading_progress(
-      RenderFrameHostImpl::kLoadingProgressNotStarted);
-  return true;
-}
-
 bool ForEachFrameInternal(
     const base::Callback<void(RenderFrameHost*)>& on_frame,
     FrameTreeNode* node) {
   on_frame.Run(node->current_frame_host());
   return true;
 }
 
 bool ForEachPendingFrameInternal(
     const base::Callback<void(RenderFrameHost*)>& on_frame,
     FrameTreeNode* node) {
@@ skipping 41 matching lines @@
 // termination is not allowed for the current RenderFrameHost of
 // |frame_tree_node|. Used with FrameTree::ForEach.
 bool SuddenTerminationAllowed(bool* sudden_termination_allowed,
                               FrameTreeNode* frame_tree_node) {
   if (frame_tree_node->current_frame_host()->SuddenTerminationAllowed())
     return true;
   *sudden_termination_allowed = false;
   return false;
 }
 
-// Returns true if at least one of the nodes in the |frame_tree| is loading.
-bool IsFrameTreeLoading(FrameTree& frame_tree) {
-  bool is_loading = false;
-  frame_tree.ForEach(base::Bind(&IsNodeLoading, &is_loading));
-  return is_loading;
-}
 
 }  // namespace
 
 WebContents* WebContents::Create(const WebContents::CreateParams& params) {
   return WebContentsImpl::CreateWithOpener(
       params, static_cast<WebContentsImpl*>(params.opener));
 }
 
 WebContents* WebContents::CreateWithSessionStorage(
     const WebContents::CreateParams& params,
@@ skipping 2676 matching lines @@
   // 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 (rfh->is_loading()) {
     LOG(WARNING) << "OnDidStartLoading was called twice.";
     return;
   }
 
-  if (!IsFrameTreeLoading(frame_tree_))
+  if (!frame_tree_.IsLoading())
     DidStartLoading(rfh, to_different_document);
 
   rfh->set_is_loading(true);
-  rfh->set_loading_progress(RenderFrameHostImpl::kLoadingProgressMinimum);
+
+  // The loading progress is only reset when the current RFH starts navigating.
+  FrameTreeNode* ftn = rfh->frame_tree_node();
+  if (ftn->render_manager()->current_frame_host() == rfh)

[nasko, 2015/03/24 17:26:11]

I think this check might be problematic. In browser-initiated navigation,
DidStartLoading is issued browser-side, which will set the loading state to
true, but will not reset the loading progress if the navigation is cross-site
and we have a pending RFH. Later on, the pending RFH will send a
DidStartLoading, but the RFH will already be in loading state, so this method
will return early and the progress value won't be reset.

Since this CL is just moving code around, let's not make any functional changes.

[Fabrice (no longer in Chrome), 2015/03/25 17:35:01]

Following our offline conversation, I removed the checks in this file.

+    ftn->set_loading_progress(FrameTreeNode::kLoadingProgressMinimum);
 
   // Notify the RenderFrameHostManager of the event.
-  rfh->frame_tree_node()->render_manager()->OnDidStartLoading();
+  ftn->render_manager()->OnDidStartLoading();
 
   SendLoadProgressChanged();
 }
 
 void WebContentsImpl::OnDidStopLoading() {
   // TODO(erikchen): Remove ScopedTracker below once crbug.com/465796 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile1(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "465796 WebContentsImpl::OnDidStopLoading::Start"));
 
   if (!HasValidFrameSource())
     return;
 
   RenderFrameHostImpl* rfh =
       static_cast<RenderFrameHostImpl*>(render_frame_message_source_);
 
   // 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 (!rfh->is_loading()) {
     LOG(WARNING) << "OnDidStopLoading was called twice.";
     return;
   }
 
   rfh->set_is_loading(false);
-  rfh->set_loading_progress(RenderFrameHostImpl::kLoadingProgressDone);
+
+  FrameTreeNode* ftn = rfh->frame_tree_node();
+  if (ftn->render_manager()->current_frame_host() == rfh)
+    ftn->set_loading_progress(FrameTreeNode::kLoadingProgressDone);
 
   // TODO(erikchen): Remove ScopedTracker below once crbug.com/465796 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile2(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "465796 "
           "WebContentsImpl::OnDidStopLoading::SendLoadProgressChanged"));
 
   // Update progress based on this frame's completion.
   SendLoadProgressChanged();
 
   // Then clean-up the states.
   if (loading_total_progress_ == 1.0)
     ResetLoadProgressState();
 
   // TODO(erikchen): Remove ScopedTracker below once crbug.com/465796 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile3(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "465796 WebContentsImpl::OnDidStopLoading::NotifyRenderManager"));
   // Notify the RenderFrameHostManager of the event.
-  rfh->frame_tree_node()->render_manager()->OnDidStopLoading();
+  ftn->render_manager()->OnDidStopLoading();
 
-  if (!IsFrameTreeLoading(frame_tree_)) {
+  if (!frame_tree_.IsLoading()) {
     // TODO(erikchen): Remove ScopedTracker below once crbug.com/465796 is
     // fixed.
     tracked_objects::ScopedTracker tracking_profile4(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "465796 WebContentsImpl::OnDidStopLoading::WCIDidStopLoading"));
     DidStopLoading();
   }
 
   // TODO(erikchen): Remove ScopedTracker below once crbug.com/465796 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile4(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "465796 WebContentsImpl::OnDidStopLoading::End"));
 }
 
 void WebContentsImpl::OnDidChangeLoadProgress(double load_progress) {
   if (!HasValidFrameSource())
     return;
 
   RenderFrameHostImpl* rfh =
       static_cast<RenderFrameHostImpl*>(render_frame_message_source_);
+  FrameTreeNode* ftn = rfh->frame_tree_node();
 
-  rfh->set_loading_progress(load_progress);
+  if (ftn->render_manager()->current_frame_host() != rfh)
+    return;
+
+  ftn->set_loading_progress(load_progress);
 
   // We notify progress change immediately for the first and last updates.
   // Also, since the message loop may be pretty busy when a page is loaded, it
   // might not execute a posted task in a timely manner so we make sure to
   // immediately send progress report if enough time has passed.
   base::TimeDelta min_delay =
       base::TimeDelta::FromMilliseconds(kMinimumDelayBetweenLoadingUpdatesMS);
   if (load_progress == 1.0 || loading_last_progress_update_.is_null() ||
       base::TimeTicks::Now() - loading_last_progress_update_ > min_delay) {
     // If there is a pending task to send progress, it is now obsolete.
@@ skipping 453 matching lines @@
   NotificationService::current()->Notify(
       NOTIFICATION_WEB_CONTENTS_TITLE_UPDATED,
       Source<WebContents>(this),
       Details<std::pair<NavigationEntry*, bool> >(&details));
 
   return true;
 }
 
 void WebContentsImpl::SendLoadProgressChanged() {
   loading_last_progress_update_ = base::TimeTicks::Now();
-  double progress = 0.0;
-  int frame_count = 0;
+  double progress = frame_tree_.GetLoadProgress();
 
-  frame_tree_.ForEach(
-      base::Bind(&CollectLoadProgress, &progress, &frame_count));
-  if (frame_count != 0)
-    progress /= frame_count;
   DCHECK_LE(progress, 1.0);
 
   if (progress <= loading_total_progress_)
     return;
   loading_total_progress_ = progress;
 
   if (delegate_)
     delegate_->LoadProgressChanged(this, progress);
 }
 
 void WebContentsImpl::ResetLoadProgressState() {
-  frame_tree_.ForEach(base::Bind(&ResetLoadProgress));
+  frame_tree_.ResetLoadProgress();
   loading_total_progress_ = 0.0;
   loading_weak_factory_.InvalidateWeakPtrs();
   loading_last_progress_update_ = base::TimeTicks();
 }
 
 void WebContentsImpl::NotifyViewSwapped(RenderViewHost* old_host,
                                         RenderViewHost* new_host) {
   // After sending out a swap notification, we need to send a disconnect
   // notification so that clients that pick up a pointer to |this| can NULL the
   // pointer.  See Bug 1230284.
@@ skipping 1097 matching lines @@
   node->render_manager()->ResumeResponseDeferredAtStart();
 }
 
 void WebContentsImpl::SetForceDisableOverscrollContent(bool force_disable) {
   force_disable_overscroll_content_ = force_disable;
   if (view_)
     view_->SetOverscrollControllerEnabled(CanOverscrollContent());
 }
 
 }  // namespace content