From 3ba7819822c6f7a4d0ec9987cb9b9128e99dda6e Mon Sep 17 00:00:00 2001 From: "davemoore@chromium.org" Date: Fri, 4 Feb 2011 18:37:17 +0000 Subject: Revert 73810 - 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 TBR=davemoore@chromium.org Review URL: http://codereview.chromium.org/6246111 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@73821 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/sessions/session_restore.cc | 8 ++---- .../sessions/session_restore_browsertest.cc | 33 ---------------------- 2 files changed, 2 insertions(+), 39 deletions(-) (limited to 'chrome') diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc index c986f42..49e6399 100644 --- a/chrome/browser/sessions/session_restore.cc +++ b/chrome/browser/sessions/session_restore.cc @@ -267,12 +267,8 @@ void TabLoader::Observe(NotificationType type, case NotificationType::TAB_CONTENTS_DESTROYED: { TabContents* tab_contents = Source(source).ptr(); if (!got_first_paint_) { - 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); + render_widget_hosts_loading_.erase( + tab_contents->GetRenderWidgetHostView()->GetRenderWidgetHost()); } HandleTabClosedOrLoaded(&tab_contents->controller()); break; diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc index c9f79af..a6b6f2a 100644 --- a/chrome/browser/sessions/session_restore_browsertest.cc +++ b/chrome/browser/sessions/session_restore_browsertest.cc @@ -142,36 +142,3 @@ 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(); -} - -- cgit v1.1