[sensors] [win] Fix ambient light not working properly Windows 10.

[sensors] [win] Fix ambient light not working properly on Windows 10.

The event listener was never called despite SetEventSink returning
success. Debbuging further it turns out that the event listener was
created in the sensor thread, however the listener was not set in
the sensor thread (from StartSensor()), this seems to confuse COM.

BUG=606766
Committed: https://crrev.com/26947f590d1526ea0cf3d485382da6dc1a068ec5
Cr-Commit-Position: refs/heads/master@{#435513}

[darktears, 2016-11-11 22:31:57]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 22:33:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[darktears, 2016-11-11 22:52:53]

alexis.menard@intel.com changed reviewers:
+ alexander.shalamov@intel.com, mikhail.pozdnyakov@intel.com

[darktears, 2016-11-11 22:52:53]

PTAL

[commit-bot: I haz the power, 2016-11-12 00:04:54]

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

[commit-bot: I haz the power, 2016-11-12 00:04:55]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[darktears, 2016-11-14 22:40:51]

Description was changed from

==========
[sensors] [win] Fix ambient light not working on Lenovo Miix with Windows 10.

The event listener was never called despite SetEventSink returning
success. Debbuging further it turns out that the event listener was
created in the sensor thread, however the listener was not set in
the sensor thread (from StartSensor()), this seems to confuse COM.

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

to

==========
[sensors] [win] Fix ambient light not working properly on Windows 10.

The event listener was never called despite SetEventSink returning
success. Debbuging further it turns out that the event listener was
created in the sensor thread, however the listener was not set in
the sensor thread (from StartSensor()), this seems to confuse COM.

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

[darktears, 2016-11-14 22:43:51]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 22:46:43]

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, 2016-11-14 23:35:07]

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

[commit-bot: I haz the power, 2016-11-14 23:35:07]

Dry run: This issue passed the CQ dry run.

[shalamov, 2016-11-15 07:31:19]

https://codereview.chromium.org/2495073002/diff/20001/device/generic_sensor/p...
File device/generic_sensor/platform_sensor_reader_win.cc (right):

https://codereview.chromium.org/2495073002/diff/20001/device/generic_sensor/p...
device/generic_sensor/platform_sensor_reader_win.cc:287: base::AutoLock
autolock(lock_);
If it is accessed only on sensor thread, I think you can remove lock.

https://codereview.chromium.org/2495073002/diff/20001/device/generic_sensor/p...
device/generic_sensor/platform_sensor_reader_win.cc:294: sensor_active_ = true;
Should this be moved to:

if (!sensor_active_) {
  postTask();
  sensor_active_ = true;
}

then, 

if (FAILED(...)) {
  ...
  sensor_active_ = false;
}

Otherwise, we can post multiple tasks for ListenSensorEvent()

[darktears, 2016-11-15 16:55:25]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-15 16:55:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[darktears, 2016-11-15 16:55:47]

On 2016/11/15 at 07:31:19, alexander.shalamov wrote:
>
https://codereview.chromium.org/2495073002/diff/20001/device/generic_sensor/p...
> File device/generic_sensor/platform_sensor_reader_win.cc (right):
> 
>
https://codereview.chromium.org/2495073002/diff/20001/device/generic_sensor/p...
> device/generic_sensor/platform_sensor_reader_win.cc:287: base::AutoLock
autolock(lock_);
> If it is accessed only on sensor thread, I think you can remove lock.
> 
>
https://codereview.chromium.org/2495073002/diff/20001/device/generic_sensor/p...
> device/generic_sensor/platform_sensor_reader_win.cc:294: sensor_active_ =
true;
> Should this be moved to:
> 
> if (!sensor_active_) {
>   postTask();
>   sensor_active_ = true;
> }
> 
> then, 
> 
> if (FAILED(...)) {
>   ...
>   sensor_active_ = false;

This one is not needed StopSensor() takes care of it.

> }
> 
> Otherwise, we can post multiple tasks for ListenSensorEvent()

[commit-bot: I haz the power, 2016-11-15 17:59:39]

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

[commit-bot: I haz the power, 2016-11-15 17:59:39]

Dry run: This issue passed the CQ dry run.

[shalamov, 2016-11-15 19:11:40]

lgtm

[darktears, 2016-11-15 20:31:48]

alexis.menard@intel.com changed reviewers:
+ reillyg@chromium.org, timvolodine@chomium.org

[Mikhail, 2016-11-21 11:17:40]

https://codereview.chromium.org/2495073002/diff/40001/device/generic_sensor/p...
File device/generic_sensor/platform_sensor_reader_win.cc (right):

https://codereview.chromium.org/2495073002/diff/40001/device/generic_sensor/p...
device/generic_sensor/platform_sensor_reader_win.cc:288:
event_listener_->AddRef();
why is that needed?

[darktears, 2016-11-22 18:30:34]

On 2016/11/21 at 11:17:40, mikhail.pozdnyakov wrote:
>
https://codereview.chromium.org/2495073002/diff/40001/device/generic_sensor/p...
> File device/generic_sensor/platform_sensor_reader_win.cc (right):
> 
>
https://codereview.chromium.org/2495073002/diff/40001/device/generic_sensor/p...
> device/generic_sensor/platform_sensor_reader_win.cc:288:
event_listener_->AddRef();
> why is that needed?

@alexander : I believe it's not needed, or if so then one would expect a Release
in the StopSensor() method.

