Revert 203400 "Add a killswitch for CSD malware IP match and rep..."

This CL introduced a race on the host_ variable.

> Add a killswitch for CSD malware IP match and report feature. Use a new killswitch whitelist URL which is part of the CSD whitelist.
> 
> BUG=176647
> 
> Review URL: https://chromiumcodereview.appspot.com/14999008

TBR=kewang@google.com

Review URL: https://codereview.chromium.org/16398002

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

8 files changed, 6 insertions, 67 deletions

diff --git a/chrome/browser/safe_browsing/client_side_detection_host.cc b/chrome/browser/safe_browsing/client_side_detection_host.cc
index cc8dc94..81cb917 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host.cc
@@ -177,8 +177,6 @@ class ClientSideDetectionHost::ShouldClassifyUrlRequest
       return;
     }
 
-    host_->malware_killswitch_on_ = database_manager_->MalwareKillSwitchOn();
-
     BrowserThread::PostTask(
         BrowserThread::UI,
         FROM_HERE,
@@ -253,7 +251,6 @@ ClientSideDetectionHost::ClientSideDetectionHost(WebContents* tab)
       csd_service_(NULL),
       weak_factory_(this),
       unsafe_unique_page_id_(-1),
-      malware_killswitch_on_(false),
       malware_report_enabled_(false) {
   DCHECK(tab);
   // Note: csd_service_ and sb_service will be NULL here in testing.
@@ -389,16 +386,15 @@ void ClientSideDetectionHost::OnPhishingDetectionDone(
       browse_info_.get() &&
       verdict->ParseFromString(verdict_str) &&
       verdict->IsInitialized()) {
-    // We do the malware IP matching and request sending if the feature
-    // is enabled
-    if (malware_report_enabled_ && !malware_killswitch_on_) {
+    if (malware_report_enabled_) {
       scoped_ptr<ClientMalwareRequest> malware_verdict(
           new ClientMalwareRequest);
       // Start browser-side malware feature extraction.  Once we're done it will
       // send the malware client verdict request.
       malware_verdict->set_url(verdict->url());
       feature_extractor_->ExtractMalwareFeatures(
-          browse_info_.get(), malware_verdict.get());
+          browse_info_.get(),
+          malware_verdict.get());
       MalwareFeatureExtractionDone(malware_verdict.Pass());
     }
 
diff --git a/chrome/browser/safe_browsing/client_side_detection_host.h b/chrome/browser/safe_browsing/client_side_detection_host.h
index e89d4d7..70dbbbe 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.h
+++ b/chrome/browser/safe_browsing/client_side_detection_host.h
@@ -137,12 +137,8 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   int unsafe_unique_page_id_;
   scoped_ptr<SafeBrowsingUIManager::UnsafeResource> unsafe_resource_;
 
-  // Whether the malware IP matching feature killswitch is on.
-  bool malware_killswitch_on_;
-
   // Whether the malware bad ip matching and report feature is enabled.
   bool malware_report_enabled_;
-
   DISALLOW_COPY_AND_ASSIGN(ClientSideDetectionHost);
 };
 
diff --git a/chrome/browser/safe_browsing/database_manager.cc b/chrome/browser/safe_browsing/database_manager.cc
index b1800e6..278dbac 100644
--- a/chrome/browser/safe_browsing/database_manager.cc
+++ b/chrome/browser/safe_browsing/database_manager.cc
@@ -307,14 +307,6 @@ bool SafeBrowsingDatabaseManager::MatchDownloadWhitelistString(
   return database_->ContainsDownloadWhitelistedString(str);
 }
 
-bool SafeBrowsingDatabaseManager::MalwareKillSwitchOn() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (!enabled_ || !MakeDatabaseAvailable()) {
-    return true;
-  }
-  return database_->MalwareIPMatchKillSwitchOn();
-}
-
 bool SafeBrowsingDatabaseManager::CheckBrowseUrl(const GURL& url,
                                                  Client* client) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
diff --git a/chrome/browser/safe_browsing/database_manager.h b/chrome/browser/safe_browsing/database_manager.h
index 5c57d63..ad1dd1e 100644
--- a/chrome/browser/safe_browsing/database_manager.h
+++ b/chrome/browser/safe_browsing/database_manager.h
@@ -172,9 +172,6 @@ class SafeBrowsingDatabaseManager
   // This method is expected to be called on the IO thread.
   virtual bool MatchDownloadWhitelistString(const std::string& str);
 
-  // Check if the CSD malware IP matching kill switch is turned on.
-  virtual bool MalwareKillSwitchOn();
-
   // Called on the IO thread to cancel a pending check if the result is no
   // longer needed.
   void CancelCheck(Client* client);
diff --git a/chrome/browser/safe_browsing/safe_browsing_database.cc b/chrome/browser/safe_browsing/safe_browsing_database.cc
index 6b93d3a..db81dc9 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database.cc
@@ -70,12 +70,6 @@ const size_t kMaxWhitelistSize = 5000;
 const char kWhitelistKillSwitchUrl[] =
     "sb-ssl.google.com/safebrowsing/csd/killswitch";  // Don't change this!
 
-// If the hash of this exact expression is on a whitelist then the
-// malware IP blacklisting feature will be disabled in csd.
-// Don't change this!
-const char kMalwareIPKillSwitchUrl[] =
-    "sb-ssl.google.com/safebrowsing/csd/killswitch_malware";
-
 // To save space, the incoming |chunk_id| and |list_id| are combined
 // into an |encoded_chunk_id| for storage by shifting the |list_id|
 // into the low-order bits.  These functions decode that information.
@@ -1614,12 +1608,3 @@ void SafeBrowsingDatabaseNew::LoadWhitelist(
     whitelist->first.swap(new_whitelist);
   }
 }
