Bug 218251 – [GPUProcess] Use async IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Chris Dumez]

Use async IPC for RemoteAudioDestinationManager's StartAudioDestination / StopAudioDestination.

[Peng Liu]

*** Bug 217990 has been marked as a duplicate of this bug. ***

[Chris Dumez]

Created attachment 412454 [details]
Patch

[Peng Liu]

Comment on attachment 412454 [details]
Patch

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

> Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:132
> +            completionHandler(Exception { InvalidStateError, "Failed to start the audio device"_s });

Nit. Can be done in one line as in DefaultAudioDestinationNode::resume().

> Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:148
> +        completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to start audio device"_s }));

Nit. Maybe we can use the same error message as in DefaultAudioDestinationNode::startRendering()?
s/"Failed to start audio device"_s/"Failed to start the audio device"_s/

> Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:163
> +        completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to stop audio device"_s }));

Ditto. s/"Failed to stop audio device"_s/"Failed to stop the audio device"_s/

> Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:106
> +    auto success = !m_audioOutputUnitAdaptor.start();

Nit. OK as it is. Maybe "auto success = m_audioOutputUnitAdaptor.start() == noErr" is better?

> Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:118
> +    auto success = !m_audioOutputUnitAdaptor.stop();

Ditto.

[Chris Dumez]

(In reply to Peng Liu from comment #3)
> Comment on attachment 412454 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=412454&action=review
> 
> > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:132
> > +            completionHandler(Exception { InvalidStateError, "Failed to start the audio device"_s });
> 
> Nit. Can be done in one line as in DefaultAudioDestinationNode::resume().
> 
> > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:148
> > +        completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to start audio device"_s }));
> 
> Nit. Maybe we can use the same error message as in
> DefaultAudioDestinationNode::startRendering()?
> s/"Failed to start audio device"_s/"Failed to start the audio device"_s/
> 
> > Source/WebCore/Modules/webaudio/DefaultAudioDestinationNode.cpp:163
> > +        completionHandler(success ? WTF::nullopt : makeOptional(Exception { InvalidStateError, "Failed to stop audio device"_s }));
> 
> Ditto. s/"Failed to stop audio device"_s/"Failed to stop the audio device"_s/
> 
> > Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:106
> > +    auto success = !m_audioOutputUnitAdaptor.start();
> 
> Nit. OK as it is. Maybe "auto success = m_audioOutputUnitAdaptor.start() ==
> noErr" is better?
> 
> > Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.cpp:118
> > +    auto success = !m_audioOutputUnitAdaptor.stop();
> 
> Ditto.

Thanks for looking. I will fix all these shortly.

[Chris Dumez]

Created attachment 412460 [details]
Patch

[Geoffrey Garen]

Comment on attachment 412460 [details]
Patch

r=me

[EWS]

Committed r269073: <https://trac.webkit.org/changeset/269073>

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

[Radar WebKit Bug Importer]

<rdar://problem/70739259>