New issue
Advanced search Search tips

Issue 798409 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 788965
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 721427



Sign in to add a comment

Device Orientation: race condition leading to `OnSensorCreated` called twice

Project Member Reported by mlamouri@chromium.org, Jan 2 2018

Issue description

STR:
- Open Chromium with debug enabled
- Open a YouTube video
- Reload the page

Expected result: all is working fine
Actual result: renderer crash

Reason seems to be that YT creates two <video> which creates two device orientation listeners. The first one creates the sensor provider mojo binding and calls `GetSensor`. The second one sees that there is no sensor but binding so calls `GetSensor` again. It leads to a crash as `OnSensorCreated` isn't expected to be called twice.
 
Blocking: 721427
Mergedinto: 788965
Status: Duplicate (was: Assigned)
This is the same as  issue 788965 , juncai@ has a patch out for review that fixes it that's been delayed by the holidays: https://chromium-review.googlesource.com/c/chromium/src/+/798595
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ecc9bc569b81f128ffb3a4a7f702bdacd39c5ae

commit 1ecc9bc569b81f128ffb3a4a7f702bdacd39c5ae
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Wed Jan 03 19:15:26 2018

Device Sensor: early exit if sensors was already created.

This is a fix around what looks like a race where `GetSensor` is called
twice and leads to `OnSensorCreated` to also be called twice. However,
the latter method doesn't expects this and has direct and indirects
checks to prevent it.

Bug:  798409 
Change-Id: Ic2a3acd72eec2a49334551347cb3fd690377e3ed
Reviewed-on: https://chromium-review.googlesource.com/847003
Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526760}
[modify] https://crrev.com/1ecc9bc569b81f128ffb3a4a7f702bdacd39c5ae/content/renderer/device_sensors/device_sensor_event_pump.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 5 2018

Labels: merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fbeb6df58442b011d5ebccad251286dcb15dc9f7

commit fbeb6df58442b011d5ebccad251286dcb15dc9f7
Author: Reilly Grant <reillyg@chromium.org>
Date: Fri Jan 05 19:55:15 2018

[M-64] Revert DeviceOrientationEventPump refactoring

This patch reverts the following two changes in order to resolve
regressions introduced in M-64 which will be fixed on HEAD with other
changes:

"Reland: Refactor DeviceOrientationEventPump to use generic sensor as its backend"
https://chromium-review.googlesource.com/726261

"Return Sensor pipe in SensorInitParams"
https://chromium-review.googlesource.com/757283

This patch was tested by running all *Sensor* tests in unit_tests,
browser_tests, content_unittests, content_browsertests and
services_unittests, by verifying that "rotate to full screen"
works on youtube.com and by running the one-shot.html test from
 issue 799043 .

Bug:  721427 , 794713 , 798409 , 799043 
Change-Id: I2c0573578d6a333b62aa9fd53a57d08d94454783
Reviewed-on: https://chromium-review.googlesource.com/850822
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#423}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/browser/device_sensors/device_sensor_browsertest.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/browser/generic_sensor/sensor_provider_proxy_impl.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/browser/generic_sensor/sensor_provider_proxy_impl.h
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/browser/generic_sensor_browsertest.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/renderer/device_sensors/device_motion_event_pump.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/renderer/device_sensors/device_motion_event_pump.h
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/renderer/device_sensors/device_motion_event_pump_unittest.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/renderer/device_sensors/device_orientation_event_pump.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/renderer/device_sensors/device_orientation_event_pump.h
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/renderer/device_sensors/device_orientation_event_pump_unittest.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/renderer/device_sensors/device_sensor_event_pump.h
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/content/renderer/renderer_blink_platform_impl.cc
[delete] https://crrev.com/45377e18a2ec8f726df564835e9e8e8ccaeea6ec/content/test/data/device_sensors/device_orientation_fallback_to_absolute_test.html
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/services/device/device_service.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/services/device/device_service.h
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/services/device/generic_sensor/generic_sensor_service_unittest.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/services/device/generic_sensor/sensor_provider_impl.cc
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/services/device/generic_sensor/sensor_provider_impl.h
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/services/device/public/interfaces/sensor_provider.mojom
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/third_party/WebKit/LayoutTests/sensor/resources/sensor-helpers.js
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/third_party/WebKit/Source/modules/sensor/SensorProxy.cpp
[modify] https://crrev.com/fbeb6df58442b011d5ebccad251286dcb15dc9f7/third_party/WebKit/Source/modules/sensor/SensorProxy.h

Sign in to add a comment