Download cleanup tool based on experiment config and OS bitness

chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc

Index: chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc
diff --git a/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc b/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc
index a394077e24752dee0a91919730628f4bb1d5265d..32f4020aa6c0e4db8eceb5fc701858a9bd785409 100644
--- a/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc
+++ b/chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.cc
@@ -7,7 +7,9 @@
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
+#include "base/win/windows_version.h"
 #include "components/variations/variations_associated_data.h"
+#include "url/origin.h"
 
 namespace {
 
@@ -23,6 +25,9 @@ const char kSRTElevationAsNeededGroup[] = "AsNeeded";
 const char kSRTReporterTrial[] = "srt_reporter";
 const char kSRTReporterOffGroup[] = "Off";
 
+const char kDownloadRootPath[] =
+    "https://dl.google.com/dl/softwareremovaltool/win/";
+
 // The download links of the Software Removal Tool.
 const char kMainSRTDownloadURL[] =
     "https://dl.google.com/dl"
@@ -55,12 +60,41 @@ bool IsSwReporterEnabled() {
       kSRTReporterOffGroup, base::CompareCase::SENSITIVE);
 }
 
-const char* GetSRTDownloadURL() {
+GURL GetLegacyDownloadURL() {
   if (base::StartsWith(base::FieldTrialList::FindFullName(kSRTPromptTrial),
                        kSRTCanaryGroup, base::CompareCase::SENSITIVE)) {
-    return kCanarySRTDownloadURL;
+    return GURL(kCanarySRTDownloadURL);
   }
-  return kMainSRTDownloadURL;
+  return GURL(kMainSRTDownloadURL);
+}
+
+GURL GetSRTDownloadURL() {
+  constexpr char kDownloadGroupParam[] = "download_group";
+  const std::string download_group =
+      variations::GetVariationParamValue(kSRTPromptTrial, kDownloadGroupParam);
+  if (download_group.empty())
+    return GetLegacyDownloadURL();
+
+  const char* architecture = base::win::OSInfo::GetInstance()->architecture() ==

[csharp, 2017/06/14 20:14:29]

why not string instead of char*?

[veranika, 2017/06/15 18:00:19]

Done.

+                                     base::win::OSInfo::X86_ARCHITECTURE
+                                 ? "x86"
+                                 : "x64";
+
+  // Construct download URL using the following pattern:
+  // https://dl.google.com/.../win/{arch}/{group}/chrome_cleanup_tool.exe
+  std::string download_url_str = std::string(kDownloadRootPath) + architecture +
+                                 "/" + download_group +
+                                 "/chrome_cleanup_tool.exe?chrome-prompt=1";
+  GURL download_url(download_url_str);
+
+  // Ensure URL construction didn't change origin.
+  const GURL kDownloadRoot(kDownloadRootPath);

[csharp, 2017/06/14 20:14:29]

nit: I think since these are just const for this call (instead of program life),
the naming scheme should just be download_root and known_good_origin (or at
least that what I've generally seen)

[veranika, 2017/06/15 18:00:20]

Done.

+  const url::Origin kKnownGoodOrigin(kDownloadRoot);
+  url::Origin current_origin(download_url);
+  if (!current_origin.IsSameOriginWith(kKnownGoodOrigin))
+    return GetLegacyDownloadURL();
+
+  return download_url;
 }
 
 std::string GetIncomingSRTSeed() {