Fix a crash when a frame was inserted into a popup and navigated. I added a...

chrome/browser/navigation_controller.cc

 // Copyright (c) 2006-2008 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 "chrome/browser/navigation_controller.h"
 
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
@@ skipping 568 matching lines @@
       " navigated to a valid page. This should be impossible.";
     return NAV_IGNORE;
   }
 
   if (params.page_id > active_contents_->GetMaxPageID()) {
     // Greater page IDs than we've ever seen before are new pages. We may or may
     // not have a pending entry for the page, and this may or may not be the
     // main frame.
     if (PageTransition::IsMainFrame(params.transition))
       return NAV_NEW_PAGE;
+
+    // When this is a new subframe navigation, we should have a committed page
+    // for which it's a suframe in. This may not be the case when an iframe is
+    // navigated on a popup navigated to about:blank (the iframe would be
+    // written into the popup by script on the main page). For these cases,
+    // there isn't any navigation stuff we can do, so just ignore it.
+    if (!GetLastCommittedEntry())
+      return NAV_IGNORE;
+
+    // Valid subframe navigation.
     return NAV_NEW_SUBFRAME;
   }
 
   // Now we know that the notification is for an existing page. Find that entry.
   int existing_entry_index = GetEntryIndexWithPageID(
       active_contents_->type(),
       active_contents_->GetSiteInstance(),
       params.page_id);
   if (existing_entry_index == -1) {
     // The page was not found. It could have been pruned because of the limit on
@@ skipping 15 matching lines @@
     // a reload since it's the same page and not create a new entry for it
     // (the user doesn't want to have a new back/forward entry when they do
     // this). In this case, we want to just ignore the pending entry and go
     // back to where we were (the "existing entry").
     return NAV_SAME_PAGE;
   }
 
   if (AreURLsInPageNavigation(existing_entry->url(), params.url))
     return NAV_IN_PAGE;
 
-  if (!PageTransition::IsMainFrame(params.transition))
-    return NAV_AUTO_SUBFRAME;  // All manual subframes would get new IDs and
-                               // were handled above.
+  if (!PageTransition::IsMainFrame(params.transition)) {
+    // All manual subframes would get new IDs and were handled above, so we
+    // know this is auto. Since the current page was found in the navigation
+    // entry list, we're guaranteed to have a last committed entry.
+    DCHECK(GetLastCommittedEntry());
+    return NAV_AUTO_SUBFRAME;
+  }
+
   // Since we weeded out "new" navigations above, we know this is an existing
-  // navigation.
+  // (back/forward) navigation.
   return NAV_EXISTING_PAGE;
 }
 
 void NavigationController::RendererDidNavigateToNewPage(
     const ViewHostMsg_FrameNavigate_Params& params) {
   NavigationEntry* new_entry;
   if (pending_entry_) {
     // TODO(brettw) this assumes that the pending entry is appropriate for the
     // new page that was just loaded. I don't think this is necessarily the
     // case! We should have some more tracking to know for sure. This goes along
@@ skipping 17 matching lines @@
 
   InsertEntry(new_entry);
 }
 
 void NavigationController::RendererDidNavigateToExistingPage(
     const ViewHostMsg_FrameNavigate_Params& params) {
   // We should only get here for main frame navigations.
   DCHECK(PageTransition::IsMainFrame(params.transition));
 
   // This is a back/forward navigation. The existing page for the ID is
-  // guaranteed to exist, and we just need to update it with new information
-  // from the renderer.
+  // guaranteed to exist by ClassifyNavigation, and we just need to update it
+  // with new information from the renderer.
   int entry_index = GetEntryIndexWithPageID(
       active_contents_->type(),
       active_contents_->GetSiteInstance(),
       params.page_id);
   DCHECK(entry_index >= 0 &&
          entry_index < static_cast<int>(entries_.size()));
   NavigationEntry* entry = entries_[entry_index].get();
 
   // The URL may have changed due to redirects. The site instance will normally
   // be the same except during session restore, when no site instance will be
@@ skipping 13 matching lines @@
     DiscardPendingEntryInternal();
   
   int old_committed_entry_index = last_committed_entry_index_;
   last_committed_entry_index_ = entry_index;
   IndexOfActiveEntryChanged(old_committed_entry_index);
 }
 
 void NavigationController::RendererDidNavigateToSamePage(
     const ViewHostMsg_FrameNavigate_Params& params) {
   // This mode implies we have a pending entry that's the same as an existing
-  // entry for this page ID. All we need to do is update the existing entry.
+  // entry for this page ID. This entry is guaranteed to exist by
+  // ClassifyNavigation. All we need to do is update the existing entry.
   NavigationEntry* existing_entry = GetEntryWithPageID(
       active_contents_->type(),
       active_contents_->GetSiteInstance(),
       params.page_id);
 
   // We assign the entry's unique ID to be that of the new one. Since this is
   // always the result of a user action, we want to dismiss infobars, etc. like
   // a regular user-initiated navigation.
   existing_entry->set_unique_id(pending_entry_->unique_id());
 
@@ skipping 18 matching lines @@
   new_entry->set_url(params.url);
   InsertEntry(new_entry);
 }
 
 void NavigationController::RendererDidNavigateNewSubframe(
     const ViewHostMsg_FrameNavigate_Params& params) {
   // 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());
+  DCHECK(GetLastCommittedEntry()) << "ClassifyNavigation should guarantee "
+                                  << "that a last committed entry exists.";
   NavigationEntry* new_entry = new NavigationEntry(*GetLastCommittedEntry());
   new_entry->set_page_id(params.page_id);
   InsertEntry(new_entry);
 }
 
 bool NavigationController::RendererDidNavigateAutoSubframe(
     const ViewHostMsg_FrameNavigate_Params& params) {
   // We're guaranteed to have a previously committed entry, and we now need to
   // handle navigation inside of a subframe in it without creating a new entry.
   DCHECK(GetLastCommittedEntry());
@@ skipping 460 matching lines @@
 int NavigationController::GetEntryIndexWithPageID(
     TabContentsType type, SiteInstance* instance, int32 page_id) const {
   for (int i = static_cast<int>(entries_.size()) - 1; i >= 0; --i) {
     if ((entries_[i]->tab_type() == type) &&
         (entries_[i]->site_instance() == instance) &&
         (entries_[i]->page_id() == page_id))
       return i;
   }
   return -1;
 }