Do not fetch PVer4 updates when kDisableBackgroundNetworking is enabled

BUG=543161

Committed: https://crrev.com/deb2664a830bfedadc7b37b7954756aafaf0c396
Review-Url: https://codereview.chromium.org/2472903004
Cr-Original-Commit-Position: refs/heads/master@{#430365}
Cr-Commit-Position: refs/heads/master@{#431404}

chrome/browser/safe_browsing/local_database_manager_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/run_loop.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "components/safe_browsing_db/v4_get_hash_protocol_manager.h"
+#include "components/safe_browsing_db/v4_test_util.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
 #include "net/url_request/url_request_context_getter.h"
@@ -318,7 +319,7 @@ TEST_F(LocalDatabaseManagerTest, ServiceStopWithPendingChecks) {
   SafeBrowsingDatabaseManager::Client client;
 
   // Start the service and flush tasks to ensure database is made available.
-  db_manager->StartOnIOThread(NULL, V4ProtocolConfig());
+  db_manager->StartOnIOThread(NULL, GetTestV4ProtocolConfig());
   content::RunAllBlockingPoolTasksUntilIdle();
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(db_manager->DatabaseAvailable());

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -482,12 +482,11 @@ SafeBrowsingProtocolConfig SafeBrowsingService::GetProtocolConfig() const {
 
 V4ProtocolConfig
 SafeBrowsingService::GetV4ProtocolConfig() const {
-  V4ProtocolConfig config;
-  config.client_name = GetProtocolConfigClientName();
-  config.version = SafeBrowsingProtocolManagerHelper::Version();
-  config.key_param = google_apis::GetAPIKey();;
-
-  return config;
+  base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
+  return V4ProtocolConfig(
+      GetProtocolConfigClientName(),
+      cmdline->HasSwitch(switches::kDisableBackgroundNetworking),
+      google_apis::GetAPIKey(), SafeBrowsingProtocolManagerHelper::Version());
 }
 
 std::string SafeBrowsingService::GetProtocolConfigClientName() const {

chrome/test/BUILD.gn

@@ -4257,7 +4257,10 @@ test("unit_tests") {
       "../utility/safe_browsing/mac/read_stream_unittest.cc",
       "../utility/safe_browsing/mac/udif_unittest.cc",
     ]
-    deps += [ ":test_proto" ]
+    deps += [
+      ":test_proto",
+      "//components/safe_browsing_db:v4_test_util",
+    ]
 
     if (is_mac) {
       deps += [ ":mac_safe_browsing_test_data" ]

components/safe_browsing_db/BUILD.gn

@@ -288,6 +288,18 @@ source_set("v4_store") {
   ]
 }
 
+static_library("v4_test_util") {
+  testonly = true
+  sources = [
+    "v4_test_util.cc",
+    "v4_test_util.h",
+  ]
+  deps = [
+    ":util",
+    ":v4_protocol_manager_util",
+  ]
+}
+
 static_library("v4_update_protocol_manager") {
   sources = [
     "v4_update_protocol_manager.cc",
@@ -330,6 +342,7 @@ source_set("v4_get_hash_protocol_manager_unittest") {
     ":v4_database",
     ":v4_get_hash_protocol_manager",
     ":v4_local_database_manager",
+    ":v4_test_util",
     "//base",
     "//base/test:test_support",
     "//content/test:test_support",
@@ -347,6 +360,7 @@ source_set("v4_local_database_manager_unittest") {
   deps = [
     ":v4_database",
     ":v4_local_database_manager",
+    ":v4_test_util",
     "//base",
     "//base/test:test_support",
     "//content/test:test_support",
@@ -356,6 +370,24 @@ source_set("v4_local_database_manager_unittest") {
   ]
 }
 
+source_set("v4_update_protocol_manager_unittest") {
+  testonly = true
+  sources = [
+    "v4_update_protocol_manager_unittest.cc",
+  ]
+  deps = [
+    ":safebrowsing_proto",
+    ":util",
+    ":v4_test_util",
+    ":v4_update_protocol_manager",
+    "//base",
+    "//base/test:test_support",
+    "//net",
+    "//net:test_support",
+    "//testing/gtest",
+  ]
+}
+
 source_set("unit_tests") {
   testonly = true
   sources = [
@@ -385,6 +417,7 @@ source_set("unit_tests") {
     ":v4_rice",
     ":v4_store",
     ":v4_store_proto",
+    ":v4_test_util",
     ":v4_update_protocol_manager",
     "//base",
     "//components/prefs:test_support",
@@ -413,6 +446,7 @@ source_set("unit_tests_mobile") {
     ":safe_browsing_api_handler",
     ":safe_browsing_api_handler_util",
     ":util",
+    ":v4_test_util",
     "//base",
     "//components/variations",
     "//testing/gtest",

components/safe_browsing_db/database_manager_unittest.cc

@@ -20,6 +20,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/safe_browsing_db/test_database_manager.h"
 #include "components/safe_browsing_db/v4_protocol_manager_util.h"
+#include "components/safe_browsing_db/v4_test_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "crypto/sha2.h"
@@ -63,7 +64,7 @@ class SafeBrowsingDatabaseManagerTest : public testing::Test {
  protected:
   void SetUp() override {
     db_manager_ = new TestSafeBrowsingDatabaseManager();
-    db_manager_->StartOnIOThread(NULL, V4ProtocolConfig());
+    db_manager_->StartOnIOThread(NULL, GetTestV4ProtocolConfig());
   }
 
   void TearDown() override {

components/safe_browsing_db/safe_browsing_api_handler_unittest.cc

@@ -4,8 +4,8 @@
 
 #include "components/safe_browsing_db/metadata.pb.h"
 #include "components/safe_browsing_db/safe_browsing_api_handler_util.h"
-#include "components/safe_browsing_db/testing_util.h"
 #include "components/safe_browsing_db/util.h"
+#include "components/safe_browsing_db/v4_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace safe_browsing {

components/safe_browsing_db/v4_get_hash_protocol_manager_unittest.cc

@@ -14,8 +14,8 @@
 #include "base/test/simple_test_clock.h"
 #include "base/time/time.h"
 #include "components/safe_browsing_db/safebrowsing.pb.h"
-#include "components/safe_browsing_db/testing_util.h"
 #include "components/safe_browsing_db/util.h"
+#include "components/safe_browsing_db/v4_test_util.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
@@ -26,14 +26,6 @@
 using base::Time;
 using base::TimeDelta;
 
-namespace {
-
-const char kClient[] = "unittest";
-const char kAppVer[] = "1.0";
-const char kKeyParam[] = "test_key_param";
-
-}  // namespace
-
 namespace safe_browsing {
 
 namespace {
@@ -76,16 +68,13 @@ class V4GetHashProtocolManagerTest : public PlatformTest {
   }
 
   std::unique_ptr<V4GetHashProtocolManager> CreateProtocolManager() {
-    V4ProtocolConfig config;
-    config.client_name = kClient;
-    config.version = kAppVer;
-    config.key_param = kKeyParam;
     StoresToCheck stores_to_check(
         {GetUrlMalwareId(), GetChromeUrlApiId(),
          ListIdentifier(CHROME_PLATFORM, URL, SOCIAL_ENGINEERING_PUBLIC),
          ListIdentifier(CHROME_PLATFORM, URL,
                         POTENTIALLY_HARMFUL_APPLICATION)});
-    return V4GetHashProtocolManager::Create(NULL, stores_to_check, config);
+    return V4GetHashProtocolManager::Create(NULL, stores_to_check,
+                                            GetTestV4ProtocolConfig());
   }
 
   static void SetupFetcherToReturnOKResponse(

components/safe_browsing_db/v4_local_database_manager_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/test/test_simple_task_runner.h"
 #include "components/safe_browsing_db/v4_database.h"
 #include "components/safe_browsing_db/v4_local_database_manager.h"
+#include "components/safe_browsing_db/v4_test_util.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "testing/platform_test.h"
@@ -168,7 +169,8 @@ class V4LocalDatabaseManagerTest : public PlatformTest {
   }
 
   void StartLocalDatabaseManager() {
-    v4_local_database_manager_->StartOnIOThread(NULL, V4ProtocolConfig());
+    v4_local_database_manager_->StartOnIOThread(NULL,
+                                                GetTestV4ProtocolConfig());
   }
 
   void StopLocalDatabaseManager() {

components/safe_browsing_db/v4_protocol_manager_util.cc

@@ -175,7 +175,14 @@ ListIdentifier::ListIdentifier(const ListUpdateResponse& response)
                      response.threat_entry_type(),
                      response.threat_type()) {}
 
-V4ProtocolConfig::V4ProtocolConfig() : disable_auto_update(false) {}
+V4ProtocolConfig::V4ProtocolConfig(const std::string& client_name,
+                                   bool disable_auto_update,
+                                   const std::string& key_param,
+                                   const std::string& version)
+    : client_name(client_name),
+      disable_auto_update(disable_auto_update),
+      key_param(key_param),
+      version(version) {}
 
 V4ProtocolConfig::V4ProtocolConfig(const V4ProtocolConfig& other) = default;
 

components/safe_browsing_db/v4_protocol_manager_util.h

@@ -49,18 +49,24 @@ struct V4ProtocolConfig {
   // The safe browsing client name sent in each request.
   std::string client_name;
 
-  // Current product version sent in each request.
-  std::string version;
+  // Disable auto-updates using a command line switch.
+  bool disable_auto_update;
 
   // The Google API key.
   std::string key_param;
 
-  // Disable auto-updates using a command line switch?
-  bool disable_auto_update;
+  // Current product version sent in each request.
+  std::string version;
 
-  V4ProtocolConfig();
+  V4ProtocolConfig(const std::string& client_name,
+                   bool disable_auto_update,
+                   const std::string& key_param,
+                   const std::string& version);
   V4ProtocolConfig(const V4ProtocolConfig& other);
   ~V4ProtocolConfig();
+
+ private:
+  V4ProtocolConfig();
 };
 
 // Different types of threats that SafeBrowsing protects against. This is the

components/safe_browsing_db/v4_protocol_manager_util_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/base64.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
+#include "components/safe_browsing_db/v4_test_util.h"
 #include "net/base/escape.h"
 #include "net/http/http_request_headers.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -18,10 +19,6 @@ using base::TimeDelta;
 
 namespace {
 
-const char kClient[] = "unittest";
-const char kAppVer[] = "1.0";
-const char kKeyParam[] = "test_key_param";
-
 bool VectorContains(const std::vector<std::string>& data,
                     const std::string& str) {
   return std::find(data.begin(), data.end(), str) != data.end();
@@ -32,12 +29,6 @@ bool VectorContains(const std::vector<std::string>& data,
 namespace safe_browsing {
 
 class V4ProtocolManagerUtilTest : public testing::Test {
- protected:
-  void PopulateV4ProtocolConfig(V4ProtocolConfig* config) {
-    config->client_name = kClient;
-    config->version = kAppVer;
-    config->key_param = kKeyParam;
-  }
 };
 
 TEST_F(V4ProtocolManagerUtilTest, TestBackOffLogic) {
@@ -115,13 +106,11 @@ TEST_F(V4ProtocolManagerUtilTest, TestBackOffLogic) {
 }
 
 TEST_F(V4ProtocolManagerUtilTest, TestGetRequestUrlAndUpdateHeaders) {
-  V4ProtocolConfig config;
-  PopulateV4ProtocolConfig(&config);
-
   net::HttpRequestHeaders headers;
   GURL gurl;
   V4ProtocolManagerUtil::GetRequestUrlAndHeaders("request_base64", "someMethod",
-                                                 config, &gurl, &headers);
+                                                 GetTestV4ProtocolConfig(),
+                                                 &gurl, &headers);
   std::string expectedUrl =
       "https://safebrowsing.googleapis.com/v4/someMethod?"
       "$req=request_base64&$ct=application/x-protobuf&key=test_key_param";

components/safe_browsing_db/v4_test_util.cc

@@ -0,0 +1,30 @@
+// Copyright 2016 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 "components/safe_browsing_db/util.h"
+#include "components/safe_browsing_db/v4_test_util.h"
+
+namespace safe_browsing {
+
+namespace {
+
+const char kClient[] = "unittest";
+const char kAppVer[] = "1.0";
+const char kKeyParam[] = "test_key_param";
+
+}  // namespace
+
+V4ProtocolConfig GetTestV4ProtocolConfig(bool disable_auto_update) {
+  return V4ProtocolConfig(kClient, disable_auto_update, kKeyParam, kAppVer);
+}
+
+std::ostream& operator<<(std::ostream& os, const ThreatMetadata& meta) {
+  os << "{threat_pattern_type=" << static_cast<int>(meta.threat_pattern_type)
+     << ", api_permissions=[";
+  for (auto p : meta.api_permissions)
+    os << p << ",";
+  return os << "], population_id=" << meta.population_id << "}";
+}
+
+}  // namespace safe_browsing

components/safe_browsing_db/v4_test_util.h

@@ -0,0 +1,23 @@
+// Copyright 2016 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.
+
+#ifndef COMPONENTS_SAFE_BROWSING_DB_V4_TEST_UTIL_H_
+#define COMPONENTS_SAFE_BROWSING_DB_V4_TEST_UTIL_H_
+
+// Contains methods useful for tests.
+
+#include <ostream>
+
+namespace safe_browsing {
+
+struct ThreatMetadata;
+struct V4ProtocolConfig;
+
+V4ProtocolConfig GetTestV4ProtocolConfig(bool disable_auto_update = false);
+
+std::ostream& operator<<(std::ostream& os, const ThreatMetadata& meta);
+
+}  // namespace safe_browsing
+
+#endif  // COMPONENTS_SAFE_BROWSING_DB_V4_TEST_UTIL_H_

components/safe_browsing_db/v4_update_protocol_manager.cc

@@ -142,8 +142,6 @@ void V4UpdateProtocolManager::ScheduleNextUpdate(
 void V4UpdateProtocolManager::ScheduleNextUpdateWithBackoff(bool back_off) {
   DCHECK(CalledOnValidThread());
 
-  // TODO(vakh): Set disable_auto_update correctly using the command line
-  // switch.
   if (config_.disable_auto_update) {
     DCHECK(!IsUpdateScheduled());
     return;