Switch to the new ExternalConstantsOverrider.

external_constants.cc now uses ExternalConstantsOverrider to load overrides
rather than DevOverridesProvider. Tests have been updated to use
ExternalConstantsBuilder or write the override file directly, whichever
is more appropriate.

Bug: 1154901
Change-Id: I35383e25058c1799f99583128b5e695cbf90ac1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2634264Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: Adam Norberg <norberg@google.com>
Cr-Commit-Position: refs/heads/master@{#845352}

chrome/updater/BUILD.gn

@@ -115,7 +115,6 @@ if (is_win || is_mac) {
       "external_constants.h",
       "external_constants_builder.cc",
       "external_constants_builder.h",
-      "external_constants_impl.h",
       "external_constants_override.cc",
       "external_constants_override.h",
       "installer.cc",
@@ -153,7 +152,6 @@ if (is_win || is_mac) {
         "app/server/mac/service_protocol.mm",
         "app/server/mac/update_service_wrappers.h",
         "app/server/mac/update_service_wrappers.mm",
-        "external_constants_mac.mm",
         "installer_mac.cc",
         "launchd_util.cc",
         "launchd_util.h",
@@ -180,7 +178,6 @@ if (is_win || is_mac) {
         "app/server/win/server.h",
         "app/server/win/service_main.cc",
         "app/server/win/service_main.h",
-        "external_constants_win.cc",
         "lib_util_win.cc",
         "prefs_win.cc",
         "service_factory_win.cc",
@@ -336,8 +333,6 @@ if (is_win || is_mac) {
       "enum_traits_unittest.cc",
       "external_constants_builder_unittest.cc",
       "external_constants_override_unittest.cc",
-      "external_constants_unittest.cc",
-      "external_constants_unittest.h",
       "lib_util_unittest.cc",
       "persisted_data_unittest.cc",
       "policy_manager_unittest.cc",
@@ -378,10 +373,7 @@ if (is_win || is_mac) {
     ]
 
     if (is_win) {
-      sources += [
-        "external_constants_win_unittest.cc",
-        "test/integration_tests_win.cc",
-      ]
+      sources += [ "test/integration_tests_win.cc" ]
 
       deps += [
         "//chrome/updater/app/server/win:updater_idl_idl",
@@ -399,7 +391,6 @@ if (is_win || is_mac) {
 
     if (is_mac) {
       sources += [
-        "external_constants_mac_unittest.mm",
         "mac/scoped_xpc_service_mock.h",
         "mac/scoped_xpc_service_mock.mm",
         "mac/update_service_proxy_test.mm",

chrome/updater/external_constants.cc

@@ -3,9 +3,9 @@
 // found in the LICENSE file.
 
 #include "chrome/updater/external_constants.h"
-#include "chrome/updater/external_constants_impl.h"
 
 #include "chrome/updater/constants.h"
+#include "chrome/updater/external_constants_override.h"
 #include "chrome/updater/updater_branding.h"
 #include "url/gurl.h"
 
@@ -37,16 +37,15 @@ ExternalConstants::ExternalConstants(
 ExternalConstants::~ExternalConstants() = default;
 
 std::unique_ptr<ExternalConstants> CreateExternalConstants() {
-  return std::make_unique<DevOverrideProvider>(
+  std::unique_ptr<ExternalConstants> overrider =
+      ExternalConstantsOverrider::FromDefaultJSONFile(
           std::make_unique<DefaultExternalConstants>());
+  return overrider ? std::move(overrider)
+                   : std::make_unique<DefaultExternalConstants>();
 }
 
 std::unique_ptr<ExternalConstants> CreateDefaultExternalConstantsForTesting() {
   return std::make_unique<DefaultExternalConstants>();
 }
 
-DevOverrideProvider::DevOverrideProvider(
-    std::unique_ptr<ExternalConstants> next_provider)
-    : ExternalConstants(std::move(next_provider)) {}
-
 }  // namespace updater

chrome/updater/external_constants_impl.h

-// Copyright 2020 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 CHROME_UPDATER_EXTERNAL_CONSTANTS_IMPL_H_
-#define CHROME_UPDATER_EXTERNAL_CONSTANTS_IMPL_H_
-
-#include "chrome/updater/external_constants.h"
-
-class GURL;
-
-namespace updater {
-
-class DevOverrideProvider : public ExternalConstants {
- public:
-  explicit DevOverrideProvider(
-      std::unique_ptr<ExternalConstants> next_provider);
-  ~DevOverrideProvider() override = default;
-
-  // Overrides of ExternalConstants:
-  std::vector<GURL> UpdateURL() const override;
-  bool UseCUP() const override;
-  double InitialDelay() const override;
-};
-
-}  // namespace updater
-
-#endif  // CHROME_UPDATER_EXTERNAL_CONSTANTS_IMPL_H_

chrome/updater/external_constants_mac.mm

-// Copyright 2020 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 "chrome/updater/external_constants.h"
-
-#import <Foundation/Foundation.h>
-
-#include "base/mac/scoped_nsobject.h"
-#include "base/strings/sys_string_conversions.h"
-#include "chrome/updater/constants.h"
-#include "chrome/updater/external_constants_impl.h"
-#include "url/gurl.h"
-
-#include "base/logging.h"
-
-namespace updater {
-
-std::vector<GURL> DevOverrideProvider::UpdateURL() const {
-  @autoreleasepool {
-    NSUserDefaults* userDefaults = [[NSUserDefaults alloc]
-        initWithSuiteName:[NSString
-                              stringWithUTF8String:kUserDefaultsSuiteName]];
-    NSURL* url = [userDefaults
-        URLForKey:[NSString stringWithUTF8String:kDevOverrideKeyUrl]];
-    if (url == nil)
-      return next_provider_->UpdateURL();
-    return {GURL(base::SysNSStringToUTF8([url absoluteString]))};
-  }
-}
-
-bool DevOverrideProvider::UseCUP() const {
-  @autoreleasepool {
-    NSUserDefaults* userDefaults = [[NSUserDefaults alloc]
-        initWithSuiteName:[NSString
-                              stringWithUTF8String:kUserDefaultsSuiteName]];
-    id use_cup = [userDefaults
-        objectForKey:[NSString stringWithUTF8String:kDevOverrideKeyUseCUP]];
-    if (use_cup)
-      return [use_cup boolValue];
-    return next_provider_->UseCUP();
-  }
-}
-
-double DevOverrideProvider::InitialDelay() const {
-  @autoreleasepool {
-    NSUserDefaults* userDefaults = [[NSUserDefaults alloc]
-        initWithSuiteName:[NSString
-                              stringWithUTF8String:kUserDefaultsSuiteName]];
-    id initial_delay = [userDefaults
-        objectForKey:[NSString
-                         stringWithUTF8String:kDevOverrideKeyInitialDelay]];
-    if (initial_delay)
-      return [initial_delay integerValue];
-    return next_provider_->InitialDelay();
-  }
-}
-
-}  // namespace updater

chrome/updater/external_constants_mac_unittest.mm

-// Copyright 2020 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 "chrome/updater/external_constants_unittest.h"
-
-#import <Foundation/Foundation.h>
-#include <memory>
-
-#include "base/mac/scoped_nsobject.h"
-#include "chrome/updater/constants.h"
-#include "chrome/updater/external_constants.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "url/gurl.h"
-
-namespace updater {
-
-namespace {
-
-void ClearUserDefaults() {
-  @autoreleasepool {
-    NSUserDefaults* userDefaults = [[NSUserDefaults alloc]
-        initWithSuiteName:[NSString
-                              stringWithUTF8String:kUserDefaultsSuiteName]];
-    [userDefaults
-        removeObjectForKey:[NSString stringWithUTF8String:kDevOverrideKeyUrl]];
-    [userDefaults
-        removeObjectForKey:[NSString
-                               stringWithUTF8String:kDevOverrideKeyUseCUP]];
-    [userDefaults
-        removeObjectForKey:
-            [NSString stringWithUTF8String:kDevOverrideKeyInitialDelay]];
-  }
-}
-
-}  // namespace
-
-void DevOverrideTest::SetUp() {
-  ClearUserDefaults();
-}
-
-void DevOverrideTest::TearDown() {
-  ClearUserDefaults();
-}
-
-TEST_F(DevOverrideTest, TestDevOverrides) {
-  std::unique_ptr<ExternalConstants> consts = CreateExternalConstants();
-
-  @autoreleasepool {
-    NSUserDefaults* userDefaults = [[NSUserDefaults alloc]
-        initWithSuiteName:[NSString
-                              stringWithUTF8String:kUserDefaultsSuiteName]];
-    [userDefaults setURL:[NSURL URLWithString:@"http://localhost:8080"]
-                  forKey:[NSString stringWithUTF8String:kDevOverrideKeyUrl]];
-    [userDefaults
-        setBool:NO
-         forKey:[NSString stringWithUTF8String:kDevOverrideKeyUseCUP]];
-    [userDefaults
-        setInteger:0
-            forKey:[NSString stringWithUTF8String:kDevOverrideKeyInitialDelay]];
-  }
-
-  EXPECT_FALSE(consts->UseCUP());
-  std::vector<GURL> urls = consts->UpdateURL();
-  ASSERT_EQ(urls.size(), 1u);
-  EXPECT_EQ(urls[0], GURL("http://localhost:8080"));
-  ASSERT_TRUE(urls[0].is_valid());
-  ASSERT_EQ(consts->InitialDelay(), 0);
-}
-
-}  // namespace updater

chrome/updater/external_constants_unittest.cc

-// Copyright 2020 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 "chrome/updater/external_constants_unittest.h"
-
-#include <memory>
-
-#include "chrome/updater/constants.h"
-#include "chrome/updater/external_constants.h"
-#include "chrome/updater/updater_branding.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "url/gurl.h"
-
-namespace updater {
-
-TEST_F(DevOverrideTest, TestDefaults) {
-  std::unique_ptr<ExternalConstants> consts = CreateExternalConstants();
-  EXPECT_TRUE(consts->UseCUP());
-  std::vector<GURL> urls = consts->UpdateURL();
-  ASSERT_EQ(urls.size(), 1ul);
-  EXPECT_EQ(urls[0], GURL(UPDATE_CHECK_URL));
-  EXPECT_TRUE(urls[0].is_valid());
-}
-
-}  // namespace updater

chrome/updater/external_constants_unittest.h

-// Copyright 2020 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 CHROME_UPDATER_EXTERNAL_CONSTANTS_UNITTEST_H_
-#define CHROME_UPDATER_EXTERNAL_CONSTANTS_UNITTEST_H_
-
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace updater {
-
-class DevOverrideTest : public ::testing::Test {
- protected:
-  void SetUp() override;
-  void TearDown() override;
-};
-
-}  // namespace updater
-
-#endif  // CHROME_UPDATER_EXTERNAL_CONSTANTS_UNITTEST_H_

chrome/updater/external_constants_win.cc

-// Copyright 2020 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 "chrome/updater/external_constants.h"
-
-#include "base/strings/utf_string_conversions.h"
-#include "base/win/registry.h"
-#include "chrome/updater/constants.h"
-#include "chrome/updater/external_constants_impl.h"
-#include "chrome/updater/win/constants.h"
-#include "url/gurl.h"
-
-namespace updater {
-
-std::vector<GURL> DevOverrideProvider::UpdateURL() const {
-  base::win::RegKey key;
-  if (key.Open(HKEY_CURRENT_USER, UPDATE_DEV_KEY, KEY_READ) == ERROR_SUCCESS) {
-    base::string16 url;
-    if (key.ReadValue(base::UTF8ToUTF16(kDevOverrideKeyUrl).c_str(), &url) ==
-        ERROR_SUCCESS)
-      return {GURL(url)};
-  }
-  return next_provider_->UpdateURL();
-}
-
-bool DevOverrideProvider::UseCUP() const {
-  base::win::RegKey key;
-  if (key.Open(HKEY_CURRENT_USER, UPDATE_DEV_KEY, KEY_READ) == ERROR_SUCCESS) {
-    DWORD use_cup = 0;
-    if (key.ReadValueDW(base::UTF8ToUTF16(kDevOverrideKeyUseCUP).c_str(),
-                        &use_cup) == ERROR_SUCCESS) {
-      if (use_cup == 0)
-        return false;
-      else if (use_cup == 1)
-        return true;
-    }
-  }
-  return next_provider_->UseCUP();
-}
-
-double DevOverrideProvider::InitialDelay() const {
-  base::win::RegKey key;
-  if (key.Open(HKEY_CURRENT_USER, UPDATE_DEV_KEY, KEY_READ) == ERROR_SUCCESS) {
-    DWORD initial_delay = 0;
-    if (key.ReadValueDW(base::UTF8ToUTF16(kDevOverrideKeyInitialDelay).c_str(),
-                        &initial_delay) == ERROR_SUCCESS)
-      return initial_delay;
-  }
-  return next_provider_->InitialDelay();
-}
-
-}  // namespace updater

chrome/updater/external_constants_win_unittest.cc

-// Copyright 2020 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 "chrome/updater/external_constants_unittest.h"
-
-#include <memory>
-
-#include "base/strings/utf_string_conversions.h"
-#include "base/win/registry.h"
-#include "chrome/updater/constants.h"
-#include "chrome/updater/external_constants.h"
-#include "chrome/updater/win/constants.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "url/gurl.h"
-
-namespace updater {
-
-namespace {
-
-void ClearUserDefaults() {
-  base::win::RegKey key(HKEY_CURRENT_USER, L"", KEY_SET_VALUE);
-  key.DeleteKey(UPDATE_DEV_KEY);
-}
-
-}  // namespace
-
-void DevOverrideTest::SetUp() {
-  ClearUserDefaults();
-}
-
-void DevOverrideTest::TearDown() {
-  ClearUserDefaults();
-}
-
-TEST_F(DevOverrideTest, TestDevOverrides) {
-  std::unique_ptr<ExternalConstants> consts = CreateExternalConstants();
-
-  base::win::RegKey key;
-  const base::char16 val[] = L"http://localhost:8080";
-  ASSERT_EQ(key.Create(HKEY_CURRENT_USER, UPDATE_DEV_KEY, KEY_WRITE),
-            ERROR_SUCCESS);
-  ASSERT_EQ(key.WriteValue(base::UTF8ToUTF16(kDevOverrideKeyUrl).c_str(), val),
-            ERROR_SUCCESS);
-  DWORD use_cup = 0;
-  ASSERT_EQ(
-      key.WriteValue(base::UTF8ToUTF16(kDevOverrideKeyUseCUP).c_str(), use_cup),
-      ERROR_SUCCESS);
-  DWORD initial_delay = 1;
-  ASSERT_EQ(
-      key.WriteValue(base::UTF8ToUTF16(kDevOverrideKeyInitialDelay).c_str(),
-                     initial_delay),
-      ERROR_SUCCESS);
-
-  ASSERT_FALSE(consts->UseCUP());
-  std::vector<GURL> urls = consts->UpdateURL();
-  ASSERT_EQ(urls.size(), 1u);
-  ASSERT_EQ(urls[0], GURL("http://localhost:8080"));
-  ASSERT_TRUE(urls[0].is_valid());
-  ASSERT_EQ(consts->InitialDelay(), 1);
-}
-
-}  // namespace updater

chrome/updater/test/integration_tests_mac.mm

@@ -4,6 +4,7 @@
 
 #include <stdint.h>
 #include <string>
+#include <vector>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ -16,6 +17,7 @@
 #include "base/version.h"
 #include "chrome/common/mac/launchd.h"
 #include "chrome/updater/constants.h"
+#include "chrome/updater/external_constants_builder.h"
 #include "chrome/updater/launchd_util.h"
 #import "chrome/updater/mac/util.h"
 #include "chrome/updater/mac/xpc_service_names.h"
@@ -102,20 +104,11 @@ void ExpectServiceAbsent(const std::string& service) {
 #endif  // defined(COMPONENT_BUILD
 
 void EnterTestMode(const GURL& url) {
-  @autoreleasepool {
-    NSUserDefaults* userDefaults = [[NSUserDefaults alloc]
-        initWithSuiteName:[NSString
-                              stringWithUTF8String:kUserDefaultsSuiteName]];
-    [userDefaults
-        setURL:[NSURL URLWithString:base::SysUTF8ToNSString(url.spec())]
-        forKey:[NSString stringWithUTF8String:kDevOverrideKeyUrl]];
-    [userDefaults
-        setBool:NO
-         forKey:[NSString stringWithUTF8String:kDevOverrideKeyUseCUP]];
-    [userDefaults
-        setInteger:0
-            forKey:[NSString stringWithUTF8String:kDevOverrideKeyInitialDelay]];
-  }
+  ASSERT_TRUE(ExternalConstantsBuilder()
+                  .SetUpdateURL(std::vector<std::string>{url.spec()})
+                  .SetUseCUP(false)
+                  .SetInitialDelay(0)
+                  .Overwrite());
 }
 
 // crbug.com/1112527: These tests are not compatible with component build.
@@ -140,15 +133,6 @@ void Clean() {
   EXPECT_TRUE(base::DeletePathRecursively(GetDataDirPath()));
 
   @autoreleasepool {
-    NSUserDefaults* userDefaults = [[NSUserDefaults alloc]
-        initWithSuiteName:[NSString
-                              stringWithUTF8String:kUserDefaultsSuiteName]];
-    [userDefaults
-        removeObjectForKey:[NSString stringWithUTF8String:kDevOverrideKeyUrl]];
-    [userDefaults
-        removeObjectForKey:[NSString
-                               stringWithUTF8String:kDevOverrideKeyUseCUP]];
-
     // TODO(crbug.com/1096654): support machine case (Launchd::Domain::Local and
     // Launchd::Type::Daemon).
     RemoveJobFromLaunchd(Launchd::Domain::User, Launchd::Type::Agent,

chrome/updater/test/integration_tests_win.cc

@@ -4,6 +4,7 @@
 
 #include <wrl/client.h>
 #include <string>
+#include <vector>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ -21,6 +22,7 @@
 #include "chrome/updater/app/server/win/updater_internal_idl.h"
 #include "chrome/updater/app/server/win/updater_legacy_idl.h"
 #include "chrome/updater/constants.h"
+#include "chrome/updater/external_constants_builder.h"
 #include "chrome/updater/test/integration_tests.h"
 #include "chrome/updater/updater_branding.h"
 #include "chrome/updater/updater_version.h"
@@ -75,8 +77,6 @@ base::FilePath GetDataDirPath() {
 
 void Clean() {
   // TODO(crbug.com/1062288): Delete the Client / ClientState registry keys.
-  base::win::RegKey(HKEY_CURRENT_USER, L"", KEY_SET_VALUE)
-      .DeleteKey(UPDATE_DEV_KEY);
   // TODO(crbug.com/1062288): Delete the COM server items.
   // TODO(crbug.com/1062288): Delete the COM service items.
   // TODO(crbug.com/1062288): Delete the COM interfaces.
@@ -100,19 +100,11 @@ void ExpectClean() {
 }
 
 void EnterTestMode(const GURL& url) {
-  base::win::RegKey key(HKEY_CURRENT_USER, L"", KEY_SET_VALUE);
-  ASSERT_EQ(key.Create(HKEY_CURRENT_USER, UPDATE_DEV_KEY, KEY_WRITE),
-            ERROR_SUCCESS);
-  ASSERT_EQ(key.WriteValue(base::UTF8ToUTF16(kDevOverrideKeyUrl).c_str(),
-                           base::UTF8ToUTF16(url.spec()).c_str()),
-            ERROR_SUCCESS);
-  ASSERT_EQ(key.WriteValue(base::UTF8ToUTF16(kDevOverrideKeyUseCUP).c_str(),
-                           DWORD{0}),
-            ERROR_SUCCESS);
-  ASSERT_EQ(
-      key.WriteValue(base::UTF8ToUTF16(kDevOverrideKeyInitialDelay).c_str(),
-                     DWORD{0}),
-      ERROR_SUCCESS);
+  ASSERT_TRUE(ExternalConstantsBuilder()
+                  .SetUpdateURL(std::vector<std::string>{url.spec()})
+                  .SetUseCUP(false)
+                  .SetInitialDelay(0)
+                  .Overwrite());
 }
 
 void ExpectInstalled() {