SensorProxy::UpdateSuspendedStatus doesn't match expected behavior or spec |
|||||||
Issue descriptionSpec: https://www.w3.org/TR/generic-sensor/#focused-area When a cross-origin iframe is focused, we get the focused frame, then call IsCrossOriginSubframe, and see that its origin is different from the top frame, and call Suspend. When a non-cross-origin iframe is focused, we get the focused frame, determine it isn't cross-origin, and Resume. For in-process cross-origin iframes, this means that we'll get sensor data in the iframe only when the iframe is not focused. It seems like we should check our current frame vs. the focused frame and determine if we are same-origin.
,
Jun 5 2018
It would be nice to get this into M68.
,
Jun 5 2018
+alexander.shalamov@ as FYI. I can look into fixing this but I am traveling this week so I may not get to it immediately. Assigning this to myself (Available issues should not have an owner assigned).
,
Jun 5 2018
,
Jun 5 2018
reillyg@ I quickly made wip CL, https://chromium-review.googlesource.com/c/chromium/src/+/1087049 I'm not sure if you already working on the issue. If you want, I can clean-up CL and think about the tests. billorr@ Thanks for reporting, do you happen to have html snippet that expose this issue?
,
Jun 5 2018
This was reported internally by a team using the webvr polyfill. In theory sticking any of the (polyfill) links from https://webvr.info/samples/ in an iframe should repro. I reached out to the internal reporter to see if I could share their repro.
,
Jun 8 2018
Updated https://chromium-review.googlesource.com/c/chromium/src/+/1087049 and added tests. If someone have other tests in mind, please add comment here or in the gerrit.
,
Jun 8 2018
To repro, you'd want to stick any of the https://immersive-web.github.io/webxr-samples/ into an iframe. These samples use webxr-polyfill.js that by default uses the RelativeOrientationSensor. https://webvr.info/samples/ use an old webvr-polyfill.js 0.9.41 that depends on the devicemotion event only. RelativeOrientationSenosr is used in more recent versions (>=0.10.3).
,
Jun 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f73dfaffea91c083fe003a96728df96d9e535493 commit f73dfaffea91c083fe003a96728df96d9e535493 Author: Alexander Shalamov <alexander.shalamov@intel.com> Date: Mon Jun 11 19:10:01 2018 [sensors] Check that sensor frame and focused frame of same origin In order to implement mitigation stragegy defined in [1], focused frame and frame associated with the sensor should be of the same origin. [1] https://www.w3.org/TR/generic-sensor/#focused-area Bug: 849501 Change-Id: I06fce4e66873cd98e59e45d00ffe27de8e7d66f9 Reviewed-on: https://chromium-review.googlesource.com/1087049 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Commit-Queue: Alexander Shalamov <alexander.shalamov@intel.com> Cr-Commit-Position: refs/heads/master@{#566096} [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/accelerometer/Accelerometer-iframe-access.https-expected.txt [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/accelerometer/Accelerometer-iframe-access.https.html [modify] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/generic-sensor/generic-sensor-feature-policy-test.sub.js [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/generic-sensor/generic-sensor-iframe-tests.sub.js [modify] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/generic-sensor/generic-sensor-tests.js [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/generic-sensor/resources/iframe_sensor_handler.html [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/gyroscope/Gyroscope-iframe-access.https.html [modify] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/lint.whitelist [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/magnetometer/Magnetometer-iframe-access.https-expected.txt [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/magnetometer/Magnetometer-iframe-access.https.html [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/orientation-sensor/AbsoluteOrientationSensor-iframe-access.https.html [add] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/WebKit/LayoutTests/external/wpt/orientation-sensor/RelativeOrientationSensor-iframe-access.https.html [modify] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/blink/renderer/modules/sensor/sensor_proxy.cc [modify] https://crrev.com/f73dfaffea91c083fe003a96728df96d9e535493/third_party/blink/renderer/modules/sensor/sensor_proxy.h
,
Jun 12 2018
I made two tests that check same-origin and cross-origin focus traversal. I think it is working according to the specification. billorr@ Would you be able to ask original reporter to check whether the issue is resolved?
,
Jul 4
,
Jul 11
This was fixed after M68 branch cut and not merged back.
,
Jul 12
Is this something we need in M68? I can take care of merging (which needs to be done together with https://chromium-review.googlesource.com/c/chromium/src/+/1127048).
,
Jul 16
Friendly ping
,
Jul 16
This does not need to be merged to M-68. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by billorr@chromium.org
, Jun 5 2018