Bug 200992

Summary: Adopt AVSystemController_ActiveAudioRouteDidChangeNotification
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
none
Follow up Patch none

Jer Noble
Reported 2019-08-21 12:52:45 PDT
Adopt AVSystemController_ActiveAudioRouteDidChangeNotification
Attachments
Patch (7.68 KB, patch)
2019-08-21 12:56 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Patch for landing (7.67 KB, patch)
2019-08-21 13:47 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Follow up Patch (3.22 KB, patch)
2019-08-21 16:11 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Jer Noble
Comment 1 2019-08-21 12:53:24 PDT
<rdar://problem/54408993>
Jer Noble
Comment 2 2019-08-21 12:56:22 PDT
Created attachment 376907 [details] Patch
Eric Carlson
Comment 3 2019-08-21 13:00:31 PDT
Comment on attachment 376907 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376907&action=review > Source/WebCore/Modules/mediastream/MediaStream.h:162 > + bool shouldOverridePauseDuringRouteChanges() const { return true; } Nit: I think it would be more logical to flip the question to ask what the class *should* do, something like "bool shouldPauseOnRouteChange()"
Jer Noble
Comment 4 2019-08-21 13:45:26 PDT
(In reply to Eric Carlson from comment #3) > Comment on attachment 376907 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=376907&action=review > > > Source/WebCore/Modules/mediastream/MediaStream.h:162 > > + bool shouldOverridePauseDuringRouteChanges() const { return true; } > > Nit: I think it would be more logical to flip the question to ask what the > class *should* do, something like "bool shouldPauseOnRouteChange()" I had that originally, but wanted the method to be explicit that we were overriding the system's instructions to pause on route changes. I will, however, make it Change() not Changes().
Jer Noble
Comment 5 2019-08-21 13:47:16 PDT
Created attachment 376915 [details] Patch for landing
WebKit Commit Bot
Comment 6 2019-08-21 14:29:58 PDT
Comment on attachment 376915 [details] Patch for landing Clearing flags on attachment: 376915 Committed r248962: <https://trac.webkit.org/changeset/248962>
WebKit Commit Bot
Comment 7 2019-08-21 14:30:00 PDT
All reviewed patches have been landed. Closing bug.
Jer Noble
Comment 8 2019-08-21 16:11:05 PDT
Reopening to attach new patch.
Jer Noble
Comment 9 2019-08-21 16:11:05 PDT
Created attachment 376943 [details] Follow up Patch
WebKit Commit Bot
Comment 10 2019-08-21 17:11:35 PDT
Comment on attachment 376943 [details] Follow up Patch Clearing flags on attachment: 376943 Committed r248978: <https://trac.webkit.org/changeset/248978>
WebKit Commit Bot
Comment 11 2019-08-21 17:11:37 PDT
All reviewed patches have been landed. Closing bug.
Jer Noble
Comment 12 2019-08-21 23:17:28 PDT
Follow-up build fix: Committed r248997: <https://trac.webkit.org/changeset/248997>
Note You need to log in before you can comment on or make changes to this bug.