[Sensors] Error handler in Senor class should stop the platform sensor

[Sensors] Error handler in Senor class should stop the platform sensor

At the moment SensorProxy is the only source of error notification for the Sensor class instances, however soon other error notification sources will turn up (e.g. permission subsystem), so we must guarantee that stopping of a Sensor due to an error is reflected on platform side.

BUG=606766
Review-Url: https://codereview.chromium.org/2839963004
Cr-Commit-Position: refs/heads/master@{#467709}
Committed: https://chromium.googlesource.com/chromium/src/+/e2d622806d0648f17eac947bd9d77b29211ccc2f

[Mikhail, 2017-04-25 15:52:40]

The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 15:53:01]

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-25 17:38:02]

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

[commit-bot: I haz the power, 2017-04-25 17:38:02]

Dry run: This issue passed the CQ dry run.

[Mikhail, 2017-04-26 07:39:48]

Description was changed from

==========
[Sensors] Error handler in Senor class should stop the platform sensor

At the moment SensorProxy is the only source of error notification for the
Sensor class instances, however soon other error notification sources will turn
up (e.g. permission subsystem), so we must guarantee that stopping of a Sensor
due to an error is reflected on platform side.

BUG=606766
==========

to

==========
[Sensors] Error handler in Senor class should stop the platform sensor

At the moment SensorProxy is the only source of error notification for the
Sensor class instances, however soon other error notification sources will turn
up (e.g. permission subsystem), so we must guarantee that stopping of a Sensor
due to an error is reflected on platform side.

BUG=606766
==========

[Mikhail, 2017-04-26 07:39:49]

mikhail.pozdnyakov@intel.com changed reviewers:
+ alexander.shalamov@intel.com, reillyg@chromium.org

[Mikhail, 2017-04-26 07:40:01]

PTAL

[Mikhail, 2017-04-26 11:57:01]

The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 11:57:18]

Dry run: CQ is trying da patch.

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

[shalamov, 2017-04-26 12:00:14]

lgtm

[commit-bot: I haz the power, 2017-04-26 13:40:31]

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

[commit-bot: I haz the power, 2017-04-26 13:40:32]

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

[Reilly Grant (use Gerrit), 2017-04-26 14:58:45]

My comments are mostly nits but it looks like there is a test failure to
investigate.

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js
(right):

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js:74:
.then(mockSensor => { return mockSensor.addConfigurationCalled(); })
.then(mockSensor => mockSensor.addConfigurationCalled())

(same below)

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js:91: });
.then(mockSensor => {
  sensorObject.stop();
  return mockSensor;
})

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js:94:
return testPromise;
nit: return sensor.mockSensorProvider.... above

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/sensor/Sensor.cpp (right):

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/Sensor.cpp:222: }
nit: no braces when both if and else clauses are a single line.

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/Sensor.cpp:251: auto start_callback =
WTF::Bind(&Sensor::OnAddConfigurationRequestCompleted,
nit: no need to save the callback in a local variable before passing it to
sensor_proxy_->AddConfiguration().

[Mikhail, 2017-04-27 06:21:51]

The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 06:22:00]

Dry run: CQ is trying da patch.

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

[Mikhail, 2017-04-27 06:31:20]

The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 06:31:43]

Dry run: CQ is trying da patch.

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

[Mikhail, 2017-04-27 07:50:34]

Thanks for review!

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js
(right):

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js:74:
.then(mockSensor => { return mockSensor.addConfigurationCalled(); })
On 2017/04/26 14:58:45, Reilly Grant wrote:
> .then(mockSensor => mockSensor.addConfigurationCalled())
> 
> (same below)

Done.

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js:91: });
On 2017/04/26 14:58:45, Reilly Grant wrote:
> .then(mockSensor => {
>   sensorObject.stop();
>   return mockSensor;
> })

Done.

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/sensor/resources/generic-sensor-tests.js:94:
return testPromise;
On 2017/04/26 14:58:45, Reilly Grant wrote:
> nit: return sensor.mockSensorProvider.... above

Done.

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/sensor/Sensor.cpp (right):

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/Sensor.cpp:222: }
On 2017/04/26 14:58:45, Reilly Grant wrote:
> nit: no braces when both if and else clauses are a single line.

Done.

https://codereview.chromium.org/2839963004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/sensor/Sensor.cpp:251: auto start_callback =
WTF::Bind(&Sensor::OnAddConfigurationRequestCompleted,
On 2017/04/26 14:58:45, Reilly Grant wrote:
> nit: no need to save the callback in a local variable before passing it to
> sensor_proxy_->AddConfiguration().

Done.

[commit-bot: I haz the power, 2017-04-27 09:14:04]

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

[commit-bot: I haz the power, 2017-04-27 09:14:05]

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

[Reilly Grant (use Gerrit), 2017-04-27 15:40:16]

lgtm

[Mikhail, 2017-04-27 16:16:04]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[Mikhail, 2017-04-27 16:16:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
alexander.shalamov@intel.com
Link to the patchset: https://codereview.chromium.org/2839963004/#ps60001
(title: "fix test failures")

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

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 16:44:10]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493309764755190,
"parent_rev": "d71abed75b07b4b9fc50591cbeb2b6c8874bf448", "commit_rev":
"e2d622806d0648f17eac947bd9d77b29211ccc2f"}

[commit-bot: I haz the power, 2017-04-27 16:44:23]

Description was changed from

==========
[Sensors] Error handler in Senor class should stop the platform sensor

At the moment SensorProxy is the only source of error notification for the
Sensor class instances, however soon other error notification sources will turn
up (e.g. permission subsystem), so we must guarantee that stopping of a Sensor
due to an error is reflected on platform side.

BUG=606766
==========

to

==========
[Sensors] Error handler in Senor class should stop the platform sensor

At the moment SensorProxy is the only source of error notification for the
Sensor class instances, however soon other error notification sources will turn
up (e.g. permission subsystem), so we must guarantee that stopping of a Sensor
due to an error is reflected on platform side.

BUG=606766

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

[commit-bot: I haz the power, 2017-04-27 16:44:24]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/e2d622806d0648f17eac947bd9d7...