When simulating a navigation via TestRenderViewHost, first start a provisional load before committin

chrome/browser/extensions/api/web_navigation/web_navigation_api.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.
 
 // Implements the Chrome Extensions WebNavigation API.
 
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api.h"
 
 #include "base/lazy_instance.h"
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api_constants.h"
@@ skipping 218 matching lines @@
       pending_web_contents_.erase(i++);
     else
       ++i;
   }
 }
 
 // WebNavigationTabObserver ------------------------------------------
 
 WebNavigationTabObserver::WebNavigationTabObserver(
     content::WebContents* web_contents)
-    : WebContentsObserver(web_contents) {
+    : WebContentsObserver(web_contents),
+      render_view_host_(NULL),
+      pending_render_view_host_(NULL) {
   g_tab_observer.Get().insert(TabObserverMap::value_type(web_contents, this));
   registrar_.Add(this,
                  content::NOTIFICATION_RESOURCE_RECEIVED_REDIRECT,
                  content::Source<content::WebContents>(web_contents));
 }
 
 WebNavigationTabObserver::~WebNavigationTabObserver() {}
 
 // static
 WebNavigationTabObserver* WebNavigationTabObserver::Get(
@@ skipping 26 matching lines @@
       NOTREACHED();
   }
 }
 
 void WebNavigationTabObserver::DidStartProvisionalLoadForFrame(
     int64 frame_id,
     bool is_main_frame,
     const GURL& validated_url,
     bool is_error_page,
     content::RenderViewHost* render_view_host) {
+  if (render_view_host == pending_render_view_host_) {
+    DCHECK(is_main_frame);
+    // Suppress incorrect duplicate navigation signals triggered by the
+    // TransfernavigationResourceThrottle. http://crbug.com/137219
+    return;
+  }
+
+  if (render_view_host_ == NULL) {

[Charlie Reis, 2012/07/18 17:21:18]

nit: if (!render_view_host_) {

+    render_view_host_ = render_view_host;
+  } else if (render_view_host != render_view_host_) {
+    DCHECK(is_main_frame);
+    DCHECK(!pending_render_view_host_);

[Charlie Reis, 2012/07/18 17:21:18]

Are you sure DidCommit or DidFail is always going to be called before DidStart,
when rapid back/forward cross-process navigations are in progress?  I'd love for
this to be true, but I'm skeptical because it relies on messages from the
renderers, allowing for races.

For example, I wouldn't be surprised to see this check fail if you tried the
following:
1) Navigate to several 5 or so different sites in the same tab via the omnibox,
causing process swaps.
2) Keep at least one of the renderer processes alive by opening another tab in
the same process.  (Might need a targeted link or window.open.)
3) Hold the Back keyboard shortcut down to race past entries, then the Forward
keyboard shortcut.  Or generally try to go back/forward fast in a random manner.

I've found this tends to expose lots of races we might not have expected.

[jochen (gone - plz use gerrit), 2012/07/18 18:03:40]

No, I'm not sure.

However, wouldn't that mean that a web contents can have almost arbitrarily many
non-swapped out RVH attached?

[Charlie Reis, 2012/07/18 18:51:34]

Yes, I don't see why messages couldn't come in from any RVH in the history of
the tab.  Remember, messages from the renderer can be arbitrarily delayed based
on how heavily loaded the client is.  Is there anything that would prevent this?

+    pending_render_view_host_ = render_view_host;
+  }
+
   // Ignore navigations of sub frames, if the main frame isn't committed yet.
   // This might happen if a sub frame triggers a navigation for both the main
   // frame and itself. Since the sub frame is about to be deleted, and there's
   // no way for an extension to tell that these navigations belong to an old
   // frame, we just suppress the events here.
   int64 main_frame_id = navigation_state_.GetMainFrameID();
   if (!is_main_frame &&
       !navigation_state_.GetNavigationCommitted(main_frame_id)) {
     return;
   }
 
   navigation_state_.TrackFrame(frame_id,
                                validated_url,
                                is_main_frame,
                                is_error_page);
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
   helpers::DispatchOnBeforeNavigate(
       web_contents(), frame_id, is_main_frame, validated_url);
 }
 
 void WebNavigationTabObserver::DidCommitProvisionalLoadForFrame(
     int64 frame_id,
     bool is_main_frame,
     const GURL& url,
     content::PageTransition transition_type,
     content::RenderViewHost* render_view_host) {
+  DCHECK(render_view_host == render_view_host_ ||
+         (render_view_host == pending_render_view_host_ && is_main_frame));
+  render_view_host_ = render_view_host;
+  pending_render_view_host_ = NULL;
+
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
 
   bool is_reference_fragment_navigation =
       IsReferenceFragmentNavigation(frame_id, url);
   bool is_history_navigation =
       navigation_state_.GetNavigationCommitted(frame_id);
 
   // Update the URL as it might have changed.
   navigation_state_.UpdateFrame(frame_id, url);
@@ skipping 35 matching lines @@
   }
 }
 
 void WebNavigationTabObserver::DidFailProvisionalLoad(
     int64 frame_id,
     bool is_main_frame,
     const GURL& validated_url,
     int error_code,
     const string16& error_description,
     content::RenderViewHost* render_view_host) {
+  DCHECK(render_view_host == render_view_host_ ||
+         (render_view_host == pending_render_view_host_ && is_main_frame));
+  if (render_view_host == pending_render_view_host_)
+    pending_render_view_host_ = NULL;
+
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
   navigation_state_.SetErrorOccurredInFrame(frame_id);
   helpers::DispatchOnErrorOccurred(
       web_contents(), validated_url, frame_id, is_main_frame, error_code);
 }
 
 void WebNavigationTabObserver::DocumentLoadedInFrame(
     int64 frame_id) {
   if (!navigation_state_.CanSendEvents(frame_id))
@@ skipping 174 matching lines @@
     frame->frame_id = helpers::GetFrameId(
         navigation_state.IsMainFrame(frame_id), frame_id);
     frame->error_occurred = navigation_state.GetErrorOccurredInFrame(frame_id);
     result_list.push_back(frame);
   }
   results_ = GetAllFrames::Results::Create(result_list);
   return true;
 }
 
 }  // namespace extensions