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/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) {
+  // 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();
+
+  // TODO(atwilson): Listen for changes to the active user and generate a
+  // status upload if it changes.

[Mattias Nissler (ping if slow), 2015/01/23 13:46:35]

I don't understand this TODO. Does it refer to adding the currently active user
to device status uploads? Or does it refer to user status uploads? The latter
don't belong here IMHO as they'd use a different CloudPolicyClient and
StatusUploader instance.

[Andrew T Wilson (Slow), 2015/01/23 19:16:19]

I'm planning to upload session status using the device cloud policy client.
There's no reason not to do this, and in fact for kiosk sessions there isn't a
user cloud policy client, right?

Basically this TODO is saying that we want to trigger a status upload after a
session starts/ends because various things (accessible drives/networks, session
status) will change.

That said, I've removed the TODO because it really doesn't add any value as you
pointed out.

+}
+
+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)));

[Mattias Nissler (ping if slow), 2015/01/23 13:46:35]

#include "base/bind_helpers.h"

[Andrew T Wilson (Slow), 2015/01/23 19:16:19]

Done.

+  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);

[Mattias Nissler (ping if slow), 2015/01/23 13:46:34]

Hm, session status doesn't make sense for device status reporting, just remove
this?

[Andrew T Wilson (Slow), 2015/01/23 19:16:19]

They are uploaded together. That's why this class is called StatusUploader and
not DeviceStatusUploader :)

+  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(

[Mattias Nissler (ping if slow), 2015/01/23 13:46:35]

You're assuming that |client_| is registered with the server here. That should
either be documented if we can safely assume the CloudPolicyClient will always
have a client ID / DM token available (I haven't checked), or alternatively
StatusUploader could register as an observer with CloudPolicyClient to be
notified of (un-)registration and take the proper action. The latter removes any
assumptions about the state of the CloudPolicyClient dependency, so is probably
preferable from a code hygiene perspective.

[Andrew T Wilson (Slow), 2015/01/23 19:16:19]

Yes, I'm assuming it's registered already - I'll add a DCHECK() to enforce this.
On un-registration, the idea is that DCPM will blow away the client and status
uploader and create a new one.

+      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