Refactor the loading tracking logic in WebContentsImpl.

content/browser/frame_host/frame_tree_node.h

 // 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.
 
 #ifndef CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_H_
 #define CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_H_
 
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 94 matching lines @@
     return replication_state_;
   }
 
   void set_is_loading(bool is_loading) {
     is_loading_ = is_loading;
   }
   bool is_loading() const {
     return is_loading_;
   }
 
+  void set_loading_progress(double loading_progress) {
+    loading_progress_ = loading_progress;
+  }
+  double get_loading_progress() const { return loading_progress_; }

[clamy, 2015/02/13 12:13:43]

s/get_loading_progress()/loading_progress()

nit: Add an empty line above.

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

Done.

[clamy, 2015/02/16 14:20:59]

I don't see the modification to the accessor name in the new patch set (see the
style guide here:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Names)

[Fabrice (no longer in Chrome), 2015/02/20 12:58:58]

Done.

+
   RenderFrameHostImpl* current_frame_host() const {
     return render_manager_.current_frame_host();
   }
 
   bool IsDescendantOf(FrameTreeNode* other) const;
 
  private:
   void set_parent(FrameTreeNode* parent) { parent_ = parent; }
 
   // The next available browser-global FrameTreeNode ID.
@@ skipping 33 matching lines @@
   // proxies for this frame.
   FrameReplicationState replication_state_;
 
   // Boolean value indicating whether the frame is in the process of loading
   // a document or not. In cross-process transfer navigation the DidStartLoading
   // message is received from both existing RenderFrame and from the pending
   // RenderFrame. However, there will be only one DidStopLoading message sent by
   // the pending-which-becomes-current RenderFrame. Since both renderers belong
   // to the FrameTreeNode, it is better to ask it about the loading status than
   // RenderFrameHost or using a counter to balance the events out.
   bool is_loading_;

[carlosk, 2015/02/13 15:05:41]

Does the tracking of the loading progress happens in sync with the loading flag
being set? If so, is it possible to eliminate the |bool| value and track loading
only with the |double| (for instance is_loading() could be implemented as
|return loading_progress_ >= 1.0d|)? This would avoid inconsistent states
between them.

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

This sounded good until I actually tried it and it caused crashes and what not.
Another issue is that it is possible that due to rounding errors, we set an
intermediate load progress to 1.0 while the content is not actually done
loading.

 
+  // Double value representing the frame's completion progress (from 0 to 1).
+  double loading_progress_;
+
   DISALLOW_COPY_AND_ASSIGN(FrameTreeNode);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_H_