Add OS and firmware version to device status reports.

Add OS and firmware version to device status reports.

BUG=chromium-os:22035
TEST=Manual.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114210

[Joao da Silva, 2011-12-12 14:23:03]

LGTM with nits

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
File chrome/browser/policy/device_status_collector.cc (right):

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
chrome/browser/policy/device_status_collector.cc:9: #include
"base/message_loop.h"
Nit: not needed

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
chrome/browser/policy/device_status_collector.cc:11: #include "base/task.h"
Nit: not needed

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
chrome/browser/policy/device_status_collector.cc:14: #include
"chrome/browser/idle.h"
Nit: these 3 are already included in .h, not needed here (time.h,
version_loader.h and idle.h)

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
File chrome/browser/policy/device_status_collector.h (right):

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
chrome/browser/policy/device_status_collector.h:15: using
chromeos::VersionLoader;
Nit: I'd avoid this, but it's your call. I'd rather have these on the .cc file
only; otherwise whoever includes this header will have these names in the global
namespace.

[Patrick Dubroy, 2011-12-12 15:18:00]

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
File chrome/browser/policy/device_status_collector.cc (right):

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
chrome/browser/policy/device_status_collector.cc:9: #include
"base/message_loop.h"
On 2011/12/12 14:23:04, Joao da Silva wrote:
> Nit: not needed

Done.

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
chrome/browser/policy/device_status_collector.cc:11: #include "base/task.h"
On 2011/12/12 14:23:04, Joao da Silva wrote:
> Nit: not needed

Good catch. These were used in an older version of the code.

Done.

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
chrome/browser/policy/device_status_collector.cc:14: #include
"chrome/browser/idle.h"
On 2011/12/12 14:23:04, Joao da Silva wrote:
> Nit: these 3 are already included in .h, not needed here (time.h,
> version_loader.h and idle.h)

Ok, I've never been quite sure what the recommended thing to do in this case
was.

Done.

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
File chrome/browser/policy/device_status_collector.h (right):

http://codereview.chromium.org/8920003/diff/1/chrome/browser/policy/device_st...
chrome/browser/policy/device_status_collector.h:15: using
chromeos::VersionLoader;
On 2011/12/12 14:23:04, Joao da Silva wrote:
> Nit: I'd avoid this, but it's your call. I'd rather have these on the .cc file
> only; otherwise whoever includes this header will have these names in the
global
> namespace.

Done.

[commit-bot: I haz the power, 2011-12-12 15:18:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dubroy@chromium.org/8920003/5001

[commit-bot: I haz the power, 2011-12-12 15:49:21]

Try job failure for 8920003-5001 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[Patrick Dubroy, 2011-12-12 17:28:41]

Joao, can you take another quick look?

[Joao da Silva, 2011-12-12 17:36:19]

LGTM but please fix the chromeos-only #includes.

http://codereview.chromium.org/8920003/diff/4003/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_data_store.h (right):

http://codereview.chromium.org/8920003/diff/4003/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_data_store.h:13: #include
"chrome/browser/policy/device_status_collector.h"
Wrap this #include in #if defined(OS_CHROMEOS). Write that #if block after all
these #includes.

http://codereview.chromium.org/8920003/diff/4003/chrome/browser/prefs/browser...
File chrome/browser/prefs/browser_prefs.cc (right):

http://codereview.chromium.org/8920003/diff/4003/chrome/browser/prefs/browser...
chrome/browser/prefs/browser_prefs.cc:38: #include
"chrome/browser/policy/device_status_collector.h"
Move this header into the #if defined(OS_CHROMEOS) block below

[Patrick Dubroy, 2011-12-13 10:33:51]

http://codereview.chromium.org/8920003/diff/4003/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_data_store.h (right):

http://codereview.chromium.org/8920003/diff/4003/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_data_store.h:13: #include
"chrome/browser/policy/device_status_collector.h"
On 2011/12/12 17:36:20, Joao da Silva wrote:
> Wrap this #include in #if defined(OS_CHROMEOS). Write that #if block after all
> these #includes.

Done.

http://codereview.chromium.org/8920003/diff/4003/chrome/browser/prefs/browser...
File chrome/browser/prefs/browser_prefs.cc (right):

http://codereview.chromium.org/8920003/diff/4003/chrome/browser/prefs/browser...
chrome/browser/prefs/browser_prefs.cc:38: #include
"chrome/browser/policy/device_status_collector.h"
On 2011/12/12 17:36:20, Joao da Silva wrote:
> Move this header into the #if defined(OS_CHROMEOS) block below

Done.

[commit-bot: I haz the power, 2011-12-13 10:34:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dubroy@chromium.org/8920003/8001

[commit-bot: I haz the power, 2011-12-13 12:34:08]

Change committed as 114210