SelectSettings algorithm for video-content capture should run on the main thread |
||||||||
Issue descriptionThere are some code paths where a Blink string might be accessed on the worker thread, which triggers a DCHECK. #0 0x7fc16f737dbd base::debug::StackTrace::StackTrace() #1 0x7fc16f7361ec base::debug::StackTrace::StackTrace() #2 0x7fc16f7bcf7a logging::LogMessage::~LogMessage() #3 0x7fc15b6b7069 WTF::StringImpl::AddRef() #4 0x7fc15b6b6f25 scoped_refptr<>::AddRef() #5 0x7fc15b6cf775 scoped_refptr<>::scoped_refptr() #6 0x7fc15b6cf4dd WTF::String::String() #7 0x7fc15b7fff71 blink::WebString::ContainsOnlyASCII() #8 0x7fc15b7ffc06 blink::WebString::Ascii() #9 0x7fc16984c10f content::StringSetFromConstraint() #10 0x7fc16984e9bf content::(anonymous namespace)::VideoContentCaptureCandidates::VideoContentCaptureCandidates() #11 0x7fc16984e2ac content::SelectSettingsVideoContentCapture()
,
Dec 5 2017
,
Dec 5 2017
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/112c4ff7c946102b9a76eaa9bef37655686a9e58 commit 112c4ff7c946102b9a76eaa9bef37655686a9e58 Author: Guido Urdaneta <guidou@chromium.org> Date: Tue Dec 05 16:04:43 2017 Run constraints processing for screen capture on the main thread. Some code paths require access to Blink strings that can only be accessed on the main thread. A test that triggers the bug is coming up on crrev.com/c/738040. Bug: 791992 Change-Id: I55ae617ae623766d4bd975a19070bde410d8af1d Reviewed-on: https://chromium-review.googlesource.com/809005 Reviewed-by: Henrik Boström <hbos@chromium.org> Commit-Queue: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#521708} [modify] https://crrev.com/112c4ff7c946102b9a76eaa9bef37655686a9e58/content/renderer/media/user_media_processor.cc [modify] https://crrev.com/112c4ff7c946102b9a76eaa9bef37655686a9e58/content/renderer/media/user_media_processor.h
,
Dec 5 2017
,
Dec 5 2017
,
Dec 5 2017
Please add affected OSs.
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/363560ef6a40f1113a1440da53fc0fcb20de40e2 commit 363560ef6a40f1113a1440da53fc0fcb20de40e2 Author: Derek Cheng <imcheng@chromium.org> Date: Tue Dec 05 22:47:09 2017 Revert "Run constraints processing for screen capture on the main thread." This reverts commit 112c4ff7c946102b9a76eaa9bef37655686a9e58. Reason for revert: This might have broken browser_tests on Mac ASan 64 Tests (1): https://uberchromegw.corp.google.com/i/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/36626 Original change's description: > Run constraints processing for screen capture on the main thread. > > Some code paths require access to Blink strings that can only be > accessed on the main thread. > A test that triggers the bug is coming up on crrev.com/c/738040. > > Bug: 791992 > Change-Id: I55ae617ae623766d4bd975a19070bde410d8af1d > Reviewed-on: https://chromium-review.googlesource.com/809005 > Reviewed-by: Henrik Boström <hbos@chromium.org> > Commit-Queue: Guido Urdaneta <guidou@chromium.org> > Cr-Commit-Position: refs/heads/master@{#521708} TBR=hbos@chromium.org,guidou@chromium.org Change-Id: Ibe39bacdedfbf54fac48a6469ec59c4f32381fd1 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 791992 Reviewed-on: https://chromium-review.googlesource.com/809240 Reviewed-by: Derek Cheng <imcheng@chromium.org> Commit-Queue: Derek Cheng <imcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#521872} [modify] https://crrev.com/363560ef6a40f1113a1440da53fc0fcb20de40e2/content/renderer/media/user_media_processor.cc [modify] https://crrev.com/363560ef6a40f1113a1440da53fc0fcb20de40e2/content/renderer/media/user_media_processor.h
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f28f29f4bc751439a347c129a3b6ba5a43980d66 commit f28f29f4bc751439a347c129a3b6ba5a43980d66 Author: Derek Cheng <imcheng@chromium.org> Date: Tue Dec 05 23:20:28 2017 Revert "Revert "Run constraints processing for screen capture on the main thread."" This reverts commit 363560ef6a40f1113a1440da53fc0fcb20de40e2. Reason for revert: The revert broke compile. Will reland and disable the failing test instead. Original change's description: > Revert "Run constraints processing for screen capture on the main thread." > > This reverts commit 112c4ff7c946102b9a76eaa9bef37655686a9e58. > > Reason for revert: This might have broken browser_tests on Mac ASan 64 Tests (1): https://uberchromegw.corp.google.com/i/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/36626 > > Original change's description: > > Run constraints processing for screen capture on the main thread. > > > > Some code paths require access to Blink strings that can only be > > accessed on the main thread. > > A test that triggers the bug is coming up on crrev.com/c/738040. > > > > Bug: 791992 > > Change-Id: I55ae617ae623766d4bd975a19070bde410d8af1d > > Reviewed-on: https://chromium-review.googlesource.com/809005 > > Reviewed-by: Henrik Boström <hbos@chromium.org> > > Commit-Queue: Guido Urdaneta <guidou@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#521708} > > TBR=hbos@chromium.org,guidou@chromium.org > > Change-Id: Ibe39bacdedfbf54fac48a6469ec59c4f32381fd1 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 791992 > Reviewed-on: https://chromium-review.googlesource.com/809240 > Reviewed-by: Derek Cheng <imcheng@chromium.org> > Commit-Queue: Derek Cheng <imcheng@chromium.org> > Cr-Commit-Position: refs/heads/master@{#521872} TBR=hbos@chromium.org,imcheng@chromium.org,guidou@chromium.org Change-Id: Ia5cf4ca4e14d3f83aa93c6e9aad71c3348a2467d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 791992 Reviewed-on: https://chromium-review.googlesource.com/809887 Reviewed-by: Derek Cheng <imcheng@chromium.org> Commit-Queue: Derek Cheng <imcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#521882} [modify] https://crrev.com/f28f29f4bc751439a347c129a3b6ba5a43980d66/content/renderer/media/user_media_processor.cc [modify] https://crrev.com/f28f29f4bc751439a347c129a3b6ba5a43980d66/content/renderer/media/user_media_processor.h
,
Dec 6 2017
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 7 2017
Please add affected OSs.
,
Dec 7 2017
,
Dec 8 2017
Can you please confirm what exactly needs to be merged? Since this was reverted?
,
Dec 9 2017
We want to merge the original CL, which was reverted and relanded without modification.
,
Dec 11 2017
Can you please also comment on why this is critical for M64? What are the implications if we take this in M65 instead? We are promoting M64 Beta to this week and would like to ensure we only take critical and necessary merges.
,
Dec 13 2017
friendly ping on this.
,
Dec 14 2017
I don't think this is critical for M64. This fixes a bug we found out about while introducing a new test. The bug is not known to be causing issues in the real world. The request was in case it was still on time without disturbing the beta release. I'm withdrawing the merge request assuming only critical patches are accepted at this point. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by guidou@chromium.org
, Dec 5 2017