Reland #2 Remove the is_loading_ field from WebContentsImpl

content/browser/frame_host/frame_tree.cc

Index: content/browser/frame_host/frame_tree.cc
diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc
index 5e8b2c1d481279fe4c88728d6ccdf4d2455be6e6..2fadc124eb05e5650925273b497680bf19d42146 100644
--- a/content/browser/frame_host/frame_tree.cc
+++ b/content/browser/frame_host/frame_tree.cc
@@ -80,6 +80,44 @@ FrameTree::NodeIterator FrameTree::NodeRange::end() {
 FrameTree::NodeRange::NodeRange(FrameTree* tree, FrameTreeNode* node_to_skip)
     : tree_(tree), node_to_skip_(node_to_skip) {}
 
+FrameTree::ConstNodeIterator::~ConstNodeIterator() {}
+
+FrameTree::ConstNodeIterator& FrameTree::ConstNodeIterator::operator++() {
+  for (size_t i = 0; i < current_node_->child_count(); ++i) {
+    const FrameTreeNode* child = current_node_->child_at(i);
+    queue_.push(child);
+  }
+
+  if (!queue_.empty()) {
+    current_node_ = queue_.front();
+    queue_.pop();
+  } else {
+    current_node_ = nullptr;
+  }
+
+  return *this;
+}
+
+bool FrameTree::ConstNodeIterator::operator==(
+    const ConstNodeIterator& rhs) const {
+  return current_node_ == rhs.current_node_;
+}
+
+FrameTree::ConstNodeIterator::ConstNodeIterator(
+    const FrameTreeNode* starting_node)
+    : current_node_(starting_node) {}
+
+FrameTree::ConstNodeIterator FrameTree::ConstNodeRange::begin() {
+  return ConstNodeIterator(tree_->root());
+}
+
+FrameTree::ConstNodeIterator FrameTree::ConstNodeRange::end() {
+  return ConstNodeIterator(nullptr);
+}
+
+FrameTree::ConstNodeRange::ConstNodeRange(const FrameTree* tree)
+    : tree_(tree) {}
+
 FrameTree::FrameTree(Navigator* navigator,
                      RenderFrameHostDelegate* render_frame_delegate,
                      RenderViewHostDelegate* render_view_delegate,
@@ -156,6 +194,10 @@ FrameTree::NodeRange FrameTree::NodesExcept(FrameTreeNode* node_to_skip) {
   return NodeRange(this, node_to_skip);
 }
 
+FrameTree::ConstNodeRange FrameTree::ConstNodes() const {
+  return ConstNodeRange(this);
+}
+
 bool FrameTree::AddFrame(
     FrameTreeNode* parent,
     int process_id,
@@ -425,8 +467,8 @@ void FrameTree::ResetLoadProgress() {
   load_progress_ = 0.0;
 }
 
-bool FrameTree::IsLoading() {
-  for (FrameTreeNode* node : Nodes()) {
+bool FrameTree::IsLoading() const {
+  for (const FrameTreeNode* node : ConstNodes()) {

[Charlie Reis, 2016/02/17 05:15:28]

This seems like a lot of extra code just to add the const label to IsLoading. 
Does it get us any specific benefits here?  On the surface it doesn't seem
worthwhile.

It's also potentially unrelated to the rest of the CL, and could land separately
if we decide we want to keep it.

[clamy, 2016/02/17 14:42:06]

We cannot use it in WebContentsImpl::IsLoading otherwise, since that function is
const. I would not be making the change otherwise.

[Charlie Reis, 2016/02/17 23:34:26]

Ok, I see it's necessary for that.

(Sigh-- the interesting part is that it's technically illegal for
WebContents::IsLoading to be const in the first place, according to the Content
API ("don't add the const identifier to interfaces").  That was legacy, though,
and it doesn't seem worthwhile to remove the const at this point.  Let's keep
what you have.)

     if (node->IsLoading())
       return true;
   }