iOS WebRTC Media Capture should not allow camera capture from background tab
radar 32199028
rdar://problem/32199028
Created attachment 310324 [details] Patch
Created attachment 310348 [details] Patch
Comment on attachment 310348 [details] Patch Attachment 310348 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3757145 New failing tests: platform/ios/mediastream/getUserMedia-disabled-in-background-tabs.html
Created attachment 310351 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 310348 [details] Patch Attachment 310348 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3761981 New failing tests: http/tests/media/hls/video-controls-live-stream.html
Created attachment 310382 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 310348 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310348&action=review > Source/WebCore/ChangeLog:11 > + On iOS, muting/unmuting the current video source according Document visibility. Nit: "muting/unmuting" => "mute/unmute", "according" => "according to" > Source/WebCore/dom/Document.cpp:6991 > +void Document::notifyVisibilityChangedToMediaCapture() > +{ > +#if ENABLE(MEDIA_STREAM) > + RealtimeMediaSourceCenter::singleton().setVisibility(!hidden()); > +#endif > +} Nit: why is it necessary to put this one line call into a separate method?
Comment on attachment 310348 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310348&action=review > LayoutTests/platform/ios/mediastream/getUserMedia-disabled-in-background-tabs.html:19 > + let audioTrack = firstStream.getAudioTracks()[0]; > + let videoTrack = firstStream.getVideoTracks()[0]; Where is firstStream set?
Created attachment 310394 [details] Patch for landing
(In reply to Eric Carlson from comment #9) > Comment on attachment 310348 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=310348&action=review > > > Source/WebCore/ChangeLog:11 > > + On iOS, muting/unmuting the current video source according Document visibility. > > Nit: "muting/unmuting" => "mute/unmute", "according" => "according to" > > > Source/WebCore/dom/Document.cpp:6991 > > +void Document::notifyVisibilityChangedToMediaCapture() > > +{ > > +#if ENABLE(MEDIA_STREAM) > > + RealtimeMediaSourceCenter::singleton().setVisibility(!hidden()); > > +#endif > > +} > > Nit: why is it necessary to put this one line call into a separate method? I did not want to pollute Document::visibilityStateChanged with too specific stuff. That is why I went this way. I can go back and inline it directly though. (In reply to Eric Carlson from comment #10) > Comment on attachment 310348 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=310348&action=review > > > LayoutTests/platform/ios/mediastream/getUserMedia-disabled-in-background-tabs.html:19 > > + let audioTrack = firstStream.getAudioTracks()[0]; > > + let videoTrack = firstStream.getVideoTracks()[0]; > > Where is firstStream set? Fixed in the uploaded patch, I'll make sure it passes on the iOS simulator bot before committing the change. Patch works on the device (manual testing)
Comment on attachment 310394 [details] Patch for landing Attachment 310394 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3763551 New failing tests: platform/ios/mediastream/getUserMedia-disabled-in-background-tabs.html
Created attachment 310410 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 310429 [details] Patch for landing
Let's land it as is so that we have the correct behavior in the regular case. And let's fix the more infrequent case of fullscreen mode (where camera should not be stopped) on a follow-up
Comment on attachment 310429 [details] Patch for landing Clearing flags on attachment: 310429 Committed r216999: <http://trac.webkit.org/changeset/216999>
All reviewed patches have been landed. Closing bug.
(In reply to WebKit Commit Bot from comment #17) > Comment on attachment 310429 [details] > Patch for landing > > Clearing flags on attachment: 310429 > > Committed r216999: <http://trac.webkit.org/changeset/216999> This change broke the Windows build: https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/1574 WebCoreTestSupport.lib(JSInternals.obj) : error LNK2019: unresolved external symbol "public: void __thiscall WebCore::Internals::setPageVisibility(bool)" (?setPageVisibility@Internals@WebCore@@QAEX_N@Z) referenced in function "__int64 __cdecl WebCore::jsInternalsPrototypeFunctionSetPageVisibilityCaller(class JSC::ExecState *,class WebCore::JSInternals *,class JSC::ThrowScope &)" (?jsInternalsPrototypeFunctionSetPageVisibilityCaller@WebCore@@YA_JPAVExecState@JSC@@PAVJSInternals@1@AAVThrowScope@3@@Z) [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Tools\DumpRenderTree\DumpRenderTreeLib.vcxproj]
Thanks Ryan, I'll fix it asap (In reply to Ryan Haddad from comment #19) > (In reply to WebKit Commit Bot from comment #17) > > Comment on attachment 310429 [details] > > Patch for landing > > > > Clearing flags on attachment: 310429 > > > > Committed r216999: <http://trac.webkit.org/changeset/216999> > > This change broke the Windows build: > https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/ > 1574 > > WebCoreTestSupport.lib(JSInternals.obj) : error LNK2019: unresolved external > symbol "public: void __thiscall WebCore::Internals::setPageVisibility(bool)" > (?setPageVisibility@Internals@WebCore@@QAEX_N@Z) referenced in function > "__int64 __cdecl > WebCore::jsInternalsPrototypeFunctionSetPageVisibilityCaller(class > JSC::ExecState *,class WebCore::JSInternals *,class JSC::ThrowScope &)" > (?jsInternalsPrototypeFunctionSetPageVisibilityCaller@WebCore@@YA_JPAVExecSta > te@JSC@@PAVJSInternals@1@AAVThrowScope@3@@Z) > [C:\cygwin\home\buildbot\slave\win- > release\build\WebKitBuild\Release\Tools\DumpRenderTree\DumpRenderTreeLib. > vcxproj]
Filed bug 172257
Reopening to attach new patch.
Created attachment 311708 [details] Patch
Test was improved as part of bug 172858.