Added NULL check to protect against a TabContents having a NULL

RenderWidgetHostView. This shouldn't happen, but the crash
server indicates that there's some situation where it does.

I also added a test to exercise the code where the tab is closed
while being restored. That did not trigger this crash but
seems good to have anyway

BUG=71566
TEST=SessionRestoreTest.CloseDuringRestore

Review URL: http://codereview.chromium.org/6334091

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

2 files changed, 39 insertions, 2 deletions

diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index 49e6399..c986f42 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -267,8 +267,12 @@ void TabLoader::Observe(NotificationType type,
     case NotificationType::TAB_CONTENTS_DESTROYED: {
       TabContents* tab_contents = Source<TabContents>(source).ptr();
       if (!got_first_paint_) {
-        render_widget_hosts_loading_.erase(
-            tab_contents->GetRenderWidgetHostView()->GetRenderWidgetHost());
+        RenderWidgetHost* render_widget_host =
+            GetRenderWidgetHost(&tab_contents->controller());
+        // The render_widget_host should never be NULL, and yet it appears
+        // to happen, according to the crash reports.
+        if (render_widget_host)
+          render_widget_hosts_loading_.erase(render_widget_host);
       }
       HandleTabClosedOrLoaded(&tab_contents->controller());
       break;
diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc
index a6b6f2a..c9f79af 100644
--- a/chrome/browser/sessions/session_restore_browsertest.cc
+++ b/chrome/browser/sessions/session_restore_browsertest.cc
@@ -142,3 +142,36 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, WindowWithOneTab) {
   // Make sure the restore was successful.
   EXPECT_EQ(0U, service->entries().size());
 }
+
+// There's some complicated logic to handle the browser being closed during
+// restore. This will exercise that path.
+IN_PROC_BROWSER_TEST_F(SessionRestoreTest,
+                       CloseDuringRestore) {
+  if (browser_defaults::kRestorePopups)
+    return;
+
+  const FilePath::CharType* kTitle1File = FILE_PATH_LITERAL("title1.html");
+  GURL url(ui_test_utils::GetTestUrl(FilePath(FilePath::kCurrentDirectory),
+                                     FilePath(kTitle1File)));
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Turn on session restore.
+  SessionStartupPref::SetStartupPref(
+      browser()->profile(),
+      SessionStartupPref(SessionStartupPref::LAST));
+
+  // Create a new popup.
+  Profile* profile = browser()->profile();
+  Browser* popup = Browser::CreateForType(Browser::TYPE_POPUP, profile);
+  popup->window()->Show();
+
+  // Close the browser.
+  browser()->window()->Close();
+
+  // Create a new window, which should trigger session restore.
+  popup->NewWindow();
+  Browser* new_browser = ui_test_utils::WaitForNewBrowser();
+
+  new_browser->window()->Close();
+}
+