[webkit-reviews] review granted: [Bug 130743] WebCore portion to tell if audio is playing in a plugin process : [Attachment 227804] patch

[bugzilla-daemon at webkit.org]

Jer Noble <jer.noble at apple.com> has granted Stephanie Lewis
<slewis at apple.com>'s request for review:
Bug 130743: WebCore portion to tell if audio is playing in a plugin process
https://bugs.webkit.org/show_bug.cgi?id=130743

Attachment 227804: patch
https://bugs.webkit.org/attachment.cgi?id=227804&action=review

------- Additional Comments from Jer Noble <jer.noble at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=227804&action=review


> Source/WebCore/platform/audio/AudioHardwareListener.h:47
> +    static bool audioHardwareListenerIsSupported();

Is there a reason this needs to be a static method?  Here's an alternative
idea:

> Source/WebCore/platform/audio/AudioHardwareListener.h:48
> +    bool isHardwareActive() const { return m_active; }

Consider this instead:

enum HardwareActivityType {
    Unknown,
    IsActive,
    IsInactive,
};
HardwareActivityType hardwareActivity() const { return m_activity; }

> Source/WebCore/platform/audio/AudioHardwareListener.h:54
> +    bool m_active;

...and:

HardwareActivityType m_activity;

> Source/WebCore/platform/audio/AudioHardwareListener.cpp:46
> +    , m_active(false)

...and:

    , m_active(Unknown)

This way, we don't neeed audioHardwareListenerIsSupported(), but instead
`listener->hardwareActivity()` will return `Unknown` when hardware activity
listening is unavailable.