Bug 196555 - [MediaStream] Host should be able to mute screen capture and camera/microphone independently
Summary: [MediaStream] Host should be able to mute screen capture and camera/microphon...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-03 12:53 PDT by Eric Carlson
Modified: 2019-05-20 11:31 PDT (History)
5 users (show)

See Also:


Attachments
Patch (35.54 KB, patch)
2019-04-03 13:44 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.66 MB, application/zip)
2019-04-03 15:13 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.59 MB, application/zip)
2019-04-03 15:49 PDT, EWS Watchlist
no flags Details
Patch (38.11 KB, patch)
2019-04-04 06:26 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (2.63 MB, application/zip)
2019-04-04 07:53 PDT, EWS Watchlist
no flags Details
Patch (40.33 KB, patch)
2019-04-04 09:29 PDT, Eric Carlson
youennf: review+
Details | Formatted Diff | Diff
Patch (39.08 KB, patch)
2019-04-04 11:29 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>