Bug 174349 - Changing the web page muted state for playing audio should not disable other tabs capture
Summary: Changing the web page muted state for playing audio should not disable other ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-07-10 22:04 PDT by youenn fablet
Modified: 2017-07-11 11:08 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.36 KB, patch)
2017-07-10 22:17 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (3.34 KB, patch)
2017-07-11 07:35 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Fixing build (1.46 KB, patch)
2017-07-11 09:09 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-07-10 22:04:47 PDT
When muting or unmuting a tab that plays audio, we currently disable any other tab capturing.
We should make sure to only disable other tabs capturing if the tab will or is capturing.
Comment 1 youenn fablet 2017-07-10 22:06:56 PDT
rdar://problem/33223988
Comment 2 youenn fablet 2017-07-10 22:17:46 PDT
Created attachment 315083 [details]
Patch
Comment 3 youenn fablet 2017-07-10 22:18:41 PDT
I was not sure what to do with interrupted sources.
Currently, they are not taken into account.
Comment 4 Eric Carlson 2017-07-11 06:03:18 PDT
Comment on attachment 315083 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.cpp:4147
> +    bool hasMutedCaptureStreams = (m_mediaState & WebCore::MediaProducer::HasMutedAudioCaptureDevice) || (m_mediaState & WebCore::MediaProducer::HasMutedVideoCaptureDevice);

Nit: "bool hasMutedCaptureStreams = m_mediaState & (WebCore::MediaProducer::HasMutedAudioCaptureDevice | WebCore::MediaProducer::HasMutedVideoCaptureDevice);" would be slightly easier to read
Comment 5 youenn fablet 2017-07-11 07:35:32 PDT
Created attachment 315109 [details]
Patch for landing
Comment 6 youenn fablet 2017-07-11 07:36:15 PDT
Thanks for the review.

(In reply to Eric Carlson from comment #4)
> Comment on attachment 315083 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=315083&action=review
> 
> > Source/WebKit2/UIProcess/WebPageProxy.cpp:4147
> > +    bool hasMutedCaptureStreams = (m_mediaState & WebCore::MediaProducer::HasMutedAudioCaptureDevice) || (m_mediaState & WebCore::MediaProducer::HasMutedVideoCaptureDevice);
> 
> Nit: "bool hasMutedCaptureStreams = m_mediaState &
> (WebCore::MediaProducer::HasMutedAudioCaptureDevice |
> WebCore::MediaProducer::HasMutedVideoCaptureDevice);" would be slightly
> easier to read

Done
Comment 7 WebKit Commit Bot 2017-07-11 08:14:39 PDT
Comment on attachment 315109 [details]
Patch for landing

Clearing flags on attachment: 315109

Committed r219330: <http://trac.webkit.org/changeset/219330>
Comment 8 WebKit Commit Bot 2017-07-11 08:14:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Carlos Alberto Lopez Perez 2017-07-11 08:50:31 PDT
(In reply to WebKit Commit Bot from comment #7)
> Comment on attachment 315109 [details]
> Patch for landing
> 
> Clearing flags on attachment: 315109
> 
> Committed r219330: <http://trac.webkit.org/changeset/219330>

I think this broke with ! ENABLE(MEDIA_STREAM)

FAILED: [...]  -o Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/WebPageProxy.cpp.o -c ../../Source/WebKit2/UIProcess/WebPageProxy.cpp
../../Source/WebKit2/UIProcess/WebPageProxy.cpp: In member function ‘void WebKit::WebPageProxy::activateMediaStreamCaptureInPage()’:
../../Source/WebKit2/UIProcess/WebPageProxy.cpp:1672:5: error: ‘UserMediaProcessManager’ has not been declared
     UserMediaProcessManager::singleton().muteCaptureMediaStreamsExceptIn(*this);
     ^
At global scope:

https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/3615/steps/compile-webkit/logs/stdio
Comment 10 youenn fablet 2017-07-11 08:56:53 PDT
Will fix it
Comment 11 youenn fablet 2017-07-11 09:09:18 PDT
Reopening to attach new patch.
Comment 12 youenn fablet 2017-07-11 09:09:19 PDT
Created attachment 315115 [details]
Fixing build
Comment 13 youenn fablet 2017-07-11 09:18:10 PDT
WPEbot seems happy now, sorry for not catching its red bubble!
Comment 14 Carlos Alberto Lopez Perez 2017-07-11 11:07:39 PDT
The commit queue is taking two much to land this (2 hours already).
And the WPE bots/EWS are currently broken due to this. 

I'm landing it manually.
Comment 15 Carlos Alberto Lopez Perez 2017-07-11 11:08:34 PDT
Comment on attachment 315115 [details]
Fixing build

Clearing flags on attachment: 315115

Committed r219344: <http://trac.webkit.org/changeset/219344>
Comment 16 Carlos Alberto Lopez Perez 2017-07-11 11:08:39 PDT
All reviewed patches have been landed.  Closing bug.