Upload non-enterprise status report with frequency of 10 minutes.

Make frequency of status reports upload a parameter of the StatusUploader
to allow easy customization.

Bug: 826894
Change-Id: Id37890fb5e65d83bb9877decdc23a5e1b9d09de2
Reviewed-on: https://chromium-review.googlesource.com/1000415Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Bartosz Fabianowski <bartfab@chromium.org>
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550617}

chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc

@@ -69,6 +69,11 @@ const char kRialtoRequisition[] = "rialto";
 const char kZeroTouchEnrollmentForced[] = "forced";
 const char kZeroTouchEnrollmentHandsOff[] = "hands-off";
 
+// Default frequency for uploading enterprise status reports. Can be overriden
+// by Device Policy.
+constexpr base::TimeDelta kDeviceStatusUploadFrequency =
+    base::TimeDelta::FromHours(3);
+
 // Fetches a machine statistic value from StatisticsProvider, returns an empty
 // string on failure.
 std::string GetMachineStatistic(const std::string& key) {
@@ -377,7 +382,7 @@ void DeviceCloudPolicyManagerChromeOS::CreateStatusUploader() {
           DeviceStatusCollector::CPUTempFetcher(),
           DeviceStatusCollector::AndroidStatusFetcher(),
           true /* is_enterprise_device */),
-      task_runner_));
+      task_runner_, kDeviceStatusUploadFrequency));
 }
 
 }  // namespace policy

chrome/browser/chromeos/policy/status_uploader.cc

@@ -36,18 +36,15 @@ const int kMinUploadScheduleDelayMs = 60 * 1000;  // 60 seconds
 
 namespace policy {
 
-const int64_t StatusUploader::kDefaultUploadDelayMs =
-    3 * 60 * 60 * 1000;  // 3 hours
-
 StatusUploader::StatusUploader(
     CloudPolicyClient* client,
     std::unique_ptr<DeviceStatusCollector> collector,
-    const scoped_refptr<base::SequencedTaskRunner>& task_runner)
+    const scoped_refptr<base::SequencedTaskRunner>& task_runner,
+    base::TimeDelta default_upload_frequency)
     : client_(client),
       collector_(std::move(collector)),
       task_runner_(task_runner),
