Solidify Entry discarding logic (NavigationHandle keeps its ID)

content/browser/frame_host/render_frame_host_impl.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/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/containers/hash_tables.h"
@@ skipping 988 matching lines @@
     bad_message::ReceivedBadMessage(
         GetProcess(), bad_message::RFH_CAN_ACCESS_FILES_OF_PAGE_STATE);
     return;
   }
 
   // If the URL does not match what the NavigationHandle expects, treat the
   // commit as a new navigation. This can happen if an ongoing slow
   // same-process navigation is interrupted by a synchronous renderer-initiated
   // navigation.
   if (navigation_handle_ &&
-      navigation_handle_->GetURL() != validated_params.url) {
+      (navigation_handle_->GetURL() != validated_params.url)) {

[Charlie Reis, 2016/02/16 21:55:07]

This change doesn't seem necessary, but I don't feel strongly either way.  (The
main cost is for people searching through git blame.)

[Charlie Harrison, 2016/02/16 22:55:23]

Removed.

     navigation_handle_.reset();
   }
 
   // Synchronous renderer-initiated navigations will send a
   // DidCommitProvisionalLoad IPC without a prior DidStartProvisionalLoad
   // message.
   if (!navigation_handle_) {
+    // There is no pending NavigationEntry in these cases, so pass 0 as the
+    // nav_id.
     navigation_handle_ =
         NavigationHandleImpl::Create(validated_params.url, frame_tree_node_,
                                      true,   // is_synchronous
                                      validated_params.is_srcdoc,
-                                     base::TimeTicks::Now());
+                                     base::TimeTicks::Now(),
+                                     0);

[Charlie Reis, 2016/02/16 21:55:07]

nit: This fits on the previous line.  Call sites don't need to have each
parameter on their own line.  (I'll defer if git cl format did this, but I'd be
surprised since the first line has two parameters as well.)

[Charlie Harrison, 2016/02/16 22:55:22]

Done.

   }
 
   accessibility_reset_count_ = 0;
   frame_tree_node()->navigator()->DidNavigate(this, validated_params);
 
   // For a top-level frame, there are potential security concerns associated
   // with displaying graphics from a previously loaded page after the URL in
   // the omnibar has been changed. It is unappealing to clear the page
   // immediately, but if the renderer is taking a long time to issue any
   // compositor output (possibly because of script deliberately creating this
@@ skipping 1533 matching lines @@
   *dst = src;
 
   if (src.routing_id != -1)
     dst->tree_id = RoutingIDToAXTreeID(src.routing_id);
 
   if (src.parent_routing_id != -1)
     dst->parent_tree_id = RoutingIDToAXTreeID(src.parent_routing_id);
 }
 
 }  // namespace content