Issue metadata
Sign in to add a comment
|
"devicemotion" fires a single event with `null` values if `document.write` is called.
Reported by
maximili...@gmail.com,
Nov 10 2017
|
||||||||||||||||||||||
Issue descriptionSteps to reproduce the problem: 1. Close down the Chrome app completely 2. Open index.html and notice that only a single `DeviceMotionEvent` is fired with null sensor values. (Using the console in dev tools, adding an event listener to "devicemotion" reproduces this effect.) 3. Open index-fix.html in a new tab and see that the issue is fixed by a javascript workaround (see comments). 4. Go back to the tab with index.html and see that the issue is now fixed and "devicemotion" fires regularly. What is the expected behavior? index.html should report the values from the DeviceMotionEvent's `accelerometer` at a regular interval without steps 3 and 4. What went wrong? Only a single `DeviceMotionEvent` is fired with all sensor values being null. Did this work before? Yes 61.0.3163 Chrome version: 62.0.3202.89 Channel: stable OS Version: OS X 10.12.6 Flash Version: This is specifically for mobile devices, and several devices were tested (both Android 6 and 7 devices). This reliably reproduced and corrected the issue on all devices. It is expected that this might address other issues like: https://bugs.chromium.org/p/chromium/issues/detail?id=783557 https://bugs.chromium.org/p/chromium/issues/detail?id=779443 A workaround was found by adding an event listener to "devicemotion" before calling `document.write`. This works even if it takes place in another tab; navigating back to the original tab will then have the DeviceMotionEvents appearing correctly. Two files are attached, where index.html shows the bug; but, index-fix.html corrects the issue with the aforementioned workaround. What is very odd is that reopening the index.html after index-fix.html fixes the issue. It seems that using `document.write` to inject html that registers an event listener to "devicemotion" causes an error in reporting `DeviceMotionEvent`s. This is only an issue until another tab adds an event listener to "devicemotion" on the parent window.
,
Nov 10 2017
,
Nov 10 2017
Those other two issues were specifically related to WebView so I don't think they're related. Adding Jun to debug.
,
Nov 10 2017
,
Nov 10 2017
Ok, thanks. Also, I'd like to add that Chrome beta (63.0.3239.41) has the same issue. The same workaround gets devicemotion firing again.
,
Nov 13 2017
In the two exmaples that were provided, the code tries to use devicemotion in an iframe. And the difference is that the index.html adds the devicemotion event from an iframe, and the index-fix.html adds the devicemotion event both in a top level frame and an iframe. Some further debugging regarding the |embedding_origin| and |requesting_origin|: https://cs.chromium.org/chromium/src/content/browser/generic_sensor/sensor_provider_proxy_impl.cc?l=65-66 shows that for the index.html, the two variables are: embedding_origin is: file:///storage/emulated/0/Download/index.html requesting_origin is: about:blank and for index-fix.html, the two variables are both: file:///storage/emulated/0/Download/index-fix.html So the SensorProviderProxyImpl::CheckPermission() returns false for the index.html: https://cs.chromium.org/chromium/src/content/browser/generic_sensor/sensor_provider_proxy_impl.cc?l=58 which causes the devicemotion event not available in the iframe. Generic sensor is not allowed in cross-origin iframes.
,
Nov 13 2017
This appears to be an accidental change in web exposed behavior. To fix it we should update SensorProviderProxyImpl to allow connections from cross-origin iframes when the requested sensor type is one of the ones used for the legacy events. (See IsExtraSensorClass in sensor_provider_impl.cc for a function that already has this list.) We can put the cross-origin iframe check into the Generic Sensor code in the renderer so that this remains forbidden when using the new API. Note that issue 598674 tracks making this the intended behavior as that this is what the specification recommends.
,
Nov 15 2017
,
Nov 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/99cbeffa5daf94b216c4d4f94ffbcd2fd1fa4c0a commit 99cbeffa5daf94b216c4d4f94ffbcd2fd1fa4c0a Author: Jun Cai <juncai@chromium.org> Date: Mon Nov 20 23:57:47 2017 Allow sensors needed for device motion/orientation events in cross-origin iframes Device motion/orientation events are allowed from cross-origin iframes, and this is the intended behavior as discussed at: https://bugs.chromium.org/p/chromium/issues/detail?id=598674 The device motion/orientation events now use the the same backend as the generic sensors API. This broke these events in cross-origin iframes because of checks to prevent such usage in the generic sensors API. To fix this, this CL adds code on the browser side to allow sensor requests from cross-origin iframes when the requested sensor type is needed for device motion/orientation events. When using new generic sensor API, sensors are still not allowed in cross-origin iframes, this is already implemented in blink::Sensor::Sensor() constructor. Bug: 783586 Change-Id: I86bb0f4ddf9e87b78e0fb772043a33bc57b7baa6 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation Reviewed-on: https://chromium-review.googlesource.com/767549 Commit-Queue: Jun Cai <juncai@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Raymes Khoury <raymes@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#518002} [modify] https://crrev.com/99cbeffa5daf94b216c4d4f94ffbcd2fd1fa4c0a/chrome/browser/generic_sensor/sensor_permission_context.cc [modify] https://crrev.com/99cbeffa5daf94b216c4d4f94ffbcd2fd1fa4c0a/content/browser/device_sensors/device_sensor_browsertest.cc [modify] https://crrev.com/99cbeffa5daf94b216c4d4f94ffbcd2fd1fa4c0a/content/browser/generic_sensor_browsertest.cc [add] https://crrev.com/99cbeffa5daf94b216c4d4f94ffbcd2fd1fa4c0a/content/test/data/device_sensors/cross_origin_iframe.html [add] https://crrev.com/99cbeffa5daf94b216c4d4f94ffbcd2fd1fa4c0a/content/test/data/generic_sensor/ambient_light_sensor_cross_origin_iframe_test.html [add] https://crrev.com/99cbeffa5daf94b216c4d4f94ffbcd2fd1fa4c0a/content/test/data/generic_sensor/cross_origin_iframe.html
,
Nov 21 2017
,
Nov 21 2017
Is this fix going into Chrome version 62 or only into future releases? It would be good to have version 62 fixed so that this doesn't remain broken.
,
Nov 21 2017
Since Chrome M62 is now in stable, it is likely that this fix will not be merged to M62. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by manoranj...@chromium.org
, Nov 10 2017