Bug 211509

Created attachment 398615 [details]
Patch

Created attachment 398620 [details]
Patch

Comment on attachment 398620 [details]
Patch

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

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:451
> +static bool isSourceCapturingForDocument(RealtimeMediaSource& activeSource, Document& document)

This name is slightly unclear about how it works. Maybe something like `isCapturingFromSameSourceAndDocument` or `isCapturingFromSourceAndDocument`?

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:464
> +void MediaStreamTrack::updateAccordingMutedState()

Nit: this name is also slightly unclear about what the method does. Maybe something like `syncToPageMutedState`, or `updateToPageMutedState`?

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:495
> +        bool muted = page->mutedState() & MediaProducer::AudioAndVideoCaptureIsMuted;

s/muted/pageMuted/

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:501
> +        bool muted = page->mutedState() & MediaProducer::AudioAndVideoCaptureIsMuted;

Ditto.

Created attachment 398633 [details]
Patch for landing

(In reply to youenn fablet from comment #4)
> Created attachment 398633 [details]
> Patch for landing

Wrong bug

iOS test is failing as the ability to allow multiple cloned tracks breaks the ability to mute/unmute the active source.

I plan to temporarily disable this GPU process test and reenable it later on.
The tricky thing about fixing this test is that we do not want to resize video frames in WebProcess, so we cannot use RealtimeVideoSource/RealtimeVideoCaptureSource here.

<rdar://problem/62537166>

(In reply to Eric Carlson from comment #3)
> Comment on attachment 398620 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=398620&action=review
> 
> > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:451
> > +static bool isSourceCapturingForDocument(RealtimeMediaSource& activeSource, Document& document)
> 
> This name is slightly unclear about how it works. Maybe something like
> `isCapturingFromSameSourceAndDocument` or `isCapturingFromSourceAndDocument`?

I prefer the current name, we want to know whether the source is used by a track of the given document.

> > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:464
> > +void MediaStreamTrack::updateAccordingMutedState()
> 
> Nit: this name is also slightly unclear about what the method does. Maybe
> something like `syncToPageMutedState`, or `updateToPageMutedState`?

I'll go with updateToPageMutedState

> > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:495
> > +        bool muted = page->mutedState() & MediaProducer::AudioAndVideoCaptureIsMuted;
> 
> s/muted/pageMuted/

OK

> > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:501
> > +        bool muted = page->mutedState() & MediaProducer::AudioAndVideoCaptureIsMuted;
> 
> Ditto.

OK

Created attachment 398713 [details]
Patch

Created attachment 398715 [details]
Patch

Created attachment 398716 [details]
Patch

Created attachment 398720 [details]
Patch

Comment on attachment 398620 [details]
Patch

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

>>> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:451
>>> +static bool isSourceCapturingForDocument(RealtimeMediaSource& activeSource, Document& document)
>> 
>> This name is slightly unclear about how it works. Maybe something like `isCapturingFromSameSourceAndDocument` or `isCapturingFromSourceAndDocument`?
> 
> I prefer the current name, we want to know whether the source is used by a track of the given document.

`isSourceCapturingForTrackInDocument`?

(In reply to Eric Carlson from comment #13)
> Comment on attachment 398620 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=398620&action=review
> 
> >>> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:451
> >>> +static bool isSourceCapturingForDocument(RealtimeMediaSource& activeSource, Document& document)
> >> 
> >> This name is slightly unclear about how it works. Maybe something like `isCapturingFromSameSourceAndDocument` or `isCapturingFromSourceAndDocument`?
> > 
> > I prefer the current name, we want to know whether the source is used by a track of the given document.
> 
> `isSourceCapturingForTrackInDocument`?

OK, sounds good

Created attachment 398843 [details]
Patch for landing

ChangeLog entry in Source/WebKit/ChangeLog is not at the top of the file.

Created attachment 398852 [details]
Patch for landing

Committed r261373: <https://trac.webkit.org/changeset/261373>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398852 [details].