[shalamov, 2016-11-23 07:57:15]

On 2016/11/22 18:30:34, darktears wrote:
> On 2016/11/21 at 11:17:40, mikhail.pozdnyakov wrote:
> >
>
https://codereview.chromium.org/2495073002/diff/40001/device/generic_sensor/p...
> > File device/generic_sensor/platform_sensor_reader_win.cc (right):
> > 
> >
>
https://codereview.chromium.org/2495073002/diff/40001/device/generic_sensor/p...
> > device/generic_sensor/platform_sensor_reader_win.cc:288:
> event_listener_->AddRef();
> > why is that needed?
> 
> @alexander : I believe it's not needed, or if so then one would expect a
Release
> in the StopSensor() method.

Sorry, missed that, SetEventSink should ref passed interface and
SetEventSink(null) should unref it. Mikhail is right, the line (288) can be
removed.

[darktears, 2016-11-23 16:05:53]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-23 16:06:24]

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, 2016-11-23 17:37:53]

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

[commit-bot: I haz the power, 2016-11-23 17:37:55]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Reilly Grant (use Gerrit), 2016-11-23 18:54:05]

This change makes sense to me but look at the failing test on the
win_chromium_x64_rel_ng bot.

[darktears, 2016-11-23 20:23:50]

On 2016/11/23 at 18:54:05, reillyg wrote:
> This change makes sense to me but look at the failing test on the
win_chromium_x64_rel_ng bot.

Yes I will, I just rebased the patch so I'm going to look.

[darktears, 2016-11-29 00:21:00]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-29 00:22:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[darktears, 2016-11-29 01:16:43]

On 2016/11/29 at 00:22:05, commit-bot wrote:
> Dry run: CQ is trying da patch. Follow status at
>  
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

@reillyg : tests are green, would love to have your final review.

[commit-bot: I haz the power, 2016-11-29 01:37:34]

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

[commit-bot: I haz the power, 2016-11-29 01:37:35]

Dry run: This issue passed the CQ dry run.

[Mikhail, 2016-11-29 05:59:25]

lgtm

[Reilly Grant (use Gerrit), 2016-11-29 16:06:57]

https://codereview.chromium.org/2495073002/diff/80001/device/generic_sensor/p...
File device/generic_sensor/platform_sensor_and_provider_unittest_win.cc (right):

https://codereview.chromium.org/2495073002/diff/80001/device/generic_sensor/p...
device/generic_sensor/platform_sensor_and_provider_unittest_win.cc:483:
EXPECT_TRUE(sensor->StartListening(client.get(), configuration));
StopListening became StartListening here.

[darktears, 2016-11-29 18:29:54]

The CQ bit was checked by alexis.menard@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-29 18:30:13]

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, 2016-11-29 19:33:36]

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

[commit-bot: I haz the power, 2016-11-29 19:33:37]

Dry run: This issue passed the CQ dry run.

[darktears, 2016-11-29 22:38:56]

On 2016/11/29 at 16:06:57, reillyg wrote:
>
https://codereview.chromium.org/2495073002/diff/80001/device/generic_sensor/p...
> File device/generic_sensor/platform_sensor_and_provider_unittest_win.cc
(right):
> 
>
https://codereview.chromium.org/2495073002/diff/80001/device/generic_sensor/p...
> device/generic_sensor/platform_sensor_and_provider_unittest_win.cc:483:
EXPECT_TRUE(sensor->StartListening(client.get(), configuration));
> StopListening became StartListening here.

oops, sorry about that. Fixed now.

[Reilly Grant (use Gerrit), 2016-11-29 22:52:46]

lgtm

[darktears, 2016-12-01 00:22:00]

The CQ bit was checked by alexis.menard@intel.com

[darktears, 2016-12-01 00:22:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
alexander.shalamov@intel.com, mikhail.pozdnyakov@intel.com
Link to the patchset: https://codereview.chromium.org/2495073002/#ps100001
(title: "Fix typo")

[commit-bot: I haz the power, 2016-12-01 00:22:28]

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, 2016-12-01 01:15:02]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1480551720432320,
"parent_rev": "8f38134a2a4b64371355bed28c7f38c9bf1a9896", "commit_rev":
"9c43c2789a6472e9471ffdd59ea3a4e968ab8bbf"}

[commit-bot: I haz the power, 2016-12-01 01:16:05]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-12-01 01:21:08]

Description was changed from

==========
[sensors] [win] Fix ambient light not working properly on Windows 10.

The event listener was never called despite SetEventSink returning
success. Debbuging further it turns out that the event listener was
created in the sensor thread, however the listener was not set in
the sensor thread (from StartSensor()), this seems to confuse COM.

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

to

==========
[sensors] [win] Fix ambient light not working properly on Windows 10.

The event listener was never called despite SetEventSink returning
success. Debbuging further it turns out that the event listener was
created in the sensor thread, however the listener was not set in
the sensor thread (from StartSensor()), this seems to confuse COM.

BUG=606766

Committed: https://crrev.com/26947f590d1526ea0cf3d485382da6dc1a068ec5
Cr-Commit-Position: refs/heads/master@{#435513}
==========

[commit-bot: I haz the power, 2016-12-01 01:21:09]

Patchset 6 (id:??) landed as
https://crrev.com/26947f590d1526ea0cf3d485382da6dc1a068ec5
Cr-Commit-Position: refs/heads/master@{#435513}