Tab capture tests in DesktopCaptureApiTest.ChooseDesktopMedia are broken |
||||||
Issue descriptionThe tabShareWithAudioPermissionGetStream and tabShareWithoutAudioPermissionGetStream tests are passing, but they should be failing. In a pending WebContentsVideoCaptureDevice CL (https://chromium-review.googlesource.com/c/chromium/src/+/879270), they began failing because the VideoFrameReceiver::OnStarted() callback was corrected to only call back if the WebContents look-up succeeded from the provided device_id string. There are two problems here: 1) The tests pass even though they should be failing. 2) There is an underlying problem with the chrome.desktopCapture.chooseDesktopMedia() API in that it is not providing the correct device ID (the render process ID and render frame ID are invalid) The attached patch illustrates the problem, and the browser_tests output line shows an invalid render_process_id and main_frame_render_id is being provided to WebContentsVideoCaptureDevice::Create(): [142686:142720:0123/150808.199006:ERROR:web_contents_video_capture_device.cc(717)] Create: process_id=-3, frame_id=-3 For now, I am disabling the offending tests to unblock other efforts.
,
Jan 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13e0127baa06ee18ca50becf9780f5cb96c4839d commit 13e0127baa06ee18ca50becf9780f5cb96c4839d Author: Yuri Wiitala <miu@chromium.org> Date: Wed Jan 24 07:01:28 2018 Disable tab capture tests in DesktopCaptureApiTest. See discussion in crbug/805145 for justification. TBR=zijiehe@chromium.org Bug: 805145 Change-Id: I12f03ca95caa852529b4924acadf6a2dec6f476f Reviewed-on: https://chromium-review.googlesource.com/882133 Reviewed-by: Yuri Wiitala <miu@chromium.org> Commit-Queue: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/heads/master@{#531465} [modify] https://crrev.com/13e0127baa06ee18ca50becf9780f5cb96c4839d/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc [modify] https://crrev.com/13e0127baa06ee18ca50becf9780f5cb96c4839d/chrome/test/data/extensions/api_test/desktop_capture/test.js
,
Jan 25 2018
FYI BraveYao@.
,
Jan 25 2018
The FakeWebContentsMediaId will lead to a FakeWebContentCaptureMachine for that browser test. See https://cs.chromium.org/chromium/src/content/browser/media/capture/web_contents_video_capture_device.cc?l=685, which is added in cl https://chromium-review.googlesource.com/c/chromium/src/+/798311. I guess if the content/browser/media/capture/fake_webcontent_capture_machine.cc/h files are included in the new web content capture implementation, then it could work again.
,
Jan 25 2018
Oh, I didn't realize there was a mock/fake embedded in the real thing! Regardless, this is a browser_test, so it should be perfectly fine to to use the real capturer: It's quite simple: Just get the render process ID and main frame of the currently-active tab's WebContents. Then, the fake can be deleted.
,
Jan 25 2018
Oh, that sounds even better! |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by m...@chromium.org
, Jan 23 2018