Bug 200992 - Adopt AVSystemController_ActiveAudioRouteDidChangeNotification
Summary: Adopt AVSystemController_ActiveAudioRouteDidChangeNotification
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-21 12:52 PDT by Jer Noble
Modified: 2019-08-21 23:17 PDT (History)
3 users (show)

See Also:


Attachments
Patch (7.68 KB, patch)
2019-08-21 12:56 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (7.67 KB, patch)
2019-08-21 13:47 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Follow up Patch (3.22 KB, patch)
2019-08-21 16:11 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2019-08-21 12:52:45 PDT
Adopt AVSystemController_ActiveAudioRouteDidChangeNotification
Comment 1 Jer Noble 2019-08-21 12:53:24 PDT
<rdar://problem/54408993>
Comment 2 Jer Noble 2019-08-21 12:56:22 PDT
Created attachment 376907 [details]
Patch
Comment 3 Eric Carlson 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()"
Comment 4 Jer Noble 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().
Comment 5 Jer Noble 2019-08-21 13:47:16 PDT
Created attachment 376915 [details]
Patch for landing
Comment 6 WebKit Commit Bot 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>
Comment 7 WebKit Commit Bot 2019-08-21 14:30:00 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Jer Noble 2019-08-21 16:11:05 PDT
Reopening to attach new patch.
Comment 9 Jer Noble 2019-08-21 16:11:05 PDT
Created attachment 376943 [details]
Follow up Patch
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2019-08-21 17:11:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Jer Noble 2019-08-21 23:17:28 PDT
Follow-up build fix: 

Committed r248997: <https://trac.webkit.org/changeset/248997>