Bug 171292

Summary: [MediaStream] Allow host application to enable/disable media capture
Product: WebKit Reporter: Andrew Gold <agold>
Component: WebKit APIAssignee: Eric Carlson <eric.carlson>
Status: ASSIGNED ---    
Severity: Normal CC: agold, buildbot, commit-queue, eric.carlson, fred.wang, jer.noble, jlewis3, jonlee, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=171728
https://bugs.webkit.org/show_bug.cgi?id=171895
Bug Depends on: 171640, 171654    
Bug Blocks:    
Attachments:
Description Flags
Proposed patch.
jer.noble: review+
Rebased patch.
none
Patch for landing.
commit-queue: commit-queue-
Updated patch.
none
Another patch for landing.
none
Update patch.
none
One more time.
buildbot: commit-queue-
Archive of layout-test-results from ews125 for ios-simulator-wk2 none

Description Andrew Gold 2017-04-25 14:40:35 PDT
Because the media stream has been cut off by the system when the app enters the background, muting the page while the host application is in the background does not properly update the state, and therefore when the app re-enters the foreground the media stream becomes unmuted.
Comment 1 Radar WebKit Bug Importer 2017-04-25 14:41:11 PDT
<rdar://problem/31821492>
Comment 2 Eric Carlson 2017-05-03 15:24:03 PDT
Created attachment 308966 [details]
Proposed patch.
Comment 3 Eric Carlson 2017-05-03 15:31:15 PDT
Created attachment 308972 [details]
Rebased patch.
Comment 4 Jer Noble 2017-05-03 15:33:50 PDT
Comment on attachment 308966 [details]
Proposed patch.

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

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:129
> -    m_ended = true;
> +    if (mode == StopMode::Silently)
> +        m_ended = true;
> +
> +
>      m_private->endTrack();
> +    m_ended = true;

Nit: I think this may deserve a comment; the side effects here are subtle, and it would be a shame if a future developer (e.g., me) accidentally broke this.
Comment 5 Eric Carlson 2017-05-03 15:44:13 PDT
Created attachment 308977 [details]
Patch for landing.
Comment 6 Build Bot 2017-05-03 15:46:12 PDT
Attachment 308977 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/UserMediaProcessManager.cpp:250:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 1 in 35 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 WebKit Commit Bot 2017-05-03 16:25:36 PDT
Comment on attachment 308977 [details]
Patch for landing.

Rejecting attachment 308977 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
WebKit/Tools/WebKitTestRunner/WorkQueueManager.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebKitTestRunner.build/Release/WebKitTestRunner\ (Library).build/Objects-normal/x86_64/WorkQueueManager.o

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebKitTestRunner.build/Release/WebKitTestRunner\ (Library).build/Objects-normal/x86_64/TestController.o TestController.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Full output: http://webkit-queues.webkit.org/results/3667447
Comment 8 Eric Carlson 2017-05-03 16:40:50 PDT
Created attachment 308983 [details]
Updated patch.
Comment 9 Eric Carlson 2017-05-03 16:57:16 PDT
Created attachment 308988 [details]
Another patch for landing.
Comment 10 Eric Carlson 2017-05-03 17:04:11 PDT
Comment on attachment 308988 [details]
Another patch for landing.

Committed r216160: https://trac.webkit.org/r216160
Comment 11 Matt Lewis 2017-05-03 18:03:44 PDT
The iOS builds were still broken after this change:
https://build.webkit.org/builders/Apple%20iOS%2010%20Simulator%20Release%20%28Build%29/builds/1361

Rolled out in:
http://trac.webkit.org/changeset/216162
Comment 12 Ryan Haddad 2017-05-03 18:07:22 PDT
It also looks like there was an API test failures on macOS before this was rolled out:

https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20(Tests)/builds/1252

FAIL MediaCaptureDisabledTest.EnableAndDisable

/Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/UserMediaDisabled.mm:119
Value of: message
  Actual: "allowed"
Expected: [(NSString *)[lastScriptMessage body] UTF8String]
Which is: "gUM allowed"


/Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/UserMediaDisabled.mm:119
Value of: message
  Actual: "denied"
Expected: [(NSString *)[lastScriptMessage body] UTF8String]
Which is: "gUM denied"


/Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/UserMediaDisabled.mm:119
Value of: message
  Actual: "allowed"
Expected: [(NSString *)[lastScriptMessage body] UTF8String]
Which is: "gUM allowed"
Comment 13 Eric Carlson 2017-05-03 21:07:09 PDT
Created attachment 309008 [details]
Update patch.
Comment 14 WebKit Commit Bot 2017-05-03 21:48:11 PDT
Comment on attachment 309008 [details]
Update patch.

Clearing flags on attachment: 309008

Committed r216172: <http://trac.webkit.org/changeset/216172>
Comment 15 Eric Carlson 2017-05-04 09:47:48 PDT
Rolled out in r216183.
Comment 16 Eric Carlson 2017-05-04 10:23:46 PDT
Created attachment 309060 [details]
One more time.
Comment 17 WebKit Commit Bot 2017-05-04 11:32:34 PDT
Comment on attachment 309060 [details]
One more time.

Clearing flags on attachment: 309060

Committed r216197: <http://trac.webkit.org/changeset/216197>
Comment 18 Build Bot 2017-05-04 12:11:15 PDT
Comment on attachment 309060 [details]
One more time.

Attachment 309060 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3673639

New failing tests:
webrtc/audio-replace-track.html
Comment 19 Build Bot 2017-05-04 12:11:17 PDT
Created attachment 309080 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 20 Ryan Haddad 2017-05-04 23:50:46 PDT
(In reply to Build Bot from comment #18)
> Comment on attachment 309060 [details]
> One more time.
> 
> Attachment 309060 [details] did not pass ios-sim-ews (ios-simulator-wk2):
> Output: http://webkit-queues.webkit.org/results/3673639
> 
> New failing tests:
> webrtc/audio-replace-track.html

This test is failing on iOS simulator after r216197 landed (alongside a timeout seen with webrtc/video-replace-track.html)

https://build.webkit.org/builders/Apple%20iOS%2010%20Simulator%20Release%20WK2%20(Tests)/builds/1185
Comment 21 Eric Carlson 2017-05-05 08:56:17 PDT
(In reply to Ryan Haddad from comment #20)
> (In reply to Build Bot from comment #18)
> > Comment on attachment 309060 [details]
> > One more time.
> > 
> > Attachment 309060 [details] did not pass ios-sim-ews (ios-simulator-wk2):
> > Output: http://webkit-queues.webkit.org/results/3673639
> > 
> > New failing tests:
> > webrtc/audio-replace-track.html
> 
> This test is failing on iOS simulator after r216197 landed (alongside a
> timeout seen with webrtc/video-replace-track.html)
> 
> https://build.webkit.org/builders/
> Apple%20iOS%2010%20Simulator%20Release%20WK2%20(Tests)/builds/1185

Filed https://bugs.webkit.org/show_bug.cgi?id=171728.
Comment 22 Jon Lee 2017-05-16 11:08:32 PDT
*** Bug 169304 has been marked as a duplicate of this bug. ***