Only delete old frames when a new main frame navigation commits

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 164 matching lines @@
     // http://crbug.com/109464.
     DCHECK(chrome::GetViewType(details->source_web_contents) !=
            chrome::VIEW_TYPE_TAB_CONTENTS);
     return;
   }
   const FrameNavigationState& frame_navigation_state =
       tab_observer->frame_navigation_state();
 
   FrameNavigationState::FrameID frame_id(
       details->source_frame_id,
-      details->source_web_contents->GetRenderViewHost()->GetProcess()->GetID());
+      details->source_web_contents->GetRenderViewHost());
   if (!frame_navigation_state.CanSendEvents(frame_id))
     return;
 
   // If the WebContents was created as a response to an IPC from a renderer
   // (and therefore doesn't yet have a TabContents), or if it isn't yet inserted
   // into a tab strip, we need to delay the extension event until the
   // WebContents is fully initialized.
   if (TabContents::FromWebContents(details->target_web_contents) == NULL ||
       details->not_yet_in_tabstrip) {
     pending_web_contents_[details->target_web_contents] =
@@ skipping 45 matching lines @@
 
 WebNavigationTabObserver::WebNavigationTabObserver(
     content::WebContents* 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));
+  registrar_.Add(this,
+                 content::NOTIFICATION_RENDER_VIEW_HOST_DELETED,
+                 content::NotificationService::AllSources());
 }
 
 WebNavigationTabObserver::~WebNavigationTabObserver() {}
 
 // static
 WebNavigationTabObserver* WebNavigationTabObserver::Get(
     content::WebContents* web_contents) {
   TabObserverMap::iterator i = g_tab_observer.Get().find(web_contents);
   return i == g_tab_observer.Get().end() ? NULL : i->second;
 }
 
+content::RenderViewHost* WebNavigationTabObserver::GetRenderViewHostInProcess(
+    int process_id) const {
+  if (render_view_host_ &&
+      render_view_host_->GetProcess()->GetID() == process_id) {
+    return render_view_host_;
+  }
+  if (pending_render_view_host_ &&
+      pending_render_view_host_->GetProcess()->GetID() == process_id) {
+    return pending_render_view_host_;
+  }
+  return NULL;
+}
+
 void WebNavigationTabObserver::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
     case content::NOTIFICATION_RESOURCE_RECEIVED_REDIRECT: {
       content::ResourceRedirectDetails* resource_redirect_details =
           content::Details<content::ResourceRedirectDetails>(details).ptr();
       ResourceType::Type resource_type =
           resource_redirect_details->resource_type;
       if (resource_type == ResourceType::MAIN_FRAME ||
           resource_type == ResourceType::SUB_FRAME) {
+        content::RenderViewHost* render_view_host = NULL;
+        if (render_view_host_ &&
+            resource_redirect_details->origin_child_id ==
+                render_view_host_->GetProcess()->GetID()) {
+          render_view_host = render_view_host_;
+        } else if (pending_render_view_host_ &&
+                   resource_redirect_details->origin_child_id ==
+                       pending_render_view_host_->GetProcess()->GetID()) {
+          render_view_host = pending_render_view_host_;
+        }
+        if (!render_view_host)
+          return;
         FrameNavigationState::FrameID frame_id(
-            resource_redirect_details->frame_id,
-            resource_redirect_details->origin_child_id);
+            resource_redirect_details->frame_id, render_view_host);
         if (!navigation_state_.CanSendEvents(frame_id))
           return;
         navigation_state_.SetIsServerRedirected(frame_id);
       }
       break;
     }
 
