Bug 196555

Summary: [MediaStream] Host should be able to mute screen capture and camera/microphone independently
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: WebRTCAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, rniwa, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews105 for mac-highsierra-wk2
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews107 for mac-highsierra-wk2
none
Patch
youennf: review+
Patch none

Description Eric Carlson 2019-04-03 12:53:30 PDT
Host should be able to mute screen capture and camera/microphone independently
Comment 1 Eric Carlson 2019-04-03 12:53:44 PDT
<rdar://problem/47303865>
Comment 2 Eric Carlson 2019-04-03 13:44:28 PDT
Created attachment 366637 [details]
Patch
Comment 3 youenn fablet 2019-04-03 15:04:18 PDT
Comment on attachment 366637 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366637&action=review

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:186
> +    case CaptureDevice::DeviceType::Window:

Probably DeviceType::Window and DeviceType::Screen should use ScreenCaptureIsMuted.

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:409
> +        if (source().interrupted() && !(pageMutedState & AudioAndVideoCaptureIsMuted))

Could we do:
if (source().interrupted() && !source().muted())

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:418
> +        if (source().interrupted() && !(pageMutedState & MediaStreamCaptureIsMuted))

Ditto here.
Comment 4 EWS Watchlist 2019-04-03 15:13:08 PDT
Comment on attachment 366637 [details]
Patch

Attachment 366637 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11755588

New failing tests:
imported/w3c/web-platform-tests/webrtc/RTCRtpTransceiver.https.html
fast/mediastream/media-stream-track-muted.html
imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-addTransceiver.https.html
imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-transceivers.https.html
fast/mediastream/RTCPeerConnection-add-removeTrack.html
Comment 5 EWS Watchlist 2019-04-03 15:13:09 PDT
Created attachment 366653 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 6 EWS Watchlist 2019-04-03 15:49:32 PDT
Comment on attachment 366637 [details]
Patch

Attachment 366637 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11755682

New failing tests:
imported/w3c/web-platform-tests/webrtc/RTCRtpTransceiver.https.html
imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-addTransceiver.https.html
imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-transceivers.https.html
fast/mediastream/RTCPeerConnection-add-removeTrack.html
Comment 7 EWS Watchlist 2019-04-03 15:49:34 PDT
Created attachment 366660 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 8 Eric Carlson 2019-04-04 06:26:57 PDT
Created attachment 366707 [details]
Patch
Comment 9 EWS Watchlist 2019-04-04 07:53:24 PDT
Comment on attachment 366707 [details]
Patch

Attachment 366707 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11763936

New failing tests:
fast/mediastream/media-stream-track-interrupted.html
Comment 10 EWS Watchlist 2019-04-04 07:53:26 PDT
Created attachment 366711 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 11 Eric Carlson 2019-04-04 09:29:59 PDT
Created attachment 366722 [details]
Patch
Comment 12 youenn fablet 2019-04-04 11:13:03 PDT
Comment on attachment 366722 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366722&action=review

> Source/WebCore/Modules/mediastream/MediaStream.cpp:120
> +        setCaptureTracksMuted(document()->page()->mutedState());

Can probably remove these lines.

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:71
> +    if (auto document = this->document()) {

This if check is probably not needed.
document() should return a valid pointer here anyway.
See also the document()->logger call.

As a future refactoring, we could make constructor take a Document& instead.

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:464
> +        AtomicString eventType = muted() ? eventNames().muteEvent : eventNames().unmuteEvent;

let's capture muted state in the lambda.

> Source/WebCore/testing/Internals.cpp:4062
> +            state |= MediaProducer::ScreenCaptureIsMuted;

It would be nice to setPageMuted through testRunner and WebView API.

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:56
> +    _WKMediaScreenCaptureMuted =  1 << 2,

s/  / /

> Source/WebKit/UIProcess/WebPageProxy.cpp:5344
> +    bool hasMutedCaptureStreams = m_mediaState & (WebCore::MediaProducer::MutedCaptureMask);

No need for ()
Comment 13 Eric Carlson 2019-04-04 11:29:43 PDT
Created attachment 366732 [details]
Patch
Comment 14 WebKit Commit Bot 2019-04-04 13:44:35 PDT
Comment on attachment 366732 [details]
Patch

Clearing flags on attachment: 366732

Committed r243899: <https://trac.webkit.org/changeset/243899>