Refactored device status uploading out of policy fetch infrastructure.

chrome/browser/chromeos/policy/status_uploader.cc

+// Copyright (c) 2015 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/browser/chromeos/policy/status_uploader.h"
+
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/location.h"
+#include "chrome/browser/chromeos/policy/device_status_collector.h"
+#include "chrome/common/pref_names.h"
+#include "components/policy/core/common/cloud/cloud_policy_client.h"
+#include "components/policy/core/common/cloud/device_management_service.h"
+
+namespace {
+const int kMinUploadDelayMs = 60 * 1000;  // 60 seconds
+}  // namespace
+
+namespace policy {
+
+const int64 StatusUploader::kDefaultUploadDelayMs =
+    3 * 60 * 60 * 1000;  // 3 hours
+
+StatusUploader::StatusUploader(
+    PrefService* local_state,
+    CloudPolicyClient* client,
+    scoped_ptr<DeviceStatusCollector> collector,
+    const scoped_refptr<base::SequencedTaskRunner>& task_runner)
+    : client_(client),
+      collector_(collector.Pass()),
+      task_runner_(task_runner) {
+  DCHECK(client->is_registered());

[Mattias Nissler (ping if slow), 2015/01/26 10:37:02]

If you don't think it's worthwhile to make this code work with
CloudPolicyClients in any state, please at least add a comment here saying what
needs to be done in order drop the is_registered() assumption in the future.

Background: One major motivation for the big re-write of the cloud policy code
was that there were lots of hidden dependencies and assumptions about various
pieces being in certain states across the entire cloud policy implementation,
leading to bugs and the inability to refactor anything. That's why we now have
only a handful of core classes (CloudPolicyClient and CloudPolicyStore for the
most part) with observer interfaces, such that you can add aspects (such as
status uploading, invalidations, etc.) in a way that doesn't affect other stuff
that is going on. Turning this around, new components shouldn't fall over if
they find the system in a state they don't expect.

I'm willing to accept that unregistered clients are a non-reality for now, but
if they turn out to become one then we need to handle the fallout here, and not
by having embedding code implement only-start-status-reporting-if-registered
kludges - hence the request for the comment.

[Andrew T Wilson (Slow), 2015/01/28 13:19:12]

Comment added.

+  // Listen for changes to the upload delay, and start sending updates to the
+  // server.
+  upload_delay_.reset(new IntegerPrefMember());
+  upload_delay_->Init(
+      prefs::kDeviceStatusUploadRate, local_state,
+      base::Bind(&StatusUploader::ScheduleNextStatusUpload,
+                 base::Unretained(this)));
+  ScheduleNextStatusUpload();
+}
+
+StatusUploader::~StatusUploader() {
+}
+
+void StatusUploader::ScheduleNextStatusUpload() {
+  // Calculate when to fire off the next update (if it should have already
+  // happened, this yields a TimeDelta of 0).
+  base::TimeDelta delta = base::TimeDelta::FromMilliseconds(
+      std::max(kMinUploadDelayMs, upload_delay_->GetValue()));
+  base::TimeDelta delay =
+      std::max((last_upload_ + delta) - base::Time::NowFromSystemTime(),
+               base::TimeDelta());
+  upload_callback_.Reset(base::Bind(&StatusUploader::UploadStatus,
+                                    base::Unretained(this)));
+  task_runner_->PostDelayedTask(FROM_HERE, upload_callback_.callback(), delay);
+}
+
+void StatusUploader::UploadStatus() {
+  // If we already have an upload request active, don't bother starting another
+  // one.
+  if (request_job_)
+    return;
+
+  enterprise_management::DeviceStatusReportRequest device_status;
+  bool have_device_status = collector_->GetDeviceStatus(&device_status);
+  enterprise_management::SessionStatusReportRequest session_status;
+  bool have_session_status = collector_->GetSessionStatus(&session_status);
+  if (!have_device_status && !have_session_status) {
+    // Don't have any status to upload - just set our timer for next time.
+    last_upload_ = base::Time::NowFromSystemTime();
+    ScheduleNextStatusUpload();
+    return;
+  }
+  request_job_ = client_->CreateUploadStatusJob(
+      have_device_status ? &device_status : nullptr,
+      have_session_status ? &session_status : nullptr);
+  request_job_->Start(base::Bind(&StatusUploader::OnUploadCompleted,
+                                 base::Unretained(this)));
+}
+
+void StatusUploader::OnUploadCompleted(
+    DeviceManagementStatus status,
+    int net_error,
+    const enterprise_management::DeviceManagementResponse& response) {
+  // Set the last upload time, regardless of whether the upload was successful
+  // or not (we don't change the time of the next upload based on whether this
+  // upload succeeded or not - if a status upload fails, we just skip it and
+  // wait until it's time to try again.
+  last_upload_ = base::Time::NowFromSystemTime();
+  request_job_.reset();
+
+  // Figure out if the upload was successful - if so, tell the collector so it
+  // can clear its cache of pending items.
+  if (status == DM_STATUS_SUCCESS)
+    collector_->OnSubmittedSuccessfully();
+
+  ScheduleNextStatusUpload();
+}
+
+}  // namespace policy