When committing a provisional load that didn't change the RVH, still abort pending navigations.

Also, when sending error events for a frame, mark them as failed, so we don't touch them again in the future

BUG=139117
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10834125

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149632 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/web_navigation/frame_navigation_state.cc

@@ -103,10 +103,12 @@ void FrameNavigationState::TrackFrame(FrameID frame_id,
 }
 
 void FrameNavigationState::StopTrackingFramesInRVH(
-    content::RenderViewHost* render_view_host) {
+    content::RenderViewHost* render_view_host,
+    FrameID id_to_skip) {
   for (std::set<FrameID>::iterator frame = frame_ids_.begin();
        frame != frame_ids_.end();) {
-    if (frame->render_view_host != render_view_host) {
+    if (frame->render_view_host != render_view_host ||
+        *frame == id_to_skip) {
       ++frame;
       continue;
     }
@@ -182,8 +184,10 @@ bool FrameNavigationState::GetNavigationCompleted(FrameID frame_id) const {
 void FrameNavigationState::SetNavigationCommitted(FrameID frame_id) {
   DCHECK(frame_state_map_.find(frame_id) != frame_state_map_.end());
   frame_state_map_[frame_id].is_committed = true;
-  if (frame_state_map_[frame_id].is_main_frame)
+  if (frame_state_map_[frame_id].is_main_frame) {
+    DCHECK_EQ(1u, frame_ids_.size());
     main_frame_id_ = frame_id;
+  }
 }
 
 bool FrameNavigationState::GetNavigationCommitted(FrameID frame_id) const {

chrome/browser/extensions/api/web_navigation/frame_navigation_state.h

@@ -57,8 +57,10 @@ class FrameNavigationState {
                   bool is_main_frame,
                   bool is_error_page);
 
-  // Stops tracking all frames for a given RenderViewHost.
-  void StopTrackingFramesInRVH(content::RenderViewHost* render_view_host);
+  // Stops tracking all frames but the frame with |id_to_skip| for a given
+  // RenderViewHost.
+  void StopTrackingFramesInRVH(content::RenderViewHost* render_view_host,
+                               FrameID id_to_skip);
 
   // Update the URL associated with a given frame.
   void UpdateFrame(FrameID frame_id, const GURL& url);

chrome/browser/extensions/api/web_navigation/frame_navigation_state_unittest.cc

@@ -45,7 +45,8 @@ TEST(FrameNavigationStateTest, TrackFrame) {
   EXPECT_EQ(frame_id1, navigation_state.GetMainFrameID());
 
   // Drop the frames.
-  navigation_state.StopTrackingFramesInRVH(fake_rvh);
+  navigation_state.StopTrackingFramesInRVH(fake_rvh,
+                                           FrameNavigationState::FrameID());
   EXPECT_FALSE(navigation_state.CanSendEvents(frame_id1));
   EXPECT_FALSE(navigation_state.IsValidFrame(frame_id1));
   EXPECT_FALSE(navigation_state.CanSendEvents(frame_id2));

chrome/browser/extensions/api/web_navigation/web_navigation_api.cc

@@ -317,7 +317,8 @@ void WebNavigationTabObserver::Observe(
         pending_render_view_host_ = NULL;
       else
         return;
-      SendErrorEvents(web_contents(), render_view_host);
+      SendErrorEvents(
+          web_contents(), render_view_host, FrameNavigationState::FrameID());
       break;
     }
 
@@ -331,8 +332,11 @@ void WebNavigationTabObserver::AboutToNavigateRenderView(
   if (!render_view_host_) {
     render_view_host_ = render_view_host;
   } else if (render_view_host != render_view_host_) {
-    if (pending_render_view_host_)
-      SendErrorEvents(web_contents(), pending_render_view_host_);
+    if (pending_render_view_host_) {
+      SendErrorEvents(web_contents(),
+                      pending_render_view_host_,
+                      FrameNavigationState::FrameID());
+    }
     pending_render_view_host_ = render_view_host;
   }
 }
@@ -372,12 +376,15 @@ void WebNavigationTabObserver::DidCommitProvisionalLoadForFrame(
   if (render_view_host != render_view_host_ &&
       render_view_host != pending_render_view_host_)
     return;
-  if (render_view_host != render_view_host_)
-    SendErrorEvents(web_contents(), render_view_host_);
+  FrameNavigationState::FrameID frame_id(frame_num, render_view_host);
+  FrameNavigationState::FrameID id_to_skip;
+  if (render_view_host == render_view_host_)
+    id_to_skip = frame_id;
+  if (is_main_frame)
+    SendErrorEvents(web_contents(), render_view_host_, id_to_skip);
   render_view_host_ = render_view_host;
   pending_render_view_host_ = NULL;
 
-  FrameNavigationState::FrameID frame_id(frame_num, render_view_host);
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
 
@@ -449,8 +456,10 @@ void WebNavigationTabObserver::DidFailProvisionalLoad(
         web_contents(), render_view_host->GetProcess()->GetID(), validated_url,
         frame_num, is_main_frame, error_code);
   }
-  if (stop_tracking_frames)
-    navigation_state_.StopTrackingFramesInRVH(render_view_host);
+  if (stop_tracking_frames) {
+    navigation_state_.StopTrackingFramesInRVH(render_view_host,
+                                              FrameNavigationState::FrameID());
+  }
 }
 
 void WebNavigationTabObserver::DocumentLoadedInFrame(
@@ -537,28 +546,31 @@ void WebNavigationTabObserver::DidOpenRequestedURL(
 void WebNavigationTabObserver::WebContentsDestroyed(content::WebContents* tab) {
   g_tab_observer.Get().erase(tab);
   registrar_.RemoveAll();
-  SendErrorEvents(tab, NULL);
+  SendErrorEvents(tab, NULL, FrameNavigationState::FrameID());
 }
 
 void WebNavigationTabObserver::SendErrorEvents(
     content::WebContents* web_contents,
-    content::RenderViewHost* render_view_host) {
+    content::RenderViewHost* render_view_host,
+    FrameNavigationState::FrameID id_to_skip) {
   for (FrameNavigationState::const_iterator frame = navigation_state_.begin();
        frame != navigation_state_.end(); ++frame) {
     if (!navigation_state_.GetNavigationCompleted(*frame) &&
         navigation_state_.CanSendEvents(*frame) &&
+        *frame != id_to_skip &&
         (!render_view_host || frame->render_view_host == render_view_host)) {
+      navigation_state_.SetErrorOccurredInFrame(*frame);
       helpers::DispatchOnErrorOccurred(
-        web_contents,
-        frame->render_view_host->GetProcess()->GetID(),
-        navigation_state_.GetUrl(*frame),
-        frame->frame_num,
-        navigation_state_.IsMainFrame(*frame),
-        net::ERR_ABORTED);
+          web_contents,
+          frame->render_view_host->GetProcess()->GetID(),
+          navigation_state_.GetUrl(*frame),
+          frame->frame_num,
+          navigation_state_.IsMainFrame(*frame),
+          net::ERR_ABORTED);
     }
   }
   if (render_view_host)
-    navigation_state_.StopTrackingFramesInRVH(render_view_host);
+    navigation_state_.StopTrackingFramesInRVH(render_view_host, id_to_skip);
 }
 
 // See also NavigationController::IsURLInPageNavigation.

chrome/browser/extensions/api/web_navigation/web_navigation_api.h

@@ -101,9 +101,11 @@ class WebNavigationTabObserver : public content::NotificationObserver,
 
   // Creates and sends onErrorOccurred events for all on-going navigations. If
   // |render_view_host| is non-NULL, only generates events for frames in this
-  // render view host.
+  // render view host. If |id_to_skip| is given, no events are sent for that
+  // frame.
   void SendErrorEvents(content::WebContents* web_contents,
-                       content::RenderViewHost* render_view_host);
+                       content::RenderViewHost* render_view_host,
+                       FrameNavigationState::FrameID id_to_skip);
 
   // Tracks the state of the frames we are sending events for.
   FrameNavigationState navigation_state_;