Change the return type of Configurator::InitialDelay from int to double.

This allows the Configurator::InitialDelay function to return sub-second
values for unit-testing and functional-testing of //chrome/updater.

The code in the CUS service was changed to convert from double
to a TimeDelta. Also, the Configurator code was changed to parse
a literal to double.

Where possible, the type was changed from int
to double. In general, the standard conversion from int to double
is used (for instance in unit tests). It's possible that there might
be cases where a truncation from double to int occurs (because these
call sites were missed) but this should not create an issue at
runtime because the values involved in the initial delay are small.

Bug: 1144151
Change-Id: I66d00c4d4c7bdf241465129619a6c20fd8c895db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2635022Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845014}

chrome/browser/component_updater/chrome_component_updater_configurator.cc

@@ -53,7 +53,7 @@ class ChromeConfigurator : public update_client::Configurator {
                      PrefService* pref_service);
 
   // update_client::Configurator overrides.
-  int InitialDelay() const override;
+  double InitialDelay() const override;
   int NextCheckDelay() const override;
   int OnDemandDelay() const override;
   int UpdateDelay() const override;
@@ -107,7 +107,7 @@ ChromeConfigurator::ChromeConfigurator(const base::CommandLine* cmdline,
   DCHECK(pref_service_);
 }
 
-int ChromeConfigurator::InitialDelay() const {
+double ChromeConfigurator::InitialDelay() const {
   return configurator_impl_.InitialDelay();
 }
 

chrome/browser/extensions/updater/chrome_update_client_config.cc

@@ -141,7 +141,7 @@ ChromeUpdateClientConfig::ChromeUpdateClientConfig(
   DCHECK(pref_service_);
 }
 
-int ChromeUpdateClientConfig::InitialDelay() const {
+double ChromeUpdateClientConfig::InitialDelay() const {
   return impl_.InitialDelay();
 }
 

chrome/browser/extensions/updater/chrome_update_client_config.h

@@ -47,7 +47,7 @@ class ChromeUpdateClientConfig : public update_client::Configurator {
   ChromeUpdateClientConfig(content::BrowserContext* context,
                            base::Optional<GURL> url_override);
 
-  int InitialDelay() const override;
+  double InitialDelay() const override;
   int NextCheckDelay() const override;
   int OnDemandDelay() const override;
   int UpdateDelay() const override;

chrome/updater/configurator.cc

@@ -50,8 +50,8 @@ Configurator::Configurator(std::unique_ptr<UpdaterPrefs> prefs)
       patch_factory_(base::MakeRefCounted<PatcherFactory>()) {}
 Configurator::~Configurator() = default;
 
-int Configurator::InitialDelay() const {
-  return base::RandInt(0, external_constants_->InitialDelay());
+double Configurator::InitialDelay() const {
+  return base::RandDouble() * external_constants_->InitialDelay();
 }
 
 int Configurator::NextCheckDelay() const {

chrome/updater/configurator.h

@@ -40,7 +40,7 @@ class Configurator : public update_client::Configurator {
   Configurator& operator=(const Configurator&) = delete;
 
   // Configurator for update_client::Configurator.
-  int InitialDelay() const override;
+  double InitialDelay() const override;
   int NextCheckDelay() const override;
   int OnDemandDelay() const override;
   int UpdateDelay() const override;

chrome/updater/external_constants.cc

@@ -25,7 +25,7 @@ class DefaultExternalConstants : public ExternalConstants {
 
   bool UseCUP() const override { return true; }
 
-  int InitialDelay() const override { return kInitialDelay; }
+  double InitialDelay() const override { return kInitialDelay; }
 };
 
 }  // namespace

chrome/updater/external_constants.h

@@ -29,7 +29,7 @@ class ExternalConstants {
 
   // Number of seconds to delay the start of the automated background tasks
   // such as update checks.
-  virtual int InitialDelay() const = 0;
+  virtual double InitialDelay() const = 0;
 
  protected:
   std::unique_ptr<ExternalConstants> next_provider_;

chrome/updater/external_constants_impl.h

@@ -20,7 +20,7 @@ class DevOverrideProvider : public ExternalConstants {
   // Overrides of ExternalConstants:
   std::vector<GURL> UpdateURL() const override;
   bool UseCUP() const override;
-  int InitialDelay() const override;
+  double InitialDelay() const override;
 };
 
 }  // namespace updater

chrome/updater/external_constants_mac.mm

@@ -42,7 +42,7 @@ bool DevOverrideProvider::UseCUP() const {
   }
 }
 
-int DevOverrideProvider::InitialDelay() const {
+double DevOverrideProvider::InitialDelay() const {
   @autoreleasepool {
     NSUserDefaults* userDefaults = [[NSUserDefaults alloc]
         initWithSuiteName:[NSString

chrome/updater/external_constants_override.cc

@@ -85,7 +85,7 @@ bool ExternalConstantsOverrider::UseCUP() const {
   return use_cup_value.GetBool();
 }
 
-int ExternalConstantsOverrider::InitialDelay() const {
+double ExternalConstantsOverrider::InitialDelay() const {
   if (!override_values_.contains(kDevOverrideKeyInitialDelay)) {
     return next_provider_->InitialDelay();
   }

chrome/updater/external_constants_override.h

@@ -38,7 +38,7 @@ class ExternalConstantsOverrider : public ExternalConstants {
   // Overrides of ExternalConstants:
   std::vector<GURL> UpdateURL() const override;
   bool UseCUP() const override;
-  int InitialDelay() const override;
+  double InitialDelay() const override;
 
  private:
   const base::flat_map<std::string, base::Value> override_values_;

chrome/updater/external_constants_win.cc

@@ -39,7 +39,7 @@ bool DevOverrideProvider::UseCUP() const {
   return next_provider_->UseCUP();
 }
 
-int DevOverrideProvider::InitialDelay() const {
+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;

components/component_updater/component_updater_command_line_config_policy.cc

@@ -97,8 +97,8 @@ ComponentUpdaterCommandLineConfigPolicy::
 
   const std::string initial_delay =
       GetSwitchArgument(switch_values, kInitialDelay);
-  int initial_delay_seconds = 0;
-  if (base::StringToInt(initial_delay, &initial_delay_seconds))
+  double initial_delay_seconds = 0;
+  if (base::StringToDouble(initial_delay, &initial_delay_seconds))
     initial_delay_ = initial_delay_seconds;
 }
 
@@ -127,7 +127,7 @@ GURL ComponentUpdaterCommandLineConfigPolicy::UrlSourceOverride() const {
   return url_source_override_;
 }
 
-int ComponentUpdaterCommandLineConfigPolicy::InitialDelay() const {
+double ComponentUpdaterCommandLineConfigPolicy::InitialDelay() const {
   return initial_delay_;
 }
 

components/component_updater/component_updater_command_line_config_policy.h

@@ -29,7 +29,7 @@ class ComponentUpdaterCommandLineConfigPolicy final
   bool PingsEnabled() const override;
   bool TestRequest() const override;
   GURL UrlSourceOverride() const override;
-  int InitialDelay() const override;
+  double InitialDelay() const override;
 
  private:
   bool background_downloads_enabled_ = false;
@@ -40,7 +40,7 @@ class ComponentUpdaterCommandLineConfigPolicy final
 
   // If non-zero, time interval in seconds until the first component
   // update check.
-  int initial_delay_ = 0;
+  double initial_delay_ = 0;
 
   GURL url_source_override_;
 

components/component_updater/component_updater_service.cc

@@ -95,7 +95,7 @@ void CrxUpdateService::Start() {
           << config_->NextCheckDelay() << " seconds. ";
 
   scheduler_->Schedule(
-      base::TimeDelta::FromSeconds(config_->InitialDelay()),
+      base::TimeDelta::FromSecondsD(config_->InitialDelay()),
       base::TimeDelta::FromSeconds(config_->NextCheckDelay()),
       base::BindRepeating(
           base::IgnoreResult(&CrxUpdateService::CheckForUpdates),

components/component_updater/configurator_impl.cc

@@ -54,7 +54,7 @@ ConfiguratorImpl::ConfiguratorImpl(
 
 ConfiguratorImpl::~ConfiguratorImpl() = default;
 
-int ConfiguratorImpl::InitialDelay() const {
+double ConfiguratorImpl::InitialDelay() const {
   if (initial_delay_)
     return initial_delay_;
   return fast_update_ ? 10 : kDelayOneMinute;

components/component_updater/configurator_impl.h

@@ -35,7 +35,7 @@ class ConfiguratorImpl {
   ~ConfiguratorImpl();
 
   // Delay in seconds from calling Start() to the first update check.
-  int InitialDelay() const;
+  double InitialDelay() const;
 
   // Delay in seconds to every subsequent update check. 0 means don't check.
   int NextCheckDelay() const;
@@ -100,7 +100,7 @@ class ConfiguratorImpl {
   const bool pings_enabled_;
   const bool require_encryption_;
   const GURL url_source_override_;
-  const int initial_delay_;
+  const double initial_delay_;
 
   DISALLOW_COPY_AND_ASSIGN(ConfiguratorImpl);
 };

components/component_updater/configurator_impl_unittest.cc

@@ -6,8 +6,11 @@
 
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "base/test/scoped_command_line.h"
 #include "components/component_updater/component_updater_command_line_config_policy.h"
+#include "components/component_updater/component_updater_switches.h"
 #include "components/component_updater/configurator_impl.h"
+#include "components/update_client/command_line_config_policy.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace component_updater {
@@ -105,15 +108,15 @@ TEST_F(ComponentUpdaterConfiguratorImplTest, InitialDelay) {
     bool PingsEnabled() const override { return false; }
     bool TestRequest() const override { return false; }
     GURL UrlSourceOverride() const override { return GURL(); }
-    int InitialDelay() const override { return initial_delay_; }
+    double InitialDelay() const override { return initial_delay_; }
 
     void set_fast_update(bool fast_update) { fast_update_ = fast_update; }
-    void set_initial_delay(int initial_delay) {
+    void set_initial_delay(double initial_delay) {
       initial_delay_ = initial_delay;
     }
 
    private:
-    int initial_delay_ = 0;
+    double initial_delay_ = 0;
     bool fast_update_ = false;
   };
 
@@ -137,6 +140,17 @@ TEST_F(ComponentUpdaterConfiguratorImplTest, InitialDelay) {
     config = std::make_unique<ConfiguratorImpl>(clcp, false);
     CHECK_EQ(2 * kDelayOneMinute, config->InitialDelay());
   }
+
+  {
+    base::test::ScopedCommandLine scoped_command_line;
+    base::CommandLine* command_line =
+        scoped_command_line.GetProcessCommandLine();
+    command_line->AppendSwitchASCII(switches::kComponentUpdater,
+                                    "initial-delay=3.14");
+    config = std::make_unique<ConfiguratorImpl>(
+        ComponentUpdaterCommandLineConfigPolicy(command_line), false);
+    CHECK_EQ(3.14, config->InitialDelay());
+  }
 }
 
 TEST_F(ComponentUpdaterConfiguratorImplTest, TestRequest) {
@@ -152,7 +166,7 @@ TEST_F(ComponentUpdaterConfiguratorImplTest, TestRequest) {
     bool PingsEnabled() const override { return false; }
     bool TestRequest() const override { return test_request_; }
     GURL UrlSourceOverride() const override { return GURL(); }
-    int InitialDelay() const override { return 0; }
+    double InitialDelay() const override { return 0; }
 
     void set_test_request(bool test_request) { test_request_ = test_request; }
 

components/update_client/command_line_config_policy.cc

@@ -37,7 +37,7 @@ GURL CommandLineConfigPolicy::UrlSourceOverride() const {
   return GURL();
 }
 
-int CommandLineConfigPolicy::InitialDelay() const {
+double CommandLineConfigPolicy::InitialDelay() const {
   return 0;
 }
 

components/update_client/command_line_config_policy.h

@@ -34,7 +34,7 @@ class CommandLineConfigPolicy {
 
   // If non-zero, time interval in seconds until the first component
   // update check.
-  virtual int InitialDelay() const;
+  virtual double InitialDelay() const;
 
   virtual ~CommandLineConfigPolicy() = default;
 };

components/update_client/configurator.h

@@ -36,7 +36,7 @@ class UnzipperFactory;
 class Configurator : public base::RefCountedThreadSafe<Configurator> {
  public:
   // Delay in seconds from calling Start() to the first update check.
-  virtual int InitialDelay() const = 0;
+  virtual double InitialDelay() const = 0;
 
   // Delay in seconds to every subsequent update check. 0 means don't check.
   virtual int NextCheckDelay() const = 0;

components/update_client/test_configurator.cc

@@ -56,7 +56,7 @@ TestConfigurator::TestConfigurator(PrefService* pref_service)
 
 TestConfigurator::~TestConfigurator() = default;
 
-int TestConfigurator::InitialDelay() const {
+double TestConfigurator::InitialDelay() const {
   return initial_time_;
 }
 
@@ -162,7 +162,7 @@ void TestConfigurator::SetOnDemandTime(int seconds) {
   ondemand_time_ = seconds;
 }
 
-void TestConfigurator::SetInitialDelay(int seconds) {
+void TestConfigurator::SetInitialDelay(double seconds) {
   initial_time_ = seconds;
 }
 

components/update_client/test_configurator.h

@@ -74,7 +74,7 @@ class TestConfigurator : public Configurator {
   TestConfigurator& operator=(const TestConfigurator&) = delete;
 
   // Overrides for Configurator.
-  int InitialDelay() const override;
+  double InitialDelay() const override;
   int NextCheckDelay() const override;
   int OnDemandDelay() const override;
   int UpdateDelay() const override;
@@ -104,7 +104,7 @@ class TestConfigurator : public Configurator {
 
   void SetBrand(const std::string& brand);
   void SetOnDemandTime(int seconds);
-  void SetInitialDelay(int seconds);
+  void SetInitialDelay(double seconds);
   void SetDownloadPreference(const std::string& download_preference);
   void SetEnabledCupSigning(bool use_cup_signing);
   void SetEnabledComponentUpdates(bool enabled_component_updates);
@@ -124,8 +124,8 @@ class TestConfigurator : public Configurator {
   class TestPatchService;
 
   std::string brand_;
-  int initial_time_;
-  int ondemand_time_;
+  double initial_time_{0};
+  int ondemand_time_{0};
   std::string download_preference_;
   bool enabled_cup_signing_;
   bool enabled_component_updates_;

ios/chrome/browser/component_updater/ios_component_updater_configurator.cc

@@ -39,7 +39,7 @@ class IOSConfigurator : public update_client::Configurator {
   explicit IOSConfigurator(const base::CommandLine* cmdline);
 
   // update_client::Configurator overrides.
-  int InitialDelay() const override;
+  double InitialDelay() const override;
   int NextCheckDelay() const override;
   int OnDemandDelay() const override;
   int UpdateDelay() const override;
@@ -88,7 +88,7 @@ IOSConfigurator::IOSConfigurator(const base::CommandLine* cmdline)
     : configurator_impl_(ComponentUpdaterCommandLineConfigPolicy(cmdline),
                          false) {}
 
-int IOSConfigurator::InitialDelay() const {
+double IOSConfigurator::InitialDelay() const {
   return configurator_impl_.InitialDelay();
 }