[webkit-reviews] review granted: [Bug 225826] [GPUP] WebContent process should not pull audio session category from the GPU Process : [Attachment 428706] Fix build failures on iOS

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 225826: [GPUP] WebContent process should not pull audio session category
from the GPU Process
https://bugs.webkit.org/show_bug.cgi?id=225826

Attachment 428706: Fix build failures on iOS

https://bugs.webkit.org/attachment.cgi?id=428706&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 428706
  --> https://bugs.webkit.org/attachment.cgi?id=428706
Fix build failures on iOS

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

I think on balance changing this from an enum to an enum class is making things
worse.

> Source/WebCore/page/DeprecatedGlobalSettings.cpp:157
> +    return (unsigned)AudioSession::sharedSession().categoryOverride();

It’s peculiar to use a C-style cast here. I suggest static_cast.

> Source/WebCore/platform/audio/AudioSession.cpp:116
> +    return return AudioSession::CategoryType::None;

This says "return return", so I don’t think it will compile.

> Source/WebCore/platform/audio/AudioSession.h:63
> -    enum CategoryType : uint8_t {
> +    enum class CategoryType : uint8_t {

Not sure this is an improvement. Does make it more wordy everywhere. Does it
make the code more clear? I’m OK with it, but not sure.

> Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:134
> +    AudioSession::CategoryType category = AudioSession::CategoryType::None;

Could use auto here to make things a little less verbose. Why write
AudioSession::CategoryType twice on the same line of code unnecessarily?