Refactor the loading tracking logic in WebContentsImpl.

content/browser/frame_host/frame_tree_node.cc

Index: content/browser/frame_host/frame_tree_node.cc
diff --git a/content/browser/frame_host/frame_tree_node.cc b/content/browser/frame_host/frame_tree_node.cc
index 845aecea62afc5945311f283f029c8d8b6504682..48b25ae460b0cbd3ea219145c1cd3261a8244c0a 100644
--- a/content/browser/frame_host/frame_tree_node.cc
+++ b/content/browser/frame_host/frame_tree_node.cc
@@ -34,8 +34,7 @@ FrameTreeNode::FrameTreeNode(FrameTree* frame_tree,
                       manager_delegate),
       frame_tree_node_id_(next_frame_tree_node_id_++),
       parent_(NULL),
-      replication_state_(name),
-      is_loading_(false) {
+      replication_state_(name) {
 }
 
 FrameTreeNode::~FrameTreeNode() {
@@ -115,4 +114,31 @@ bool FrameTreeNode::IsDescendantOf(FrameTreeNode* other) const {
   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);
+  if (pending_frame_host != nullptr)
+    return current_frame_host->is_loading() && pending_frame_host->is_loading();

[Fabrice (no longer in Chrome), 2015/02/23 20:48:45]

... s/&&/||/
I find it worrisome that none of our tests seem too bothered by it.

[Fabrice (no longer in Chrome), 2015/02/24 13:14:47]

Done.
I will add a test for this case.

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

[Charlie Reis, 2015/02/24 04:55:00]

This is not correct.  Why would the overall load progress be the average of two
different frames?  For example, we make an effort to ensure that load progress
never goes backwards, and this would fail that if a pending RFH were introduced
in the middle of a load (e.g., for a redirect).

[Fabrice (no longer in Chrome), 2015/02/24 13:14:47]

The loading progress can in fact go backwards but it is simply ignored. The way
we track this has always been an estimate anyway so I am not sure why it would
be a problem now.

[Charlie Reis, 2015/02/25 00:07:20]

To be clear: it's important for UX that the load progress never goes backwards,
but I agree that WebContentsImpl::SendLoadProgressChanged() enforces that.

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