extensions: Add CPU temperatures field to system.cpu.getInfo() function

extensions:: Add CPU temperatures field to system.cpu.getInfo() function

This field represents the CPU temperature at each thermal zone in
degrees Celsius.

BUG=chromium:709102
Review-Url: https://codereview.chromium.org/2802593005
Cr-Commit-Position: refs/heads/master@{#467861}
Committed: https://chromium.googlesource.com/chromium/src/+/c045c21a0b84cbeabd7428c9722fd37258c81bc0

[Simon Que, 2017-04-10 22:44:43]

Description was changed from

==========
WIP: Add getState() function to system.cpu

This returns real-time CPU state info. Currently this includes only the
CPU temperature.

BUG=chromium:709102
==========

to

==========
WIP: Add CPU temperatures field to system.cpu.getInfo() function

This field represents the CPU temperature at each thermal zone in
degrees Celsius.

BUG=chromium:709102
==========

[Simon Que, 2017-04-25 16:37:03]

Description was changed from

==========
WIP: Add CPU temperatures field to system.cpu.getInfo() function

This field represents the CPU temperature at each thermal zone in
degrees Celsius.

BUG=chromium:709102
==========

to

==========
extensions:: Add CPU temperatures field to system.cpu.getInfo() function

This field represents the CPU temperature at each thermal zone in
degrees Celsius.

BUG=chromium:709102
==========

[Simon Que, 2017-04-25 16:37:03]

sque@chromium.org changed reviewers:
+ rkc@chromium.org

[Simon Que, 2017-04-25 19:01:24]

sque@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org, tbarzic@chromium.org
- rkc@chromium.org

[Devlin, 2017-04-26 02:04:59]

https://codereview.chromium.org/2802593005/diff/80001/extensions/browser/api/...
File extensions/browser/api/system_cpu/cpu_info_provider.cc (right):

https://codereview.chromium.org/2802593005/diff/80001/extensions/browser/api/...
extensions/browser/api/system_cpu/cpu_info_provider.cc:48:
info_.temperatures.clear();
what is this clear()ing?

https://codereview.chromium.org/2802593005/diff/80001/extensions/browser/api/...
extensions/browser/api/system_cpu/cpu_info_provider.cc:50:
chromeos::system::CPUTemperatureReader().GetCPUTemperatures();
GetCPUTemperatures() doesn't make it clear what the return result is.  Refrain
from using auto.

https://codereview.chromium.org/2802593005/diff/80001/extensions/browser/api/...
extensions/browser/api/system_cpu/cpu_info_provider.cc:51: for (const auto& info
: cpu_temp_info) {
info_.temperatures.reserve(cpu_temp_info.size())

https://codereview.chromium.org/2802593005/diff/80001/extensions/common/api/s...
File extensions/common/api/system_cpu.idl (right):

https://codereview.chromium.org/2802593005/diff/80001/extensions/common/api/s...
extensions/common/api/system_cpu.idl:49: // Temperatures are in degrees Celsius.
Mention this is CrOS only.

[Simon Que, 2017-04-26 02:27:21]

https://codereview.chromium.org/2802593005/diff/80001/extensions/browser/api/...
File extensions/browser/api/system_cpu/cpu_info_provider.cc (right):

https://codereview.chromium.org/2802593005/diff/80001/extensions/browser/api/...
extensions/browser/api/system_cpu/cpu_info_provider.cc:48:
info_.temperatures.clear();
On 2017/04/26 02:04:59, Devlin wrote:
> what is this clear()ing?

CpuInfoProvider is a singleton class. |info_| retains its state between calls to
QueryInfo(), so the vectors must be cleared. See above, for example:
"info_.processors.clear();"

https://codereview.chromium.org/2802593005/diff/80001/extensions/browser/api/...
extensions/browser/api/system_cpu/cpu_info_provider.cc:50:
chromeos::system::CPUTemperatureReader().GetCPUTemperatures();
On 2017/04/26 02:04:58, Devlin wrote:
> GetCPUTemperatures() doesn't make it clear what the return result is.  Refrain
> from using auto.

Done.

https://codereview.chromium.org/2802593005/diff/80001/extensions/browser/api/...
extensions/browser/api/system_cpu/cpu_info_provider.cc:51: for (const auto& info
: cpu_temp_info) {
On 2017/04/26 02:04:59, Devlin wrote:
> info_.temperatures.reserve(cpu_temp_info.size())

Done.

https://codereview.chromium.org/2802593005/diff/80001/extensions/common/api/s...
File extensions/common/api/system_cpu.idl (right):

https://codereview.chromium.org/2802593005/diff/80001/extensions/common/api/s...
extensions/common/api/system_cpu.idl:49: // Temperatures are in degrees Celsius.
On 2017/04/26 02:04:59, Devlin wrote:
> Mention this is CrOS only.

Done.

[tbarzic, 2017-04-27 17:50:19]

lgtm

https://codereview.chromium.org/2802593005/diff/100001/extensions/browser/api...
File extensions/browser/api/system_cpu/system_cpu_apitest.cc (right):

https://codereview.chromium.org/2802593005/diff/100001/extensions/browser/api...
extensions/browser/api/system_cpu/system_cpu_apitest.cc:32:
info_.temperatures.clear();
info_.temperatures = {30.125, 40.0625};?

https://codereview.chromium.org/2802593005/diff/100001/extensions/common/api/...
File extensions/common/api/system_cpu.idl (right):

https://codereview.chromium.org/2802593005/diff/100001/extensions/common/api/...
extensions/common/api/system_cpu.idl:49: // Temperatures are in degrees Celsius.
Currently supported on Chrome OS
nit: I'd put "Chrome OS only" part in a separate paragraph, and and maybe make
it bold.

[Simon Que, 2017-04-27 17:59:55]

https://codereview.chromium.org/2802593005/diff/100001/extensions/browser/api...
File extensions/browser/api/system_cpu/system_cpu_apitest.cc (right):

https://codereview.chromium.org/2802593005/diff/100001/extensions/browser/api...
extensions/browser/api/system_cpu/system_cpu_apitest.cc:32:
info_.temperatures.clear();
On 2017/04/27 17:50:19, tbarzic wrote:
> info_.temperatures = {30.125, 40.0625};?

Done.

https://codereview.chromium.org/2802593005/diff/100001/extensions/common/api/...
File extensions/common/api/system_cpu.idl (right):

https://codereview.chromium.org/2802593005/diff/100001/extensions/common/api/...
extensions/common/api/system_cpu.idl:49: // Temperatures are in degrees Celsius.
Currently supported on Chrome OS
On 2017/04/27 17:50:19, tbarzic wrote:
> nit: I'd put "Chrome OS only" part in a separate paragraph, and and maybe make
> it bold.
> 

Done.

[Devlin, 2017-04-27 18:09:38]

lgtm

[Simon Que, 2017-04-27 18:16:26]

The CQ bit was checked by sque@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 18:16:52]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-27 18:35:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-27 18:35:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Simon Que, 2017-04-27 21:42:36]

On 2017/04/27 18:09:38, Devlin wrote:
> lgtm

My CL is failing the CQ buildbot even without the patch:
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...

Do I need to rebase my CL or just wait a day?

[Devlin, 2017-04-27 21:46:08]

On 2017/04/27 21:42:36, Simon Que wrote:
> On 2017/04/27 18:09:38, Devlin wrote:
> > lgtm
> 
> My CL is failing the CQ buildbot even without the patch:
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> 
> Do I need to rebase my CL or just wait a day?

For errors like this, waiting a period of time (hopefully not even a day) should
be sufficient.

[Simon Que, 2017-04-28 01:36:54]

The CQ bit was checked by sque@chromium.org

[Simon Que, 2017-04-28 01:36:54]

The patchset sent to the CQ was uploaded after l-g-t-m from tbarzic@chromium.org
Link to the patchset: https://codereview.chromium.org/2802593005/#ps120001
(title: "Responded to tbarzic's feedback")

[commit-bot: I haz the power, 2017-04-28 01:38:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-28 03:00:27]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493343414599980,
"parent_rev": "bfabbfb6a32d7f654aad1ab45f5d233445db1b21", "commit_rev":
"c045c21a0b84cbeabd7428c9722fd37258c81bc0"}

[commit-bot: I haz the power, 2017-04-28 03:00:34]

Description was changed from

==========
extensions:: Add CPU temperatures field to system.cpu.getInfo() function

This field represents the CPU temperature at each thermal zone in
degrees Celsius.

BUG=chromium:709102
==========

to

==========
extensions:: Add CPU temperatures field to system.cpu.getInfo() function

This field represents the CPU temperature at each thermal zone in
degrees Celsius.

BUG=chromium:709102

Review-Url: https://codereview.chromium.org/2802593005
Cr-Commit-Position: refs/heads/master@{#467861}
Committed:
https://chromium.googlesource.com/chromium/src/+/c045c21a0b84cbeabd7428c9722f...
==========

[commit-bot: I haz the power, 2017-04-28 03:00:35]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/c045c21a0b84cbeabd7428c9722f...