Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(104)

Issue 2900583002: Revert of [Mojo Video Capture] Hook up video capture service behind a feature flag (Closed)

Created:
3 years, 7 months ago by tsergeant
Modified:
3 years, 7 months ago
CC:
Aaron Boodman, abarth-chromium, chfremer+watch_chromium.org, chromium-reviews, darin (slow to review), darin-cc_chromium.org, feature-media-reviews_chromium.org, jam, mfoltz+watch_chromium.org, miu+watch_chromium.org, posciak+watch_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, xjz+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of [Mojo Video Capture] Hook up video capture service behind a feature flag (patchset #9 id:180001 of https://codereview.chromium.org/2867213004/ ) Reason for revert: This CL appears to cause test failures on the Webkit Android bots for blink_heap_unittests and webkit_unit_tests. Example failing build: https://luci-milo.appspot.com/buildbot/chromium.webkit/WebKit%20Android%20%28Nexus4%29/65022 OSError: [Errno 2] No such file or directory: '/b/c/b/WebKit_Android__Nexus4_/src/out/Release/gen/services/video_capture/manifest.json' Original issue's description: > [Mojo Video Capture] Hook up video capture service behind a feature flag > > This CL is part of the Mojo Video Capture work. For the bigger picture, > see [1] CL25. > > After this CL, the video capture service can be enabled by starting Chrome with > command-line flag --enable-features=MojoVideoCapture. > > Changes in this CL: > * Add a base::Feature kMojoVideoCapture to allow switching the service on and > off. > * Add a class VideoCaptureProviderSwitcher that can route device capture > requests to the service's VideoCaptureProvider while routing screen capture > requests to the InProcessVideoCaptureProvider. > * Hook up the VideoCaptureProviderSwitcher in the factory code in > MediaStreamManager. > * Register video capture service in the service manager context for the Browser > process. > * Remove build flag "enable_mojo_video_capture", since we want the service to > be testable in all builds. > * Forward switches for using fake video capture devices to utility processes. > * Add test cases for exercising the service to VideoCaptureBrowserTest. > > BUG=584797 > TEST= > content_browsertests --gtest_filter="VideoCaptureBrowserTest.*" > > [1] https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijvI8/edit?usp=sharing > > Review-Url: https://codereview.chromium.org/2867213004 > Cr-Commit-Position: refs/heads/master@{#473358} > Committed: https://chromium.googlesource.com/chromium/src/+/d99842d2bfbffba9b80aaa81aced1f1e7ebb39c8 TBR=rockot@chromium.org,emircan@chromium.org,mcasas@chromium.org,miu@chromium.org,piman@chromium.org,tsepez@chromium.org,chfremer@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=584797 Review-Url: https://codereview.chromium.org/2900583002 Cr-Commit-Position: refs/heads/master@{#473495} Committed: https://chromium.googlesource.com/chromium/src/+/0749fd4d1bf03b14b8d38cd5ccb2b32d0dac0836

Patch Set 1 #

Patch Set 2 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -316 lines) Patch
M content/browser/BUILD.gn View 1 5 chunks +22 lines, -10 lines 0 comments Download
M content/browser/renderer_host/media/media_stream_manager.cc View 1 3 chunks +0 lines, -8 lines 0 comments Download
M content/browser/renderer_host/media/service_video_capture_device_launcher.cc View 1 1 chunk +1 line, -4 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_browsertest.cc View 1 12 chunks +36 lines, -92 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_provider.h View 1 2 chunks +3 lines, -6 lines 0 comments Download
D content/browser/renderer_host/media/video_capture_provider_switcher.h View 1 chunk +0 lines, -38 lines 0 comments Download
D content/browser/renderer_host/media/video_capture_provider_switcher.cc View 1 chunk +0 lines, -88 lines 0 comments Download
M content/browser/service_manager/service_manager_context.cc View 1 3 chunks +0 lines, -8 lines 0 comments Download
M content/browser/utility_process_host_impl.cc View 1 2 chunks +0 lines, -3 lines 0 comments Download
M content/public/app/BUILD.gn View 1 1 chunk +0 lines, -1 line 0 comments Download
M content/public/app/mojo/content_browser_manifest.json View 1 chunk +1 line, -2 lines 0 comments Download
M content/test/BUILD.gn View 1 1 chunk +0 lines, -1 line 0 comments Download
M content/utility/BUILD.gn View 1 1 chunk +0 lines, -2 lines 0 comments Download
M content/utility/DEPS View 1 1 chunk +0 lines, -1 line 0 comments Download
M content/utility/utility_service_factory.cc View 1 2 chunks +0 lines, -15 lines 0 comments Download
M media/capture/video/video_capture_system.h View 1 1 chunk +1 line, -2 lines 0 comments Download
M services/video_capture/device_factory_media_to_mojo_adapter.cc View 1 1 chunk +11 lines, -4 lines 0 comments Download
M services/video_capture/public/cpp/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
D services/video_capture/public/cpp/constants.h View 1 chunk +0 lines, -16 lines 0 comments Download
D services/video_capture/public/cpp/constants.cc View 1 chunk +0 lines, -12 lines 0 comments Download
M services/video_capture/service_manifest.json View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 11 (8 generated)
tsergeant
Created Revert of [Mojo Video Capture] Hook up video capture service behind a feature flag
3 years, 7 months ago (2017-05-22 00:08:13 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2900583002/300001
3 years, 7 months ago (2017-05-22 02:54:56 UTC) #8
commit-bot: I haz the power
3 years, 7 months ago (2017-05-22 02:59:28 UTC) #11
Message was sent while issue was closed.
Committed patchset #2 (id:300001) as
https://chromium.googlesource.com/chromium/src/+/0749fd4d1bf03b14b8d38cd5ccb2...

Powered by Google App Engine
This is Rietveld 408576698