New issue
Advanced search Search tips

Issue 699201 link

Starred by 1 user
Status: Fixed
Owner:
braveyao@chromium.org
Closed: Nov 2017
Cc:
guidou@chromium.org
niklase@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Get test cases for Window/Tab capture in desktop_capture_apitest working again.

Project Member Reported by braveyao@chromium.org, Mar 7 2017 
With recent changes to getUserMedia request in Chromium, now gUM will need the OnStarted event from capture device to get the successCallback. 
Some test cases in 
/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc
/chrome/test/data/extensions/api_test/desktop_capture/test.js
are commented out in CL https://codereview.chromium.org/2721113002/ since the current chooser dialog UI returns fake source id to Window/Tab capture, from which no stream couldn't be generated.

So we need to either fake-out the chooser dialog UI or create interactive_ui_tests to make these test cases working again.

Comment 1 by sergeyu@chromium.org, Mar 14 2017 

Should this be P2 instead of P3?

Comment 2 by braveyao@chromium.org, Mar 14 2017

Labels: -Pri-3 Pri-2
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 28 2017 

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/99206df620e2fc1cbd5df2b260bdb64937d6ab8c

commit 99206df620e2fc1cbd5df2b260bdb64937d6ab8c
Author: braveyao <braveyao@webrtc.org>
Date: Tue Nov 28 00:55:07 2017

[desktopCapture] make FakeDesktopCapturer reachable by Chromium

To make desktopCapture autotest in chromium more meaningful, it's better
to creake fake capturer of each capture type. Here we can reuse the
FakeDesktopCapturer for window capture in chromium.

Bug:  chromium:699201 
Change-Id: Icbe134d99cbd4980bf27fe74c1c629a1469836ea
Reviewed-on: https://webrtc-review.googlesource.com/26360
Reviewed-by: Zijie He <zijiehe@chromium.org>
Commit-Queue: Brave Yao <braveyao@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20901}
[modify] https://crrev.com/99206df620e2fc1cbd5df2b260bdb64937d6ab8c/modules/desktop_capture/BUILD.gn
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5311d6efaa575c768e53ebe6ce09ab33d123bcac

commit 5311d6efaa575c768e53ebe6ce09ab33d123bcac
Author: Weiyong Yao <braveyao@chromium.org>
Date: Wed Nov 29 21:55:40 2017

[DesktopCapture] add back browser test cases for window/tab capture

Since cl https://codereview.chromium.org/2721113002/, gUM needs the
OnStarted event form capture device for the successCallback. (In the past
gUM will succeed unconditionally even before creating capture device.)
At that time some browser test cases for window/tab capture were commented
out because the window/tab capturer can't work with the fake source id and
will fail gUM eventually.

In the cl, we will create fake capturers to handle the fake source id
to give the expected OnStart event for window/tab capture. Then we can
make the browser test for desktop capture really work.

Bug:  699201 
Change-Id: Id503400807f553336605443bf51b8512c1f574d9
Reviewed-on: https://chromium-review.googlesource.com/782704
Commit-Queue: Weiyong Yao <braveyao@chromium.org>
Reviewed-by: Zijie He <zijiehe@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520260}
[modify] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc
[modify] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/chrome/test/data/extensions/api_test/desktop_capture/test.js
[modify] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/content/browser/BUILD.gn
[modify] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/content/browser/media/capture/desktop_capture_device.cc
[add] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/content/browser/media/capture/fake_webcontent_capture_machine.cc
[add] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/content/browser/media/capture/fake_webcontent_capture_machine.h
[modify] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/content/browser/media/capture/web_contents_audio_input_stream.cc
[modify] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/content/browser/media/capture/web_contents_video_capture_device.cc
[modify] https://crrev.com/5311d6efaa575c768e53ebe6ce09ab33d123bcac/content/public/browser/desktop_media_id.h

Comment 5 by braveyao@chromium.org, Nov 29 2017

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7dd4f91905e3c0f7093c99d0dff249ae97b95af

commit b7dd4f91905e3c0f7093c99d0dff249ae97b95af
Author: Scott Little <sclittle@chromium.org>
Date: Wed Nov 29 22:52:10 2017

Revert "[DesktopCapture] add back browser test cases for window/tab capture"

This reverts commit 5311d6efaa575c768e53ebe6ce09ab33d123bcac.

Reason for revert: Broke compile on chromium.linux/Cast Audio Linux bot: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Cast%20Audio%20Linux

