diff options
| author | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-09 23:56:31 +0000 |
|---|---|---|
| committer | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-09 23:56:31 +0000 |
| commit | a1a482b78c02ea8da62485a7fde77ff1867865be (patch) | |
| tree | 5854f16b1ceeda9fe1b8145933f77a8eab0e9463 /chrome/browser/safe_browsing | |
| parent | 9eaf0bac5e352ef93d7722d02d13e684446b3ad1 (diff) | |
| download | chromium_src-a1a482b78c02ea8da62485a7fde77ff1867865be.zip chromium_src-a1a482b78c02ea8da62485a7fde77ff1867865be.tar.gz chromium_src-a1a482b78c02ea8da62485a7fde77ff1867865be.tar.bz2 | |
This CL ensures the SafeBrowsingService is only notified once
by the SafeBrowsingInterstitialPage.
The interstitial page logic makes sure its Proceed/DontProceed
are called only once, but the DontProceed could also be called
when navigating back. Since the SafeBrowsingService::Client
is deleted when the SafeBrowsingService is notified, that would
cause a crasher on the 2nd notification.
BUG=http://crbug.com/30079
TEST=See bug.
Review URL: http://codereview.chromium.org/697002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41092 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/safe_browsing')
| -rw-r--r-- | chrome/browser/safe_browsing/safe_browsing_blocking_page.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc | 26 |
2 files changed, 36 insertions, 0 deletions
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc index 6d62b5e..d37c6ce 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc @@ -417,6 +417,16 @@ void SafeBrowsingBlockingPage::Proceed() { } void SafeBrowsingBlockingPage::DontProceed() { + DCHECK(action_taken() != DONT_PROCEED_ACTION); + // We could have already called Proceed(), in which case we must not notify + // the SafeBrowsingService again, as the client has been deleted. + if (action_taken() == PROCEED_ACTION) { + // We still want to hide the interstitial page. + InterstitialPage::DontProceed(); + // We are now deleted. + return; + } + RecordSafeBrowsingBlockingPageStats(DONT_PROCEED); NotifySafeBrowsingService(sb_service_, unsafe_resources_, false); diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc index cc55d3b..6fadf58 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc @@ -406,3 +406,29 @@ TEST_F(SafeBrowsingBlockingPageTest, NavigatingBackAndForth) { ASSERT_EQ(2, controller().entry_count()); EXPECT_EQ(kBadURL, controller().GetActiveEntry()->url().spec()); } + +// Tests that calling "don't proceed" after "proceed" has been called doesn't +// cause problems. http://crbug.com/30079 +TEST_F(SafeBrowsingBlockingPageTest, ProceedThenDontProceed) { + // Start a load. + controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED); + + // Simulate the load causing a safe browsing interstitial to be shown. + ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL); + SafeBrowsingBlockingPage* sb_interstitial = GetSafeBrowsingBlockingPage(); + ASSERT_TRUE(sb_interstitial); + + MessageLoop::current()->RunAllPending(); + + // Simulate the user clicking "proceed" then "don't proceed" (before the + // interstitial is shown). + sb_interstitial->Proceed(); + sb_interstitial->DontProceed(); + // Proceed() and DontProceed() post a task to update the + // SafeBrowsingService::Client. + MessageLoop::current()->RunAllPending(); + + // The interstitial should be gone. + EXPECT_EQ(OK, user_response()); + EXPECT_FALSE(GetSafeBrowsingBlockingPage()); +} |