-      upload_frequency_(
-          base::TimeDelta::FromMilliseconds(kDefaultUploadDelayMs)),
+      upload_frequency_(default_upload_frequency),
       has_captured_media_(false),
       weak_factory_(this) {
   // StatusUploader is currently only created for registered clients, and

chrome/browser/chromeos/policy/status_uploader.h

@@ -5,8 +5,6 @@
 #ifndef CHROME_BROWSER_CHROMEOS_POLICY_STATUS_UPLOADER_H_
 #define CHROME_BROWSER_CHROMEOS_POLICY_STATUS_UPLOADER_H_
 
-#include <stdint.h>
-
 #include <memory>
 
 #include "base/cancelable_callback.h"
@@ -32,15 +30,13 @@ class DeviceStatusCollector;
 // passed DeviceStatusCollector.
 class StatusUploader : public MediaCaptureDevicesDispatcher::Observer {
  public:
-  // Refresh constants.
-  static const int64_t kDefaultUploadDelayMs;
-
   // Constructor. |client| must be registered and must stay
   // valid and registered through the lifetime of this StatusUploader
   // object.
   StatusUploader(CloudPolicyClient* client,
                  std::unique_ptr<DeviceStatusCollector> collector,
-                 const scoped_refptr<base::SequencedTaskRunner>& task_runner);
+                 const scoped_refptr<base::SequencedTaskRunner>& task_runner,
+                 base::TimeDelta default_upload_frequency);
 
   ~StatusUploader() override;
 

chrome/browser/chromeos/policy/status_uploader_unittest.cc

@@ -42,6 +42,9 @@ namespace em = enterprise_management;
 
 namespace {
 
+constexpr base::TimeDelta kDefaultStatusUploadDelay =
+    base::TimeDelta::FromHours(1);
+
 class MockDeviceStatusCollector : public policy::DeviceStatusCollector {
  public:
   explicit MockDeviceStatusCollector(PrefService* local_state)
@@ -176,7 +179,8 @@ class StatusUploaderTest : public testing::Test {
 
 TEST_F(StatusUploaderTest, BasicTest) {
   EXPECT_FALSE(task_runner_->HasPendingTask());
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
   EXPECT_EQ(1U, task_runner_->NumPendingTasks());
   // On startup, first update should happen in 1 minute.
   EXPECT_EQ(base::TimeDelta::FromMinutes(1),
@@ -184,31 +188,30 @@ TEST_F(StatusUploaderTest, BasicTest) {
 }
 
 TEST_F(StatusUploaderTest, DifferentFrequencyAtStart) {
-  const int new_delay = StatusUploader::kDefaultUploadDelayMs * 2;
-  settings_helper_.SetInteger(chromeos::kReportUploadFrequency, new_delay);
-  const base::TimeDelta expected_delay = base::TimeDelta::FromMilliseconds(
-      new_delay);
+  const base::TimeDelta new_delay = kDefaultStatusUploadDelay * 2;
+  settings_helper_.SetInteger(chromeos::kReportUploadFrequency,
+                              new_delay.InMilliseconds());
   EXPECT_FALSE(task_runner_->HasPendingTask());
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
   ASSERT_EQ(1U, task_runner_->NumPendingTasks());
   // On startup, first update should happen in 1 minute.
   EXPECT_EQ(base::TimeDelta::FromMinutes(1),
             task_runner_->NextPendingTaskDelay());
 
   // Second update should use the delay specified in settings.
-  RunPendingUploadTaskAndCheckNext(uploader, expected_delay,
+  RunPendingUploadTaskAndCheckNext(uploader, new_delay,
                                    true /* upload_success */);
 }
 
 TEST_F(StatusUploaderTest, ResetTimerAfterStatusCollection) {
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
-  const base::TimeDelta expected_delay = base::TimeDelta::FromMilliseconds(
-      StatusUploader::kDefaultUploadDelayMs);
-  RunPendingUploadTaskAndCheckNext(uploader, expected_delay,
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
+  RunPendingUploadTaskAndCheckNext(uploader, kDefaultStatusUploadDelay,
                                    true /* upload_success */);
 
   // Handle this response also, and ensure new task is queued.
-  RunPendingUploadTaskAndCheckNext(uploader, expected_delay,
+  RunPendingUploadTaskAndCheckNext(uploader, kDefaultStatusUploadDelay,
                                    true /* upload_success */);
 
   // Now that the previous request was satisfied, a task to do the next
@@ -217,7 +220,8 @@ TEST_F(StatusUploaderTest, ResetTimerAfterStatusCollection) {
 }
 
 TEST_F(StatusUploaderTest, ResetTimerAfterFailedStatusCollection) {
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
 
   // Running the queued task should pass a callback into
   // GetDeviceAndSessionStatusAsync. We'll grab this callback and send nullptrs
@@ -239,18 +243,15 @@ TEST_F(StatusUploaderTest, ResetTimerAfterFailedStatusCollection) {
   EXPECT_EQ(1U, task_runner_->NumPendingTasks());
 
   // Check the delay of the queued upload
-  const base::TimeDelta expected_delay = base::TimeDelta::FromMilliseconds(
-      StatusUploader::kDefaultUploadDelayMs);
-  CheckPendingTaskDelay(uploader, expected_delay);
+  CheckPendingTaskDelay(uploader, kDefaultStatusUploadDelay);
 }
 
 TEST_F(StatusUploaderTest, ResetTimerAfterUploadError) {
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
 
-  const base::TimeDelta expected_delay =
-      base::TimeDelta::FromMilliseconds(StatusUploader::kDefaultUploadDelayMs);
   // Simulate upload error
-  RunPendingUploadTaskAndCheckNext(uploader, expected_delay,
+  RunPendingUploadTaskAndCheckNext(uploader, kDefaultStatusUploadDelay,
                                    false /* upload_success */);
 
   // Now that the previous request was satisfied, a task to do the next
@@ -259,7 +260,8 @@ TEST_F(StatusUploaderTest, ResetTimerAfterUploadError) {
 }
 
 TEST_F(StatusUploaderTest, ResetTimerAfterUnregisteredClient) {
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
 
   client_.SetDMToken("");
   EXPECT_FALSE(client_.is_registered());
@@ -282,25 +284,24 @@ TEST_F(StatusUploaderTest, ResetTimerAfterUnregisteredClient) {
   // A task to try again should be queued.
   ASSERT_EQ(1U, task_runner_->NumPendingTasks());
 
-  const base::TimeDelta expected_delay =
-      base::TimeDelta::FromMilliseconds(StatusUploader::kDefaultUploadDelayMs);
-  CheckPendingTaskDelay(uploader, expected_delay);
+  CheckPendingTaskDelay(uploader, kDefaultStatusUploadDelay);
 }
 
 TEST_F(StatusUploaderTest, ChangeFrequency) {
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
   // Change the frequency. The new frequency should be reflected in the timing
   // used for the next callback.
-  const int new_delay = StatusUploader::kDefaultUploadDelayMs * 2;
-  settings_helper_.SetInteger(chromeos::kReportUploadFrequency, new_delay);
-  const base::TimeDelta expected_delay = base::TimeDelta::FromMilliseconds(
-      new_delay);
-  RunPendingUploadTaskAndCheckNext(uploader, expected_delay,
+  const base::TimeDelta new_delay = kDefaultStatusUploadDelay * 2;
+  settings_helper_.SetInteger(chromeos::kReportUploadFrequency,
+                              new_delay.InMilliseconds());
+  RunPendingUploadTaskAndCheckNext(uploader, new_delay,
                                    true /* upload_success */);
 }
 
 TEST_F(StatusUploaderTest, NoUploadAfterUserInput) {
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
   // Should allow data upload before there is user input.
   EXPECT_TRUE(uploader.IsSessionDataUploadAllowed());
 
@@ -313,7 +314,8 @@ TEST_F(StatusUploaderTest, NoUploadAfterUserInput) {
 }
 
 TEST_F(StatusUploaderTest, NoUploadAfterVideoCapture) {
-  StatusUploader uploader(&client_, std::move(collector_), task_runner_);
+  StatusUploader uploader(&client_, std::move(collector_), task_runner_,
+                          kDefaultStatusUploadDelay);
   // Should allow data upload before there is video capture.
   EXPECT_TRUE(uploader.IsSessionDataUploadAllowed());
 

chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc

@@ -71,6 +71,10 @@ const char kUMAInitialFetchOAuth2Error[] =
 const char kUMAInitialFetchOAuth2NetworkError[] =
     "Enterprise.UserPolicyChromeOS.InitialFetch.OAuth2NetworkError";
 
+// Default frequency for uploading non-enterprise status reports.
+constexpr base::TimeDelta kDeviceStatusUploadFrequency =
+    base::TimeDelta::FromMinutes(10);
+
 }  // namespace
 
 UserCloudPolicyManagerChromeOS::UserCloudPolicyManagerChromeOS(
@@ -219,7 +223,7 @@ void UserCloudPolicyManagerChromeOS::Connect(
             DeviceStatusCollector::CPUTempFetcher(),
             DeviceStatusCollector::AndroidStatusFetcher(),
             false /* is_enterprise_device */),
-        task_runner_));
+        task_runner_, kDeviceStatusUploadFrequency));
   }
 }