-
-bool SafeBrowsingDatabaseNew::MalwareIPMatchKillSwitchOn() {
-  SBFullHash malware_kill_switch;
-  crypto::SHA256HashString(kMalwareIPKillSwitchUrl, &malware_kill_switch,
-                           sizeof(malware_kill_switch));
-  std::vector<SBFullHash> full_hashes;
-  full_hashes.push_back(malware_kill_switch);
-  return ContainsWhitelistedHashes(csd_whitelist_, full_hashes);
-};
diff --git a/chrome/browser/safe_browsing/safe_browsing_database.h b/chrome/browser/safe_browsing/safe_browsing_database.h
index e3234c0..219f1ea 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.h
+++ b/chrome/browser/safe_browsing/safe_browsing_database.h
@@ -171,8 +171,6 @@ class SafeBrowsingDatabase {
       const std::vector<SBPrefix>& prefixes,
       const std::vector<SBFullHashResult>& full_hits) = 0;
 
-  virtual bool MalwareIPMatchKillSwitchOn() = 0;
-
   // The name of the bloom-filter file for the given database file.
   // NOTE(shess): OBSOLETE.  Present for deleting stale files.
   static base::FilePath BloomFilterForFilename(
@@ -299,9 +297,6 @@ class SafeBrowsingDatabaseNew : public SafeBrowsingDatabase {
       const std::vector<SBPrefix>& prefixes,
       const std::vector<SBFullHashResult>& full_hits) OVERRIDE;
 
-  // Returns the value of malware_kill_switch_;
-  virtual bool MalwareIPMatchKillSwitchOn() OVERRIDE;
-
  private:
   friend class SafeBrowsingDatabaseTest;
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingDatabaseTest, HashCaching);
@@ -373,7 +368,8 @@ class SafeBrowsingDatabaseNew : public SafeBrowsingDatabase {
 
   // Lock for protecting access to variables that may be used on the
   // IO thread.  This includes |prefix_set_|, |full_browse_hashes_|,
-  // |pending_browse_hashes_|, |prefix_miss_cache_|, |csd_whitelist_|.
+  // |pending_browse_hashes_|, |prefix_miss_cache_|, |csd_whitelist_|,
+  // and |csd_whitelist_all_urls_|.
   base::Lock lookup_lock_;
 
   // Underlying persistent store for chunk data.
diff --git a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
index 7d41684..ca61644 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database_unittest.cc
@@ -1367,19 +1367,6 @@ TEST_F(SafeBrowsingDatabaseTest, Whitelists) {
   EXPECT_FALSE(database_->ContainsDownloadWhitelistedUrl(
       GURL(std::string("http://www.google.com/"))));
 
-  // Test only add the malware IP killswitch
-  csd_chunks.clear();
-  chunk.hosts.clear();
-  InsertAddChunkHostFullHashes(
-      &chunk, 15, "sb-ssl.google.com/",
-      "sb-ssl.google.com/safebrowsing/csd/killswitch_malware");
-  csd_chunks.push_back(chunk);
-  EXPECT_TRUE(database_->UpdateStarted(&lists));
-  database_->InsertChunks(safe_browsing_util::kCsdWhiteList, csd_chunks);
-  database_->UpdateFinished(true);
-
-  EXPECT_TRUE(database_->MalwareIPMatchKillSwitchOn());
-
   // Test that the kill-switch works as intended.
   csd_chunks.clear();
   download_chunks.clear();
@@ -1388,6 +1375,7 @@ TEST_F(SafeBrowsingDatabaseTest, Whitelists) {
   InsertAddChunkHostFullHashes(&chunk, 5, "sb-ssl.google.com/",
                                "sb-ssl.google.com/safebrowsing/csd/killswitch");
   csd_chunks.push_back(chunk);
+
   chunk.hosts.clear();
   InsertAddChunkHostFullHashes(&chunk, 5, "sb-ssl.google.com/",
                                "sb-ssl.google.com/safebrowsing/csd/killswitch");
@@ -1399,7 +1387,6 @@ TEST_F(SafeBrowsingDatabaseTest, Whitelists) {
                           download_chunks);
   database_->UpdateFinished(true);
 
-  EXPECT_TRUE(database_->MalwareIPMatchKillSwitchOn());
   EXPECT_TRUE(database_->ContainsCsdWhitelistedUrl(
       GURL(std::string("https://") + kGood1Url2 + "/c.html")));
   EXPECT_TRUE(database_->ContainsCsdWhitelistedUrl(
@@ -1428,12 +1415,6 @@ TEST_F(SafeBrowsingDatabaseTest, Whitelists) {
   csd_chunks.push_back(sub_chunk);
 
   sub_chunk.hosts.clear();
-  InsertSubChunkHostFullHash(
-      &sub_chunk, 10, 15, "sb-ssl.google.com/",
-      "sb-ssl.google.com/safebrowsing/csd/killswitch_malware");
-  csd_chunks.push_back(sub_chunk);
-
-  sub_chunk.hosts.clear();
   InsertSubChunkHostFullHash(&sub_chunk, 1, 5,
                              "sb-ssl.google.com/",
                              "sb-ssl.google.com/safebrowsing/csd/killswitch");
@@ -1445,7 +1426,6 @@ TEST_F(SafeBrowsingDatabaseTest, Whitelists) {
                           download_chunks);
   database_->UpdateFinished(true);
 
-  EXPECT_FALSE(database_->MalwareIPMatchKillSwitchOn());
   EXPECT_TRUE(database_->ContainsCsdWhitelistedUrl(
       GURL(std::string("https://") + kGood1Url2 + "/c.html")));
   EXPECT_TRUE(database_->ContainsCsdWhitelistedUrl(
diff --git a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
index 6d5066e..879c373 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
@@ -143,9 +143,6 @@ class TestSafeBrowsingDatabase :  public SafeBrowsingDatabase {
       const std::vector<SBFullHashResult>& full_hits) OVERRIDE {
     // Do nothing for the cache.
   }
-  virtual bool MalwareIPMatchKillSwitchOn() OVERRIDE {
-    return false;
-  }
 
   // Fill up the database with test URL.
   void AddUrl(const GURL& url,