Fix GN builds to include build and patch in Chrome-Proxy header.

Previously, GN builds would not properly include the Chromium version
build and patch numbers in the Chrome-Proxy header sent to the Data
Reduction Proxy. This CL fixes this, and adds assertions and tests to
ensure that the build and patch numbers are always present.

BUG=595471

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

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

6 files changed, 70 insertions, 100 deletions

diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc
index ca7c7e9..5c65d708 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc
@@ -4,13 +4,10 @@
 
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h"
 
-#include <algorithm>
-#include <map>
-#include <vector>
-
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_tokenizer.h"
@@ -18,6 +15,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "base/version.h"
 #include "build/build_config.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_client_config_parser.h"
@@ -43,6 +41,46 @@ std::string FormatOption(const std::string& name, const std::string& value) {
   return name + "=" + value;
 }
 
+// Returns the version of Chromium that is being used, e.g. "1.2.3.4".
+const char* ChromiumVersion() {
+  // Assert at compile time that the Chromium version is at least somewhat
+  // properly formed, e.g. the version string is at least as long as "0.0.0.0",
+  // and starts and ends with numeric digits. This is to prevent another
+  // regression like http://crbug.com/595471.
+  static_assert(arraysize(PRODUCT_VERSION) >= arraysize("0.0.0.0") &&
+                    '0' <= PRODUCT_VERSION[0] && PRODUCT_VERSION[0] <= '9' &&
+                    '0' <= PRODUCT_VERSION[arraysize(PRODUCT_VERSION) - 2] &&
+                    PRODUCT_VERSION[arraysize(PRODUCT_VERSION) - 2] <= '9',
+                "PRODUCT_VERSION must be a string of the form "
+                "'MAJOR.MINOR.BUILD.PATCH', e.g. '1.2.3.4'. "
+                "PRODUCT_VERSION='" PRODUCT_VERSION "' is badly formed.");
+
+  return PRODUCT_VERSION;
+}
+
+// Returns the build and patch numbers of |version_string|. |version_string|
+// must be a properly formed Chromium version number, e.g. "1.2.3.4".
+void GetChromiumBuildAndPatch(const std::string& version_string,
+                              std::string* build,
+                              std::string* patch) {
+  base::Version version(version_string);
+  DCHECK(version.IsValid());
+  DCHECK_EQ(4U, version.components().size());
+
+  *build = base::Uint64ToString(version.components()[2]);
+  *patch = base::Uint64ToString(version.components()[3]);
+}
+
+#define CLIENT_ENUM(name, str_value) \
+  case name:                         \
+    return str_value;
+const char* GetString(Client client) {
+  switch (client) { CLIENT_ENUMS_LIST }
+  NOTREACHED();
+  return "";
+}
+#undef CLIENT_ENUM
+
 }  // namespace
 
 const char kSessionHeaderOption[] = "ps";
@@ -59,17 +97,6 @@ const char kExperimentsOption[] = "exp";
 const char kAndroidWebViewProtocolVersion[] = "";
 #endif
 
