Enable Finch selection of Client Side Detection models.

Also update csd.proto with new fields from server-side.

I verified that these new models exist already on gstatic.
Models added in http://cl/95461410.

BUG=490457

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

Cr-Commit-Position: refs/heads/master@{#334046}

4 files changed, 170 insertions, 6 deletions

diff --git a/chrome/browser/safe_browsing/client_side_detection_service.cc b/chrome/browser/safe_browsing/client_side_detection_service.cc
index e7fab2f..191e00e 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service.cc
@@ -15,6 +15,7 @@
 #include "base/metrics/sparse_histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
@@ -24,6 +25,7 @@
 #include "chrome/common/safe_browsing/client_model.pb.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 #include "chrome/common/safe_browsing/safebrowsing_messages.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ -79,8 +81,17 @@ const char ClientSideDetectionService::kClientReportPhishingUrl[] =
     "https://sb-ssl.google.com/safebrowsing/clientreport/phishing";
 const char ClientSideDetectionService::kClientReportMalwareUrl[] =
     "https://sb-ssl.google.com/safebrowsing/clientreport/malware-check";
-const char ClientSideDetectionService::kClientModelUrl[] =
-    "https://ssl.gstatic.com/safebrowsing/csd/client_model_v5.pb";
+const char ClientSideDetectionService::kClientModelUrlPrefix[] =
+    "https://ssl.gstatic.com/safebrowsing/csd/";
+const char ClientSideDetectionService::kClientModelNamePattern[] =
+    "client_model_v5%s_variation_%d.pb";
+
+// Finch names
+const char ClientSideDetectionService::kClientModelFinchExperiment[] =
+    "ClientSideDetectionModel";
+const char ClientSideDetectionService::kClientModelFinchParam[] =
+    "ModelNum";
+
 
 struct ClientSideDetectionService::ClientReportInfo {
   ClientReportPhishingRequestCallback callback;
@@ -269,12 +280,37 @@ void ClientSideDetectionService::ScheduleFetchModel(int64 delay_ms) {
       base::TimeDelta::FromMilliseconds(delay_ms));
 }
 
+// static
+std::string ClientSideDetectionService::MakeModelName() {
+  std::string num_str = variations::GetVariationParamValue(
+      kClientModelFinchExperiment, kClientModelFinchParam);
+  int model_number = 0;
+  if (!base::StringToInt(num_str, &model_number)) {
+    model_number = 0;  // Default model
+  }
+
+  // TODO(nparker): Factor out the model-fetching so we can have
+  // up to two models: One for extended reporting, and one for not.
+  // Until then, we'll use the non-extended reporting model.
+  return FillInModelName(false /* is_extended_reporting */, model_number);
+}
+
+// static
+std::string ClientSideDetectionService::FillInModelName(
+    bool is_extended_reporting,
+    int model_number) {
+  return base::StringPrintf(kClientModelNamePattern,
+                            is_extended_reporting ? "_ext" : "", model_number);
+}
+
 void ClientSideDetectionService::StartFetchModel() {
   if (enabled_) {
     // Start fetching the model either from the cache or possibly from the
     // network if the model isn't in the cache.
+    fetching_model_name_ = MakeModelName();
+    std::string model_url = kClientModelUrlPrefix + fetching_model_name_;
     model_fetcher_ = net::URLFetcher::Create(0 /* ID used for testing */,
-                                             GURL(kClientModelUrl),
+                                             GURL(model_url),
                                              net::URLFetcher::GET, this);
     model_fetcher_->SetRequestContext(request_context_getter_.get());
     model_fetcher_->Start();
@@ -317,6 +353,8 @@ void ClientSideDetectionService::StartClientReportPhishingRequest(
     return;
   }
 
+  request->set_model_filename(model_name_);
+
   std::string request_data;
   if (!request->SerializeToString(&request_data)) {
     UMA_HISTOGRAM_COUNTS("SBClientPhishing.RequestNotSerialized", 1);
@@ -429,6 +467,8 @@ void ClientSideDetectionService::HandleModelResponse(
     // The model is valid => replace the existing model with the new one.
     model_str_.assign(data);
     model_.swap(model);
+    model_name_ = fetching_model_name_;
+    fetching_model_name_ = "";
     model_status = MODEL_SUCCESS;
   }
   EndFetchModel(model_status);
diff --git a/chrome/browser/safe_browsing/client_side_detection_service.h b/chrome/browser/safe_browsing/client_side_detection_service.h
index 671ba1a..5ed0f11 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service.h
+++ b/chrome/browser/safe_browsing/client_side_detection_service.h
@@ -180,6 +180,9 @@ class ClientSideDetectionService : public net::URLFetcherDelegate,
   FRIEND_TEST_ALL_PREFIXES(ClientSideDetectionServiceTest, IsBadIpAddress);
   FRIEND_TEST_ALL_PREFIXES(ClientSideDetectionServiceTest,
                            ModelHasValidHashIds);
+  FRIEND_TEST_ALL_PREFIXES(ClientSideDetectionServiceTest, ModelNamesTest);
+  FRIEND_TEST_ALL_PREFIXES(ClientSideDetectionServiceTest,
+                           FetchExperimentalModelTest);
 
   // CacheState holds all information necessary to respond to a caller without
   // actually making a HTTP request.
@@ -202,7 +205,10 @@ class ClientSideDetectionService : public net::URLFetcherDelegate,
 
   static const char kClientReportMalwareUrl[];
   static const char kClientReportPhishingUrl[];
-  static const char kClientModelUrl[];
+  static const char kClientModelUrlPrefix[];
+  static const char kClientModelNamePattern[];
+  static const char kClientModelFinchExperiment[];
+  static const char kClientModelFinchParam[];
   static const size_t kMaxModelSizeBytes;
   static const int kMaxReportsPerInterval;
   static const int kClientModelFetchIntervalMs;
@@ -281,6 +287,13 @@ class ClientSideDetectionService : public net::URLFetcherDelegate,
   // Returns the URL that will be used for phishing requests.
   static GURL GetClientReportUrl(const std::string& report_url);
 
+  // Construct a model name from parameters.
+  static std::string FillInModelName(bool is_extended_reporting,
+                                     int model_number);
+
+  // Construct a model name from client state.
+  static std::string MakeModelName();
+
   // Whether the service is running or not.  When the service is not running,
   // it won't download the model nor report detected phishing URLs.
   bool enabled_;
@@ -290,6 +303,11 @@ class ClientSideDetectionService : public net::URLFetcherDelegate,
   scoped_ptr<base::TimeDelta> model_max_age_;
   scoped_ptr<net::URLFetcher> model_fetcher_;
 
+  // Name of the model in model_.  This is the last component of the URL path.
+  std::string model_name_;
+  // Name of the model currently being fetched.
+  std::string fetching_model_name_;
+
   // Map of client report phishing request to the corresponding callback that
   // has to be invoked when the request is done.
   struct ClientReportInfo;
diff --git a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
index 275409f..0bd2371 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
@@ -11,10 +11,13 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/field_trial.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/safe_browsing/client_side_detection_service.h"
 #include "chrome/common/safe_browsing/client_model.pb.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/test/test_browser_thread.h"
 #include "crypto/sha2.h"
 #include "net/http/http_status_code.h"
@@ -62,7 +65,15 @@ ACTION(QuitCurrentMessageLoop) {
 
 class ClientSideDetectionServiceTest : public testing::Test {
  protected:
+  ClientSideDetectionServiceTest() {
+    // Needed to set the singlton.
+    field_trials_.reset(new base::FieldTrialList(NULL));
+  }
+
   void SetUp() override {
+    variations::testing::ClearAllVariationIDs();
+    variations::testing::ClearAllVariationParams();
+
     file_thread_.reset(new content::TestBrowserThread(BrowserThread::FILE,
                                                       &msg_loop_));
 
@@ -106,11 +117,35 @@ class ClientSideDetectionServiceTest : public testing::Test {
     return is_malware_;
   }
 
+  std::string MakeModelUrl() {
+    return ClientSideDetectionService::kClientModelUrlPrefix +
+           ClientSideDetectionService::MakeModelName();
+  }
+
+  // Set up the finch experiment to control the model number
+  // used in the model URL.
+  void SetFinchModelNumber(int model_number) {
+    variations::testing::ClearAllVariationIDs();
+    variations::testing::ClearAllVariationParams();
+
+    const std::string group_name = "ModelFoo";  // Not used in CSD code.
+    base::FieldTrialList::CreateFieldTrial(
+        ClientSideDetectionService::kClientModelFinchExperiment, group_name);
+
+    std::map<std::string, std::string> params;
+    params[ClientSideDetectionService::kClientModelFinchParam] =
+        base::IntToString(model_number);
+
+    variations::AssociateVariationParams(
+        ClientSideDetectionService::kClientModelFinchExperiment, group_name,
+        params);
+  }
+
   void SetModelFetchResponse(std::string response_data,
                              net::HttpStatusCode response_code,
                              net::URLRequestStatus::Status status) {
-    factory_->SetFakeResponse(GURL(ClientSideDetectionService::kClientModelUrl),
-                              response_data, response_code, status);
+    factory_->SetFakeResponse(GURL(MakeModelUrl()), response_data,
+                              response_code, status);
   }
 
   void SetClientReportPhishingResponse(std::string response_data,
@@ -244,6 +279,7 @@ class ClientSideDetectionServiceTest : public testing::Test {
 
   scoped_ptr<content::TestBrowserThread> browser_thread_;
   scoped_ptr<content::TestBrowserThread> file_thread_;
+  scoped_ptr<base::FieldTrialList> field_trials_;
 
   GURL phishing_url_;
   GURL confirmed_malware_url_;
@@ -372,6 +408,49 @@ TEST_F(ClientSideDetectionServiceTest, FetchModelTest) {
   Mock::VerifyAndClearExpectations(&service);
 }
 
+TEST_F(ClientSideDetectionServiceTest, ModelNamesTest) {
+  // Test the name-templating.
+  EXPECT_EQ(ClientSideDetectionService::FillInModelName(true, 3),
+            "client_model_v5_ext_variation_3.pb");
+  EXPECT_EQ(ClientSideDetectionService::FillInModelName(false, 5),
+            "client_model_v5_variation_5.pb");
+
+  // Use Finch.  Should default to 0.
+  EXPECT_EQ(ClientSideDetectionService::MakeModelName(),
+            "client_model_v5_variation_0.pb");
+
+  SetFinchModelNumber(2);
+  EXPECT_EQ(ClientSideDetectionService::MakeModelName(),
+            "client_model_v5_variation_2.pb");
+
+  SetFinchModelNumber(0);
+  EXPECT_EQ(ClientSideDetectionService::MakeModelName(),
+            "client_model_v5_variation_0.pb");
+}
+
+// Like FetchModelTest, but we vary the finch params.
+TEST_F(ClientSideDetectionServiceTest, FetchExperimentalModelTest) {
+  MockClientSideDetectionService service;
+  EXPECT_CALL(service, ScheduleFetchModel(_)).Times(1);
+  service.SetEnabledAndRefreshState(true);
+
+  ClientSideModel model;
+  model.set_max_words_per_term(0);
+  model.set_version(10);
+  model.add_hashes("bla");
+  model.add_page_term(0);
+
+  SetFinchModelNumber(1);
+  SetModelFetchResponse(model.SerializeAsString(), net::HTTP_OK,
+                        net::URLRequestStatus::SUCCESS);
+  EXPECT_CALL(service, EndFetchModel(
+      ClientSideDetectionService::MODEL_SUCCESS))
+      .WillOnce(QuitCurrentMessageLoop());
+  service.StartFetchModel();
+  msg_loop_.Run();  // EndFetchModel will quit the message loop.
+  Mock::VerifyAndClearExpectations(&service);
+}
+
 TEST_F(ClientSideDetectionServiceTest, ServiceObjectDeletedBeforeCallbackDone) {
   SetModelFetchResponse("bogus model", net::HTTP_OK,
                         net::URLRequestStatus::SUCCESS);
diff --git a/chrome/common/safe_browsing/csd.proto b/chrome/common/safe_browsing/csd.proto
index 03a4beb..0167cfa 100644
--- a/chrome/common/safe_browsing/csd.proto
+++ b/chrome/common/safe_browsing/csd.proto
@@ -15,6 +15,18 @@ option optimize_for = LITE_RUNTIME;
 
 package safe_browsing;
 
+// Protocol buffer describing the Chrome user population of the user reporting
+// data.
+message ChromeUserPopulation {
+  enum UserPopulation {
+    UNKNOWN_USER_POPULATION = 0;
+    SAFE_BROWSING = 1;
+    EXTENDED_REPORTING = 2;
+  }
+  optional UserPopulation user_population = 1;
+}
+
+
 message ClientPhishingRequest {
   // URL that the client visited.  The CGI parameters are stripped by the
   // client.
@@ -71,6 +83,12 @@ message ClientPhishingRequest {
 
   // List of shingle hashes we extracted.
   repeated uint32 shingle_hashes = 12 [packed = true];
+
+  // The model filename (basename) that was used by the client.
+  optional string model_filename = 13;
+
+  // Population that the reporting user is part of.
+  optional ChromeUserPopulation population = 14;
 }
 
 message ClientPhishingResponse {
@@ -117,6 +135,9 @@ message ClientMalwareRequest {
 
   // List of resource urls that match the malware IP list.
   repeated UrlInfo bad_ip_url_info = 7;
+
+  // Population that the reporting user is part of.
+  optional ChromeUserPopulation population = 9;
 }
 
 message ClientMalwareResponse {
@@ -278,6 +299,9 @@ message ClientDownloadRequest {
   }
 
   repeated ArchivedBinary archived_binary = 22;
+
+  // Population that the reporting user is part of.
+  optional ChromeUserPopulation population = 24;
 }
 
 message ClientDownloadResponse {
@@ -490,6 +514,9 @@ message ClientIncidentReport {
   }
 
   optional EnvironmentData environment = 3;
+
+  // Population that the reporting user is part of.
+  optional ChromeUserPopulation population = 7;
 }
 
 message ClientIncidentResponse {