Refactor the loading tracking logic in WebContentsImpl.

content/browser/frame_host/frame_tree_node.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/frame_tree_node.h"
 
 #include <queue>
 
 #include "base/command_line.h"
 #include "base/stl_util.h"
@@ skipping 16 matching lines @@
                              const std::string& name)
     : frame_tree_(frame_tree),
       navigator_(navigator),
       render_manager_(this,
                       render_frame_delegate,
                       render_view_delegate,
                       render_widget_delegate,
                       manager_delegate),
       frame_tree_node_id_(next_frame_tree_node_id_++),
       parent_(NULL),
-      replication_state_(name),
-      is_loading_(false) {
+      replication_state_(name) {
 }
 
 FrameTreeNode::~FrameTreeNode() {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableBrowserSideNavigation)) {
     navigator_->CancelNavigation(this);
   }
 }
 
 bool FrameTreeNode::IsMainFrame() const {
@@ skipping 59 matching lines @@
     return false;
 
   for (FrameTreeNode* node = parent(); node; node = node->parent()) {
     if (node == other)
       return true;
   }
 
   return false;
 }
 
+bool FrameTreeNode::IsLoading() const {
+  RenderFrameHostImpl* current_frame_host =
+      render_manager_.current_frame_host();
+  RenderFrameHostImpl* pending_frame_host =
+      render_manager_.pending_frame_host();
+
+  DCHECK(current_frame_host != nullptr);

[Charlie Reis, 2015/03/03 06:32:22]

nit: Drop the "!= nullptr"
The check is equivalent and easier to read.

[Fabrice (no longer in Chrome), 2015/03/03 13:12:16]

Done.

+  // TODO(fdegans): Change the implementation logic for PlzNavigate once
+  // DidStartLoading and DidStopLoading are properly called.
+  if (pending_frame_host != nullptr)
+    return current_frame_host->is_loading() || pending_frame_host->is_loading();
+  else

[Charlie Reis, 2015/03/03 06:32:22]

You don't need this else, since the previous branch returns.

Actually, we can clean this up a bit further:

if (pending_frame_host && pending_frame_host->is_loading())
  return true;

return current_frame_host->is_loading();

[Fabrice (no longer in Chrome), 2015/03/03 13:12:16]

Done.
I like leaving the else because it makes for easier readability imo, but your
way looks better.

+    return current_frame_host->is_loading();
+}
+
+double FrameTreeNode::GetLoadingProgress() const {
+  RenderFrameHostImpl* current_frame_host =
+      render_manager_.current_frame_host();
+
+  DCHECK(current_frame_host != nullptr);

[Charlie Reis, 2015/03/03 06:32:22]

nit: Drop the "!= nullptr"

[Fabrice (no longer in Chrome), 2015/03/03 13:12:16]

Done.

+  return current_frame_host->loading_progress();
+}
+
 }  // namespace content