-#define CLIENT_ENUM(name, str_value) \
-    case name: return str_value;
-const char* GetString(Client client) {
-  switch (client) {
-    CLIENT_ENUMS_LIST
-  }
-  NOTREACHED();
-  return "";
-}
-#undef CLIENT_ENUM
-
 // static
 bool DataReductionProxyRequestOptions::IsKeySetOnCommandLine() {
   const base::CommandLine& command_line =
@@ -81,14 +108,7 @@ bool DataReductionProxyRequestOptions::IsKeySetOnCommandLine() {
 DataReductionProxyRequestOptions::DataReductionProxyRequestOptions(
     Client client,
     DataReductionProxyConfig* config)
-    : client_(GetString(client)),
-      use_assigned_credentials_(false),
-      data_reduction_proxy_config_(config) {
-  DCHECK(data_reduction_proxy_config_);
-  GetChromiumBuildAndPatch(ChromiumVersion(), &build_, &patch_);
-  // Constructed on the UI thread, but should be checked on the IO thread.
-  thread_checker_.DetachFromThread();
-}
+    : DataReductionProxyRequestOptions(client, ChromiumVersion(), config) {}
 
 DataReductionProxyRequestOptions::DataReductionProxyRequestOptions(
     Client client,
@@ -109,35 +129,9 @@ DataReductionProxyRequestOptions::~DataReductionProxyRequestOptions() {
 void DataReductionProxyRequestOptions::Init() {
   key_ = GetDefaultKey(),
   UpdateCredentials();
-  UpdateVersion();
   UpdateExperiments();
 }
 
-std::string DataReductionProxyRequestOptions::ChromiumVersion() const {
-#if defined(PRODUCT_VERSION)
-  return PRODUCT_VERSION;
-#else
-  return std::string();
-#endif
-}
-
-void DataReductionProxyRequestOptions::GetChromiumBuildAndPatch(
-    const std::string& version,
-    std::string* build,
-    std::string* patch) const {
-  std::vector<base::StringPiece> version_parts = base::SplitStringPiece(
-      version, ".", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
-  if (version_parts.size() != 4)
-    return;
-  version_parts[2].CopyToString(build);
-  version_parts[3].CopyToString(patch);
-}
-
-void DataReductionProxyRequestOptions::UpdateVersion() {
-  GetChromiumBuildAndPatch(version_, &build_, &patch_);
-  RegenerateRequestHeaderValue();
-}
-
 void DataReductionProxyRequestOptions::UpdateExperiments() {
   std::string experiments =
       base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
@@ -321,10 +315,13 @@ void DataReductionProxyRequestOptions::RegenerateRequestHeaderValue() {
   }
   if (!client_.empty())
     headers.push_back(FormatOption(kClientHeaderOption, client_));
-  if (!build_.empty() && !patch_.empty()) {
-    headers.push_back(FormatOption(kBuildNumberHeaderOption, build_));
-    headers.push_back(FormatOption(kPatchNumberHeaderOption, patch_));
-  }
+
+  DCHECK(!build_.empty());
+  headers.push_back(FormatOption(kBuildNumberHeaderOption, build_));
+
+  DCHECK(!patch_.empty());
+  headers.push_back(FormatOption(kPatchNumberHeaderOption, patch_));
+
   for (const auto& experiment : experiments_)
     headers.push_back(FormatOption(kExperimentsOption, experiment));
 
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h
index 7ab5e36..5565596 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h
@@ -139,18 +139,6 @@ class DataReductionProxyRequestOptions {
   FRIEND_TEST_ALL_PREFIXES(DataReductionProxyRequestOptionsTest,
                            AuthHashForSalt);
 
-  // Returns the version of Chromium that is being used.
-  std::string ChromiumVersion() const;
-
-  // Returns the build and patch numbers of |version|. If |version| isn't of the
-  // form xx.xx.xx.xx build and patch are not modified.
-  void GetChromiumBuildAndPatch(const std::string& version,
-                                std::string* build,
-                                std::string* patch) const;
-
-  // Updates client type, build, and patch.
-  void UpdateVersion();
-
   // Updates the value of the experiments to be run and regenerate the header if
   // necessary.
   void UpdateExperiments();
@@ -187,7 +175,6 @@ class DataReductionProxyRequestOptions {
 
   // Name of the client and version of the data reduction proxy protocol to use.
   std::string client_;
-  std::string version_;
   std::string session_;
   std::string credentials_;
   std::string secure_session_;
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc
index 5e5af4d..310ffc9 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc
@@ -38,7 +38,6 @@ const char kOtherProxy[] = "testproxy:17";
 const char kVersion[] = "0.1.2.3";
 const char kExpectedBuild[] = "2";
 const char kExpectedPatch[] = "3";
-const char kBogusVersion[] = "0.0";
 const char kExpectedCredentials[] = "96bd72ec4a050ba60981743d41787768";
 const char kExpectedSession[] = "0-1633771873-1633771873-1633771873";
 
@@ -114,14 +113,13 @@ void SetHeaderExpectations(const std::string& session,
     expected_options.push_back(
         std::string(kClientHeaderOption) + "=" + client);
   }
-  if (!build.empty()) {
-    expected_options.push_back(
-        std::string(kBuildNumberHeaderOption) + "=" + build);
-  }
-  if (!patch.empty()) {
-    expected_options.push_back(
-        std::string(kPatchNumberHeaderOption) + "=" + patch);
-  }
+  EXPECT_FALSE(build.empty());
+  expected_options.push_back(std::string(kBuildNumberHeaderOption) + "=" +
+                             build);
+  EXPECT_FALSE(patch.empty());
+  expected_options.push_back(std::string(kPatchNumberHeaderOption) + "=" +
+                             patch);
+
   for (const auto& experiment : experiments) {
     expected_options.push_back(
         std::string(kExperimentsOption) + "=" + experiment);
@@ -258,26 +256,13 @@ TEST_F(DataReductionProxyRequestOptionsTest, AuthorizationIgnoresEmptyKey) {
   VerifyExpectedHeader(params()->DefaultOrigin(), expected_header);
 }
 
-TEST_F(DataReductionProxyRequestOptionsTest, AuthorizationBogusVersion) {
-  std::string expected_header;
-  SetHeaderExpectations(kExpectedSession2, kExpectedCredentials2, std::string(),
-                        kClientStr, std::string(), std::string(),
-                        std::vector<std::string>(), &expected_header);
-
-  CreateRequestOptions(kBogusVersion);
-
-  // Now set a key.
-  request_options()->SetKeyOnIO(kTestKey2);
-  VerifyExpectedHeader(params()->DefaultOrigin(), expected_header);
-}
-
 TEST_F(DataReductionProxyRequestOptionsTest, SecureSession) {
   std::string expected_header;
   SetHeaderExpectations(std::string(), std::string(), kSecureSession,
-                        kClientStr, std::string(), std::string(),
+                        kClientStr, kExpectedBuild, kExpectedPatch,
                         std::vector<std::string>(), &expected_header);
 
-  CreateRequestOptions(kBogusVersion);
+  CreateRequestOptions(kVersion);
   request_options()->SetSecureSession(kSecureSession);
   VerifyExpectedHeader(params()->DefaultOrigin(), expected_header);
 }
@@ -291,10 +276,10 @@ TEST_F(DataReductionProxyRequestOptionsTest, ParseExperiments) {
   expected_experiments.push_back("\"foo,bar\"");
   std::string expected_header;
   SetHeaderExpectations(kExpectedSession, kExpectedCredentials, std::string(),
-                        kClientStr, std::string(), std::string(),
+                        kClientStr, kExpectedBuild, kExpectedPatch,
                         expected_experiments, &expected_header);
 
-  CreateRequestOptions(kBogusVersion);
+  CreateRequestOptions(kVersion);
   VerifyExpectedHeader(params()->DefaultOrigin(), expected_header);
 }
 
@@ -347,10 +332,10 @@ TEST_F(DataReductionProxyRequestOptionsTest, ParseExperimentsFromFieldTrial) {
       expected_experiments.push_back(test.expected_experiment);
 
     SetHeaderExpectations(kExpectedSession, kExpectedCredentials, std::string(),
-                          kClientStr, std::string(), std::string(),
+                          kClientStr, kExpectedBuild, kExpectedPatch,
                           expected_experiments, &expected_header);
 
-    CreateRequestOptions(kBogusVersion);
+    CreateRequestOptions(kVersion);
     VerifyExpectedHeader(params()->DefaultOrigin(), expected_header);
   }
 }
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
index d1a7a72..8af0b73 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
@@ -85,10 +85,8 @@ void TestDataReductionProxyRequestOptions::set_offset(
 
 MockDataReductionProxyRequestOptions::MockDataReductionProxyRequestOptions(
     Client client,
-    const std::string& version,
     DataReductionProxyConfig* config)
-    : TestDataReductionProxyRequestOptions(client, version, config) {
-}
+    : TestDataReductionProxyRequestOptions(client, "1.2.3.4", config) {}
 
 MockDataReductionProxyRequestOptions::~MockDataReductionProxyRequestOptions() {
 }
@@ -438,8 +436,8 @@ DataReductionProxyTestContext::Builder::Build() {
   scoped_ptr<DataReductionProxyRequestOptions> request_options;
   if (use_mock_request_options_) {
     test_context_flags |= USE_MOCK_REQUEST_OPTIONS;
-    request_options.reset(new MockDataReductionProxyRequestOptions(
-        client_, std::string(), config.get()));
+    request_options.reset(
+        new MockDataReductionProxyRequestOptions(client_, config.get()));
   } else {
     request_options.reset(
         new DataReductionProxyRequestOptions(client_, config.get()));
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
index c10d794..e7fb15f 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
@@ -83,7 +83,6 @@ class MockDataReductionProxyRequestOptions
     : public TestDataReductionProxyRequestOptions {
  public:
   MockDataReductionProxyRequestOptions(Client client,
-                                       const std::string& version,
                                        DataReductionProxyConfig* config);
 
   ~MockDataReductionProxyRequestOptions();
diff --git a/components/data_reduction_proxy/core/common/BUILD.gn b/components/data_reduction_proxy/core/common/BUILD.gn
index b1951ad..0d1935d 100644
--- a/components/data_reduction_proxy/core/common/BUILD.gn
+++ b/components/data_reduction_proxy/core/common/BUILD.gn
@@ -113,4 +113,8 @@ source_set("unit_tests") {
 process_version("version_header") {
   template_file = "version.h.in"
   output = "$target_gen_dir/version.h"
+  extra_args = [
+    "-e",
+    "VERSION_FULL=\"%s.%s.%s.%s\" % (MAJOR,MINOR,BUILD,PATCH)",
+  ]
 }