Specific failing build: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Cast%20Audio%20Linux/builds/6806

Original change's description:
> [DesktopCapture] add back browser test cases for window/tab capture
> 
> Since cl https://codereview.chromium.org/2721113002/, gUM needs the
> OnStarted event form capture device for the successCallback. (In the past
> gUM will succeed unconditionally even before creating capture device.)
> At that time some browser test cases for window/tab capture were commented
> out because the window/tab capturer can't work with the fake source id and
> will fail gUM eventually.
> 
> In the cl, we will create fake capturers to handle the fake source id
> to give the expected OnStart event for window/tab capture. Then we can
> make the browser test for desktop capture really work.
> 
> Bug:  699201 
> Change-Id: Id503400807f553336605443bf51b8512c1f574d9
> Reviewed-on: https://chromium-review.googlesource.com/782704
> Commit-Queue: Weiyong Yao <braveyao@chromium.org>
> Reviewed-by: Zijie He <zijiehe@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520260}

TBR=avi@chromium.org,braveyao@chromium.org,zijiehe@chromium.org

Change-Id: I1f60dd0a3a3f989d0ab11c62fe3371e68b21dc24
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  699201 
Reviewed-on: https://chromium-review.googlesource.com/797934
Reviewed-by: Scott Little <sclittle@chromium.org>
Commit-Queue: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520286}
[modify] https://crrev.com/b7dd4f91905e3c0f7093c99d0dff249ae97b95af/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc
[modify] https://crrev.com/b7dd4f91905e3c0f7093c99d0dff249ae97b95af/chrome/test/data/extensions/api_test/desktop_capture/test.js
[modify] https://crrev.com/b7dd4f91905e3c0f7093c99d0dff249ae97b95af/content/browser/BUILD.gn
[modify] https://crrev.com/b7dd4f91905e3c0f7093c99d0dff249ae97b95af/content/browser/media/capture/desktop_capture_device.cc
[delete] https://crrev.com/8263e5f94e74576fa320e92a87fb30c40fc249f4/content/browser/media/capture/fake_webcontent_capture_machine.cc
[delete] https://crrev.com/8263e5f94e74576fa320e92a87fb30c40fc249f4/content/browser/media/capture/fake_webcontent_capture_machine.h
[modify] https://crrev.com/b7dd4f91905e3c0f7093c99d0dff249ae97b95af/content/browser/media/capture/web_contents_audio_input_stream.cc
[modify] https://crrev.com/b7dd4f91905e3c0f7093c99d0dff249ae97b95af/content/browser/media/capture/web_contents_video_capture_device.cc
[modify] https://crrev.com/b7dd4f91905e3c0f7093c99d0dff249ae97b95af/content/public/browser/desktop_media_id.h
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 30 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9

commit 5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9
Author: Weiyong Yao <braveyao@chromium.org>
Date: Thu Nov 30 02:14:11 2017

Reland"[DesktopCapture] add back browser test cases for window/tab capture"

Original: https://chromium-review.googlesource.com/c/chromium/src/+/782704
Revert:   https://chromium-review.googlesource.com/c/chromium/src/+/797934

Cast Audio Linux bots doesn't have |enable_webrtc| defined. So the two new
files shouldn't be added there in BUILD.gn as in original cl, but together
with "web_contents_video_capture_device.*"

TBR=avi@chromium.org,zijiehe@chromium.org

BUG:  699201 
Change-Id: I4df15c66d44d40f3379c5e2333d96f8b2b97c065
Reviewed-on: https://chromium-review.googlesource.com/798311
Reviewed-by: Weiyong Yao <braveyao@chromium.org>
Commit-Queue: Weiyong Yao <braveyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520392}
[modify] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc
[modify] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/chrome/test/data/extensions/api_test/desktop_capture/test.js
[modify] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/content/browser/BUILD.gn
[modify] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/content/browser/media/capture/desktop_capture_device.cc
[add] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/content/browser/media/capture/fake_webcontent_capture_machine.cc
[add] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/content/browser/media/capture/fake_webcontent_capture_machine.h
[modify] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/content/browser/media/capture/web_contents_audio_input_stream.cc
[modify] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/content/browser/media/capture/web_contents_video_capture_device.cc
[modify] https://crrev.com/5ac2e62976d8f74562719ec6ca0fe3e215bf9fe9/content/public/browser/desktop_media_id.h
►

Sign in to add a comment