Currently, there is a bug in the way we show the csd phishing interstitial.

chrome/browser/safe_browsing/safe_browsing_service.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/path_service.h"
@@ skipping 323 matching lines @@
 void SafeBrowsingService::DisplayBlockingPage(
     const GURL& url,
     const GURL& original_url,
     const std::vector<GURL>& redirect_urls,
     ResourceType::Type resource_type,
     UrlCheckResult result,
     Client* client,
     int render_process_host_id,
     int render_view_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
-  // Check if the user has already ignored our warning for this render_view
-  // and domain.
-  for (size_t i = 0; i < white_listed_entries_.size(); ++i) {
-    const WhiteListedEntry& entry = white_listed_entries_[i];
-    if (entry.render_process_host_id == render_process_host_id &&
-        entry.render_view_id == render_view_id &&
-        entry.result == result &&
-        entry.domain ==
-        net::RegistryControlledDomainService::GetDomainAndRegistry(url)) {
-      MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
-          this, &SafeBrowsingService::NotifyClientBlockingComplete,
-          client, true));
-      return;
-    }
-  }
-

[mattm, 2011/05/31 22:00:41]

does this mean the whitelist isn't used for non-csd blocking pages?

[noelutz, 2011/06/01 01:18:01]

It is.  But instead of checking the whitelist here we check it in
DoDisplayBlockingPage.

[mattm, 2011/06/01 02:14:52]

oh, right.  I had confused which one calls the other.

   UnsafeResource resource;
   resource.url = url;
   resource.original_url = original_url;
   resource.redirect_urls = redirect_urls;
   resource.resource_type = resource_type;
   resource.threat_type= result;
   resource.client = client;
   resource.render_process_host_id = render_process_host_id;
   resource.render_view_id = render_view_id;
 
@@ skipping 75 matching lines @@
 
 void SafeBrowsingService::OnBlockingPageDone(
     const std::vector<UnsafeResource>& resources,
     bool proceed) {
   for (std::vector<UnsafeResource>::const_iterator iter = resources.begin();
        iter != resources.end(); ++iter) {
     const UnsafeResource& resource = *iter;
     NotifyClientBlockingComplete(resource.client, proceed);
 
     if (proceed) {
-      // Whitelist this domain and warning type for the given tab.
-      WhiteListedEntry entry;
-      entry.render_process_host_id = resource.render_process_host_id;
-      entry.render_view_id = resource.render_view_id;
-      entry.domain = net::RegistryControlledDomainService::GetDomainAndRegistry(
-            resource.url);
-      entry.result = resource.threat_type;
-      white_listed_entries_.push_back(entry);
+      BrowserThread::PostTask(
+          BrowserThread::UI,
+          FROM_HERE,
+          NewRunnableMethod(this,
+                            &SafeBrowsingService::UpdateWhitelist,
+                            resource));
     }
   }
 }
 
 void SafeBrowsingService::OnNewMacKeys(const std::string& client_key,
                                        const std::string& wrapped_key) {
   PrefService* prefs = g_browser_process->local_state();
   if (prefs) {
     prefs->SetString(prefs::kSafeBrowsingClientKey, client_key);
     prefs->SetString(prefs::kSafeBrowsingWrappedKey, wrapped_key);
@@ skipping 507 matching lines @@
     check->result = SAFE;
     if (index != -1)
       check->result = GetResultFromListname(full_hashes[index].list_name);
   }
   SafeBrowsingCheckDone(check);
   return (index != -1);
 }
 
 void SafeBrowsingService::DoDisplayBlockingPage(
     const UnsafeResource& resource) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // Check if the user has already ignored our warning for this render_view
+  // and domain.
+  if (IsWhitelisted(resource)) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        NewRunnableMethod(this,
+                          &SafeBrowsingService::NotifyClientBlockingComplete,
+                          resource.client, true));
+    return;
+  }
+
   // The tab might have been closed.
   TabContents* wc =
       tab_util::GetTabContentsByID(resource.render_process_host_id,
                                    resource.render_view_id);
 
   if (!wc) {
     // The tab is gone and we did not have a chance at showing the interstitial.
     // Just act as "Don't Proceed" was chosen.
     std::vector<UnsafeResource> resources;
     resources.push_back(resource);
@@ skipping 188 matching lines @@
   check->is_download = true;
   check->timeout_task =
       NewRunnableMethod(this, &SafeBrowsingService::TimeoutCallback, check);
   checks_.insert(check);
 
   safe_browsing_thread_->message_loop()->PostTask(FROM_HERE, task);
 
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE, check->timeout_task, timeout_ms);
 }
+
+void SafeBrowsingService::UpdateWhitelist(UnsafeResource resource) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // Whitelist this domain and warning type for the given tab.
+  WhiteListedEntry entry;
+  entry.render_process_host_id = resource.render_process_host_id;
+  entry.render_view_id = resource.render_view_id;
+  entry.domain = net::RegistryControlledDomainService::GetDomainAndRegistry(
+      resource.url);
+  entry.result = resource.threat_type;
+  white_listed_entries_.push_back(entry);
+}
+
+bool SafeBrowsingService::IsWhitelisted(const UnsafeResource& resource) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // Check if the user has already ignored our warning for this render_view
+  // and domain.
+  for (size_t i = 0; i < white_listed_entries_.size(); ++i) {
+    const WhiteListedEntry& entry = white_listed_entries_[i];
+    if (entry.render_process_host_id == resource.render_process_host_id &&
+        entry.render_view_id == resource.render_view_id &&
+        // Threat type must be the same or in the case of phishing they can
+        // either be client-side phishing URL or a SafeBrowsing phishing URL.
+        // If we show one type of phishing warning we don't want to show a
+        // second phishing warning.
+        (entry.result == resource.threat_type ||
+         (entry.result == URL_PHISHING &&
+          resource.threat_type == CLIENT_SIDE_PHISHING_URL) ||
+         (entry.result == CLIENT_SIDE_PHISHING_URL &&
+          resource.threat_type == URL_PHISHING))  &&
+        entry.domain ==
+        net::RegistryControlledDomainService::GetDomainAndRegistry(
+            resource.url)) {
+      return true;
+    }
+  }
+  return false;
+}