Fix the go-back and proceed actions for client side phishing interstitials.

These interstitials show after the page has finished loading, so we should treat them similarly to interstitials we show for blocked subresources.  Also, tweak the main InterstitialPage class so that it won't call TabContents::SetIsLoading(true) unless the tab was loading at the time we showed the interstitial.

BUG=none
TEST=tested go back, proceed, and nav-away in the browser

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

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

4 files changed, 31 insertions, 11 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index 1e99a0f..182719a 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -125,13 +125,13 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
     TabContents* tab_contents,
     const UnsafeResourceList& unsafe_resources)
     : ChromeInterstitialPage(tab_contents,
-                             IsMainPage(unsafe_resources),
+                             IsMainPageLoadBlocked(unsafe_resources),
                              unsafe_resources[0].url),
       sb_service_(sb_service),
-      is_main_frame_(IsMainPage(unsafe_resources)),
+      is_main_frame_load_blocked_(IsMainPageLoadBlocked(unsafe_resources)),
       unsafe_resources_(unsafe_resources) {
   RecordUserAction(SHOW);
-  if (!is_main_frame_) {
+  if (!is_main_frame_load_blocked_) {
     navigation_entry_index_to_remove_ =
         tab()->controller().last_committed_entry_index();
   } else {
@@ -302,7 +302,7 @@ void SafeBrowsingBlockingPage::PopulateMalwareStringDictionary(
   // Check to see if we're blocking the main page, or a sub-resource on the
   // main page.
   string16 description1, description3, description5;
-  if (is_main_frame_) {
+  if (is_main_frame_load_blocked_) {
     description1 = l10n_util::GetStringFUTF16(
         IDS_SAFE_BROWSING_MALWARE_DESCRIPTION1, UTF8ToUTF16(url().host()));
   } else {
@@ -686,8 +686,16 @@ void SafeBrowsingBlockingPage::ShowBlockingPage(
 }
 
 // static
-bool SafeBrowsingBlockingPage::IsMainPage(
+bool SafeBrowsingBlockingPage::IsMainPageLoadBlocked(
     const UnsafeResourceList& unsafe_resources) {
+  // Client-side phishing detection interstitials never block the main frame
+  // load, since they happen after the page is finished loading.
+  if (unsafe_resources[0].threat_type ==
+      SafeBrowsingService::CLIENT_SIDE_PHISHING_URL) {
+    return false;
+  }
+
+  // Otherwise, check the threat type.
   return unsafe_resources.size() == 1 &&
          unsafe_resources[0].resource_type == ResourceType::MAIN_FRAME;
 }
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.h b/chrome/browser/safe_browsing/safe_browsing_blocking_page.h
index 7ae667d..6ae1549 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.h
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.h
@@ -128,8 +128,10 @@ class SafeBrowsingBlockingPage : public ChromeInterstitialPage {
                                         const UnsafeResourceList& resources,
                                         bool proceed);
 
-  // Returns true if the passed |unsafe_resources| is for the main page.
-  static bool IsMainPage(const UnsafeResourceList& unsafe_resources);
+  // Returns true if the passed |unsafe_resources| is blocking the load of
+  // the main page.
+  static bool IsMainPageLoadBlocked(
+      const UnsafeResourceList& unsafe_resources);
 
  private:
   friend class SafeBrowsingBlockingPageFactoryImpl;
@@ -138,8 +140,11 @@ class SafeBrowsingBlockingPage : public ChromeInterstitialPage {
   SafeBrowsingService* sb_service_;
   MessageLoop* report_loop_;
 
-  // Whether the flagged resource is the main page (or a sub-resource is false).
-  bool is_main_frame_;
+  // True if the interstitial is blocking the main page because it is on one
+  // of our lists.  False if a subresource is being blocked, or in the case of
+  // client-side detection where the interstitial is shown after page load
+  // finishes.
+  bool is_main_frame_load_blocked_;
 
   // The index of a navigation entry that should be removed when DontProceed()
   // is invoked, -1 if not entry should be removed.
diff --git a/content/browser/tab_contents/interstitial_page.cc b/content/browser/tab_contents/interstitial_page.cc
index 3d86c38..4292f15 100644
--- a/content/browser/tab_contents/interstitial_page.cc
+++ b/content/browser/tab_contents/interstitial_page.cc
@@ -153,6 +153,7 @@ InterstitialPage::InterstitialPage(TabContents* tab,
       original_child_id_(tab->render_view_host()->process()->id()),
       original_rvh_id_(tab->render_view_host()->routing_id()),
       should_revert_tab_title_(false),
+      tab_was_loading_(false),
       resource_dispatcher_host_notified_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(rvh_view_delegate_(
           new InterstitialPageRVHViewDelegate(this))) {
@@ -378,6 +379,7 @@ void InterstitialPage::DidNavigate(
   // by the UI tests) expects to consider a navigation as complete. Without
   // this, navigating in a UI test to a URL that triggers an interstitial would
   // hang.
+  tab_was_loading_ = tab_->is_loading();
   tab_->SetIsLoading(false, NULL);
 }
 
@@ -442,8 +444,9 @@ void InterstitialPage::Proceed() {
   Disable();
   action_taken_ = PROCEED_ACTION;
 
-  // Resumes the throbber.
-  tab_->SetIsLoading(true, NULL);
+  // Resumes the throbber, if applicable.
+  if (tab_was_loading_)
+    tab_->SetIsLoading(true, NULL);
 
   // If this is a new navigation, the old page is going away, so we cancel any
   // blocked requests for it.  If it is not a new navigation, then it means the
diff --git a/content/browser/tab_contents/interstitial_page.h b/content/browser/tab_contents/interstitial_page.h
index 1340893..d9b06fe 100644
--- a/content/browser/tab_contents/interstitial_page.h
+++ b/content/browser/tab_contents/interstitial_page.h
@@ -219,6 +219,10 @@ class InterstitialPage : public NotificationObserver,
   // it to its original value).
   bool should_revert_tab_title_;
 
+  // Whether or not the tab was loading resources when the interstitial was
+  // shown.  We restore this state if the user proceeds from the interstitial.
+  bool tab_was_loading_;
+
   // Whether the ResourceDispatcherHost has been notified to cancel/resume the
   // resource requests blocked for the RenderViewHost.
   bool resource_dispatcher_host_notified_;