diff options
| author | bauerb@chromium.org <bauerb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-29 19:19:06 +0000 |
|---|---|---|
| committer | bauerb@chromium.org <bauerb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-29 19:19:06 +0000 |
| commit | d497b0781cb67b0d7359a633d88c1ada077992aa (patch) | |
| tree | a53748014d4d8f9265a5c8e266d0fd4d7639cd0c /chrome/browser | |
| parent | 53d1edcbd0c3fc30b5e3af6529a516c41ba8debe (diff) | |
| download | chromium_src-d497b0781cb67b0d7359a633d88c1ada077992aa.zip chromium_src-d497b0781cb67b0d7359a633d88c1ada077992aa.tar.gz chromium_src-d497b0781cb67b0d7359a633d88c1ada077992aa.tar.bz2 | |
Fix a regression appearing in r41875 where navigating back from an interstitial page wouldn't work.
Also, factor out a method WindowDispositionShouldOpenNewTab that's used to decide if a given window disposition opens in a new tab or the current tab.
BUG=39248
TEST=SSLUITest.TestHTTPSExpiredCertAndGoBackViaButton, SSLUITest.TestHTTPSExpiredCertAndGoBackViaMenu, manual test: Go to www.google.com, then to https://ebay.com to show the HTTPS interstitial, then navigate back using the toolbar.
Review URL: http://codereview.chromium.org/1406001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42974 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
| -rw-r--r-- | chrome/browser/browser.cc | 19 | ||||
| -rw-r--r-- | chrome/browser/browser.h | 3 | ||||
| -rw-r--r-- | chrome/browser/ssl/ssl_browser_tests.cc | 30 |
3 files changed, 45 insertions, 7 deletions
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index 48cb020..be26e1e 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -893,11 +893,16 @@ void Browser::UpdateCommandsForFullscreenMode(bool is_fullscreen) { /////////////////////////////////////////////////////////////////////////////// // Browser, Assorted browser commands: +bool Browser::ShouldOpenNewTabForWindowDisposition( + WindowOpenDisposition disposition) { + return (disposition == NEW_FOREGROUND_TAB || + disposition == NEW_BACKGROUND_TAB); +} + NavigationController& Browser::GetOrCloneNavigationControllerForDisposition( WindowOpenDisposition disposition) { TabContents* current_tab = GetSelectedTabContents(); - if (disposition == NEW_FOREGROUND_TAB || - disposition == NEW_BACKGROUND_TAB) { + if (ShouldOpenNewTabForWindowDisposition(disposition)) { TabContents* cloned = current_tab->Clone(); tabstrip_model_.AddTabContents(cloned, -1, false, PageTransition::LINK, @@ -916,11 +921,13 @@ void Browser::GoBack(WindowOpenDisposition disposition) { if (current_tab->controller().CanGoBack()) { NavigationController& controller = GetOrCloneNavigationControllerForDisposition(disposition); - // The interstitial won't be copied to the new tab, so we don't need to - // go back. - if (!current_tab->interstitial_page()) { - controller.GoBack(); + // If we are on an interstitial page and clone the tab, it won't be copied + // to the new tab, so we don't need to go back. + if (current_tab->interstitial_page() && + ShouldOpenNewTabForWindowDisposition(disposition)) { + return; } + controller.GoBack(); } } diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h index 51470f7..801f3cd 100644 --- a/chrome/browser/browser.h +++ b/chrome/browser/browser.h @@ -821,6 +821,9 @@ class Browser : public TabStripModelDelegate, // Shared code between Reload() and ReloadAll(). void ReloadInternal(bool ignore_cache); + // Return true if the window dispositions means opening a new tab. + bool ShouldOpenNewTabForWindowDisposition(WindowOpenDisposition disposition); + // Depending on the disposition, return the |NavigationController| for the // current tab or clone the current tab and return its |NavigationController|. NavigationController& GetOrCloneNavigationControllerForDisposition( diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc index c21b481..087d5a9 100644 --- a/chrome/browser/ssl/ssl_browser_tests.cc +++ b/chrome/browser/ssl/ssl_browser_tests.cc @@ -220,8 +220,36 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndDontProceed) { CheckUnauthenticatedState(tab); } +// Visits a page with https error and then goes back using Browser::GoBack. +IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoBackViaButton) { + scoped_refptr<HTTPTestServer> http_server = PlainServer(); + ASSERT_TRUE(http_server.get() != NULL); + scoped_refptr<HTTPSTestServer> bad_https_server = BadCertServer(); + ASSERT_TRUE(bad_https_server.get() != NULL); + + // First navigate to an HTTP page. + ui_test_utils::NavigateToURL(browser(), http_server->TestServerPageW( + L"files/ssl/google.html")); + TabContents* tab = browser()->GetSelectedTabContents(); + NavigationEntry* entry = tab->controller().GetActiveEntry(); + ASSERT_TRUE(entry); + + // Now go to a bad HTTPS page that shows an interstitial. + ui_test_utils::NavigateToURL(browser(), + bad_https_server->TestServerPageW(L"files/ssl/google.html")); + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + true); // Interstitial showing + + // Simulate user clicking on back button (crbug.com/39248). + browser()->GoBack(CURRENT_TAB); + + // We should be back at the original good page. + EXPECT_FALSE(browser()->GetSelectedTabContents()->interstitial_page()); + CheckUnauthenticatedState(tab); +} + // Visits a page with https error and then goes back using GoToOffset. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoBack) { +IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoBackViaMenu) { scoped_refptr<HTTPTestServer> http_server = PlainServer(); ASSERT_TRUE(http_server.get() != NULL); scoped_refptr<HTTPSTestServer> bad_https_server = BadCertServer(); |