track.applyConstraints({advanced: []}) causes immediate crash |
||||||||
Issue description
Chrome Version: 60.0.3112.101
Operating System: Mac OS X 10.12.6
Setting the 'advanced' constraints of a MediaStreamTrack to an empty array causes an immediate crash.
This was observed on stable and canary, on Mac and Android (this crash report is for stable on Mac)
Test case:
async function go() {
const stream = await navigator.mediaDevices.getUserMedia({video: true});
const track = stream.getVideoTracks()[0];
track.applyConstraints({advanced: []});
}
go();
****DO NOT CHANGE BELOW THIS LINE****
Crash ID: crash/b8f6e42200f4073b
,
Aug 30 2017
,
Aug 31 2017
Tested the issue using #60.0.3112.113 on Mac 10.12.6 and was unable to reproduce the issue as per the steps mentioned in comment #0. No Crash is observed while running the above script in console. Note: 1. This Crash has first started in M59. 2. This Crash is observed in Windows, Mac and Android as well Using Code Search for the file, "MediaStreamTrack.cpp" assigning to the concern owner who might be related. Suspecting Commit# https://chromium.googlesource.com/chromium/src/+/1d5b6a1cd0258c12363b53f0d382471141463abe @mcasas -- Could you please look into the issue, kindly re-assign if this is not related to your changes. Link to the list of the builds ============================== https://goto.google.com/zytlk Thank You.
,
Aug 31 2017
guidou@ can you PTAL? I'm OOO and should be easily reproduceable.
,
Aug 31 2017
I will take a look.
,
Aug 31 2017
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8b4d097871e37ad6e622bece2939bf33b9e307b commit b8b4d097871e37ad6e622bece2939bf33b9e307b Author: Guido Urdaneta <guidou@chromium.org> Date: Thu Aug 31 11:49:06 2017 Make empty ImageCapture advanced constraint sets clear constraints. Bug: 760522 Change-Id: I11747935573162515b1c029f613593e70c633d61 Reviewed-on: https://chromium-review.googlesource.com/645266 Commit-Queue: Guido Urdaneta <guidou@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/master@{#498821} [modify] https://crrev.com/b8b4d097871e37ad6e622bece2939bf33b9e307b/third_party/WebKit/LayoutTests/fast/imagecapture/MediaStreamTrack-applyConstraints.html [modify] https://crrev.com/b8b4d097871e37ad6e622bece2939bf33b9e307b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp [modify] https://crrev.com/b8b4d097871e37ad6e622bece2939bf33b9e307b/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
,
Aug 31 2017
,
Aug 31 2017
I think we should request merge to M61 before it becomes stable. mcasas@: WDYT?
,
Aug 31 2017
Looping to folks who works for M61.
,
Aug 31 2017
Based on the crash_id provided in bug report, This crash had 106 number of crashes on Android during M59(59.0.3071.125) and just 10 crashes overall on Desktop(including Chrome OS). I am not sure the functional impact and if we have to make a call just based on crash number it's too low crash rate to consider the merge to M61 at this stage, Please find the data here : https://goto.google.com/zrbos Please do correct me and let us know why this should be merged to M61 before it hits stable(as per comment#9).
,
Sep 1 2017
My main argument for merging to M61 is that it removes an easy way to crash the renderer with a very simple (and thus low-risk) patch. However, I will let mcasas@ decide if we should try to make the case for merging to M61 since he is in a better position to judge the impact of both the bug and the fix.
,
Sep 1 2017
+ mcasas@, PTAL comment #12.
,
Sep 1 2017
Merging sounds reasonable code-wise since it has been baked in M62 and it's a low risk feature, however since M61 is very close to cut and the crash rate is quite reduced, we could let it pass and wait for 62 to roll. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by mscales@chromium.org
, Aug 30 2017