Ensure that the pending entry is cleared when a navigation commits.

content/browser/web_contents/navigation_controller_impl.cc

 // Copyright (c) 2012 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/web_contents/navigation_controller_impl.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ skipping 890 matching lines @@
   // At this point, we know that the navigation has just completed, so
   // record the time.
   //
   // TODO(akalin): Use "sane time" as described in
   // http://www.chromium.org/developers/design-documents/sane-time .
   base::Time timestamp =
       time_smoother_.GetSmoothedTime(get_timestamp_callback_.Run());
   DVLOG(1) << "Navigation finished at (smoothed) timestamp "
            << timestamp.ToInternalValue();
 
+  // We should not have a pending entry anymore.  Clear it again in case any
+  // error cases above forgot to do so.
+  DiscardNonCommittedEntriesInternal();
+
   // All committed entries should have nonempty content state so WebKit doesn't
   // get confused when we go back to them (see the function for details).
   DCHECK(!params.content_state.empty());
   NavigationEntryImpl* active_entry =
-      NavigationEntryImpl::FromNavigationEntry(GetActiveEntry());
+      NavigationEntryImpl::FromNavigationEntry(GetLastCommittedEntry());

[Charlie Reis, 2013/02/22 21:30:23]

This was the source of the bug, since we were using the pending entry before. 
Now it's just to be safe, given that the pending entry is discarded above.

   active_entry->SetTimestamp(timestamp);
   active_entry->SetContentState(params.content_state);
   // No longer needed since content state will hold the post data if any.
   active_entry->SetBrowserInitiatedPostData(NULL);
 
   // Once committed, we do not need to track if the entry was initiated by
   // the renderer.
   active_entry->set_is_renderer_initiated(false);
 
   // The active entry's SiteInstance should match our SiteInstance.
-  DCHECK(active_entry->site_instance() == web_contents_->GetSiteInstance());
+  CHECK(active_entry->site_instance() == web_contents_->GetSiteInstance());

[Charlie Reis, 2013/02/22 21:30:23]

This DCHECK would have caught the bug, so I'd like to promote it to a CHECK so
that we have crash dumps to help diagnose it if it happens again.

 
   // Now prep the rest of the details for the notification and broadcast.
   details->entry = active_entry;
   details->is_main_frame =
       PageTransitionIsMainFrame(params.transition);
   details->serialized_security_info = params.security_info;
   details->http_status_code = params.http_status_code;
   NotifyNavigationEntryCommitted(details);
 
   return true;
@@ skipping 285 matching lines @@
   // have to query the entry index again.
   last_committed_entry_index_ =
       GetEntryIndexWithPageID(web_contents_->GetSiteInstance(), params.page_id);
 }
 
 void NavigationControllerImpl::RendererDidNavigateNewSubframe(
     const ViewHostMsg_FrameNavigate_Params& params) {
   if (PageTransitionStripQualifier(params.transition) ==
       PAGE_TRANSITION_AUTO_SUBFRAME) {
     // This is not user-initiated. Ignore.
+    DiscardNonCommittedEntriesInternal();
     return;
   }
 
   // Manual subframe navigations just get the current entry cloned so the user
   // can go back or forward to it. The actual subframe information will be
   // stored in the page state for each of those entries. This happens out of
   // band with the actual navigations.
   DCHECK(GetLastCommittedEntry()) << "ClassifyNavigation should guarantee "
                                   << "that a last committed entry exists.";
   NavigationEntryImpl* new_entry = new NavigationEntryImpl(
@@ skipping 16 matching lines @@
       params.page_id);
   if (entry_index < 0 ||
       entry_index >= static_cast<int>(entries_.size())) {
     NOTREACHED();
     return false;
   }
 
   // Update the current navigation entry in case we're going back/forward.
   if (entry_index != last_committed_entry_index_) {
     last_committed_entry_index_ = entry_index;
+    DiscardNonCommittedEntriesInternal();
     return true;
   }
+
+  // We do not need to discard the pending entry in this case, since we will
+  // not generate commit notifications for this auto-subframe navigation.
   return false;
 }
 
 int NavigationControllerImpl::GetIndexOfEntry(
     const NavigationEntryImpl* entry) const {
   const NavigationEntries::const_iterator i(std::find(
       entries_.begin(),
       entries_.end(),
       entry));
   return (i == entries_.end()) ? -1 : static_cast<int>(i - entries_.begin());
@@ skipping 501 matching lines @@
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 void NavigationControllerImpl::SetTakeScreenshotCallbackForTest(
     const base::Callback<void(RenderViewHost*)>& take_screenshot_callback) {
   take_screenshot_callback_ = take_screenshot_callback;
 }
 
 }  // namespace content