+    case content::NOTIFICATION_RENDER_VIEW_HOST_DELETED: {
+      content::RenderViewHost* render_view_host =
+          content::Source<content::RenderViewHost>(source).ptr();
+      if (render_view_host == render_view_host_)
+        render_view_host_ = NULL;
+      else if (render_view_host == pending_render_view_host_)
+        pending_render_view_host_ = NULL;
+      else
+        return;
+      SendErrorEvents(web_contents(), render_view_host);

[Matt Perry, 2012/07/31 09:58:10]

Technically this is dangerous. This notification is sent from the RVH
destructor. SendErrorEvents ends up invoking a virtual method (GetProcess()) on
the RVH that is being destroyed. I think this particular case is fine, but a
purist would say this is not allowed. (It's dangerous because the vtable refers
to the RVH only - not a derived class - while in the destructor, as all derived
classes have invoked their destructors).

[jochen (gone - plz use gerrit), 2012/07/31 10:12:33]

I agree that it's a bit borderline :-/

+      break;
+    }
+
     default:
       NOTREACHED();
   }
 }
 
 void WebNavigationTabObserver::AboutToNavigateRenderView(
     content::RenderViewHost* render_view_host) {
   if (!render_view_host_) {
     render_view_host_ = render_view_host;
   } else if (render_view_host != render_view_host_) {
-    // TODO(jochen): If pending_render_view_host_ is non-NULL, send error events
-    // for all ongoing navigations in that RVH.
+    if (pending_render_view_host_)
+      SendErrorEvents(web_contents(), pending_render_view_host_);
     pending_render_view_host_ = render_view_host;
   }
 }
 
 void WebNavigationTabObserver::DidStartProvisionalLoadForFrame(
     int64 frame_num,
     bool is_main_frame,
     const GURL& validated_url,
     bool is_error_page,
     content::RenderViewHost* render_view_host) {
   if (!render_view_host_)
     render_view_host_ = render_view_host;
   if (render_view_host != render_view_host_ &&
       render_view_host != pending_render_view_host_)
     return;
 
-  FrameNavigationState::FrameID frame_id(
-      frame_num, render_view_host->GetProcess()->GetID());
+  FrameNavigationState::FrameID frame_id(frame_num, render_view_host);
 
   navigation_state_.TrackFrame(frame_id,
                                validated_url,
                                is_main_frame,
                                is_error_page);
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
 
   helpers::DispatchOnBeforeNavigate(
       web_contents(), render_view_host->GetProcess()->GetID(), frame_num,
       is_main_frame, validated_url);
 }
 
 void WebNavigationTabObserver::DidCommitProvisionalLoadForFrame(
     int64 frame_num,
     bool is_main_frame,
     const GURL& url,
     content::PageTransition transition_type,
     content::RenderViewHost* render_view_host) {
   if (render_view_host != render_view_host_ &&
       render_view_host != pending_render_view_host_)
     return;
-  // TODO(jochen): If we switched the RVH, send error events for all ongoing
-  // navigations in the old RVH.
+  if (render_view_host != render_view_host_)
+    SendErrorEvents(web_contents(), render_view_host_);
   render_view_host_ = render_view_host;
   pending_render_view_host_ = NULL;
 
-  FrameNavigationState::FrameID frame_id(
-      frame_num, render_view_host->GetProcess()->GetID());
+  FrameNavigationState::FrameID frame_id(frame_num, render_view_host);
   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 38 matching lines @@
 void WebNavigationTabObserver::DidFailProvisionalLoad(
     int64 frame_num,
     bool is_main_frame,
     const GURL& validated_url,
     int error_code,
     const string16& error_description,
     content::RenderViewHost* render_view_host) {
   if (render_view_host != render_view_host_ &&
       render_view_host != pending_render_view_host_)
     return;
-  if (render_view_host == pending_render_view_host_)
+  bool stop_tracking_frames = false;
+  if (render_view_host == pending_render_view_host_) {
     pending_render_view_host_ = NULL;
+    stop_tracking_frames = true;
+  }
 
-  FrameNavigationState::FrameID frame_id(
-      frame_num, render_view_host->GetProcess()->GetID());
-  if (!navigation_state_.CanSendEvents(frame_id))
-    return;
-
-  navigation_state_.SetErrorOccurredInFrame(frame_id);
-  helpers::DispatchOnErrorOccurred(
-      web_contents(), render_view_host->GetProcess()->GetID(), validated_url,
-      frame_num, is_main_frame, error_code);
+  FrameNavigationState::FrameID frame_id(frame_num, render_view_host);
+  if (navigation_state_.CanSendEvents(frame_id)) {
+    navigation_state_.SetErrorOccurredInFrame(frame_id);
+    helpers::DispatchOnErrorOccurred(
+        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);
 }
 
 void WebNavigationTabObserver::DocumentLoadedInFrame(
     int64 frame_num,
     content::RenderViewHost* render_view_host) {
   if (render_view_host != render_view_host_)
     return;
-  FrameNavigationState::FrameID frame_id(
-      frame_num, render_view_host->GetProcess()->GetID());
+  FrameNavigationState::FrameID frame_id(frame_num, render_view_host);
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
   helpers::DispatchOnDOMContentLoaded(web_contents(),
                                       navigation_state_.GetUrl(frame_id),
                                       navigation_state_.IsMainFrame(frame_id),
                                       frame_num);
 }
 
 void WebNavigationTabObserver::DidFinishLoad(
     int64 frame_num,
     const GURL& validated_url,
     bool is_main_frame,
     content::RenderViewHost* render_view_host) {
   if (render_view_host != render_view_host_)
     return;
-  FrameNavigationState::FrameID frame_id(
-      frame_num, render_view_host->GetProcess()->GetID());
+  FrameNavigationState::FrameID frame_id(frame_num, render_view_host);
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
   navigation_state_.SetNavigationCompleted(frame_id);
   DCHECK_EQ(navigation_state_.GetUrl(frame_id), validated_url);
   DCHECK_EQ(navigation_state_.IsMainFrame(frame_id), is_main_frame);
   helpers::DispatchOnCompleted(web_contents(),
                                validated_url,
                                is_main_frame,
                                frame_num);
 }
 
 void WebNavigationTabObserver::DidFailLoad(
     int64 frame_num,
     const GURL& validated_url,
     bool is_main_frame,
     int error_code,
     const string16& error_description,
     content::RenderViewHost* render_view_host) {
   if (render_view_host != render_view_host_)
     return;
-  FrameNavigationState::FrameID frame_id(
-      frame_num, render_view_host->GetProcess()->GetID());
+  FrameNavigationState::FrameID frame_id(frame_num, render_view_host);
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
   navigation_state_.SetErrorOccurredInFrame(frame_id);
   helpers::DispatchOnErrorOccurred(
       web_contents(), render_view_host->GetProcess()->GetID(), validated_url,
       frame_num, is_main_frame, error_code);
 }
 
 void WebNavigationTabObserver::DidOpenRequestedURL(
     content::WebContents* new_contents,
     const GURL& url,
     const content::Referrer& referrer,
     WindowOpenDisposition disposition,
     content::PageTransition transition,
     int64 source_frame_num) {
-  FrameNavigationState::FrameID frame_id(
-      source_frame_num, render_view_host_->GetProcess()->GetID());
+  FrameNavigationState::FrameID frame_id(source_frame_num, render_view_host_);
   if (!navigation_state_.CanSendEvents(frame_id))
     return;
 
   // We only send the onCreatedNavigationTarget if we end up creating a new
   // window.
   if (disposition != SINGLETON_TAB &&
       disposition != NEW_FOREGROUND_TAB &&
       disposition != NEW_BACKGROUND_TAB &&
       disposition != NEW_POPUP &&
       disposition != NEW_WINDOW &&
       disposition != OFF_THE_RECORD)
     return;
 
   helpers::DispatchOnCreatedNavigationTarget(
       web_contents(),
       new_contents->GetBrowserContext(),
       source_frame_num,
       navigation_state_.IsMainFrame(frame_id),
       new_contents,
       url);
 }
 
 void WebNavigationTabObserver::WebContentsDestroyed(content::WebContents* tab) {
   g_tab_observer.Get().erase(tab);
+  registrar_.RemoveAll();
+  SendErrorEvents(tab, NULL);
+}
+
+void WebNavigationTabObserver::SendErrorEvents(
+    content::WebContents* web_contents,
+    content::RenderViewHost* render_view_host) {
   for (FrameNavigationState::const_iterator frame = navigation_state_.begin();
        frame != navigation_state_.end(); ++frame) {
     if (!navigation_state_.GetNavigationCompleted(*frame) &&
-        navigation_state_.CanSendEvents(*frame)) {
+        navigation_state_.CanSendEvents(*frame) &&
+        (!render_view_host || frame->render_view_host == render_view_host)) {
       helpers::DispatchOnErrorOccurred(
-        tab,
-        frame->render_process_id,
+        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);
 }
 
 // See also NavigationController::IsURLInPageNavigation.
 bool WebNavigationTabObserver::IsReferenceFragmentNavigation(
     FrameNavigationState::FrameID frame_id,
     const GURL& url) {
   GURL existing_url = navigation_state_.GetUrl(frame_id);
   if (existing_url == url)
     return false;
 
@@ skipping 27 matching lines @@
   WebNavigationTabObserver* observer =
       WebNavigationTabObserver::Get(web_contents);
   DCHECK(observer);
 
   const FrameNavigationState& frame_navigation_state =
       observer->frame_navigation_state();
 
   if (frame_id == 0)
     frame_id = frame_navigation_state.GetMainFrameID().frame_num;
 
-  FrameNavigationState::FrameID internal_frame_id(frame_id, process_id);
+  content::RenderViewHost* render_view_host =
+      observer->GetRenderViewHostInProcess(process_id);
+  if (!render_view_host)
+    return true;
+
+  FrameNavigationState::FrameID internal_frame_id(frame_id, render_view_host);
   if (!frame_navigation_state.IsValidFrame(internal_frame_id))
     return true;
 
   GURL frame_url = frame_navigation_state.GetUrl(internal_frame_id);
   if (!frame_navigation_state.IsValidUrl(frame_url))
     return true;
 
   GetFrame::Results::Details frame_details;
   frame_details.url = frame_url.spec();
   frame_details.error_occurred =
@@ skipping 33 matching lines @@
        it != navigation_state.end(); ++it) {
     FrameNavigationState::FrameID frame_id = *it;
     GURL frame_url = navigation_state.GetUrl(frame_id);
     if (!navigation_state.IsValidUrl(frame_url))
       continue;
     linked_ptr<GetAllFrames::Results::DetailsElement> frame(
         new GetAllFrames::Results::DetailsElement());
     frame->url = frame_url.spec();
     frame->frame_id = helpers::GetFrameId(
         navigation_state.IsMainFrame(frame_id), frame_id.frame_num);
-    frame->process_id = frame_id.render_process_id;
+    frame->process_id = frame_id.render_view_host->GetProcess()->GetID();
     frame->error_occurred = navigation_state.GetErrorOccurredInFrame(frame_id);
     result_list.push_back(frame);
   }
   results_ = GetAllFrames::Results::Create(result_list);
   return true;
 }
 
 }  // namespace extensions