[webkit-reviews] review granted: [Bug 116145] Playing many sounds with HTML5 Audio makes WebKit unresponsive : [Attachment 215974] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 116145: Playing many sounds with HTML5 Audio makes WebKit unresponsive
https://bugs.webkit.org/show_bug.cgi?id=116145

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

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=215974&action=review


>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.h:134
> +    virtual double maximumDurationToCacheMediaTime() const { return 5; }

This will only change the behavior on the Mac. If you don't think it is OK to
make this change on Windows yet, please file a bug so we can test fix.

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.h:256
> +    unsigned m_pendingStatusChanges;
> +    int m_cachedItemStatus;
> +    bool m_cachedLikelyToKeepUp;
> +    bool m_cachedBufferEmpty;
> +    bool m_cachedBufferFull;
> +    RetainPtr<NSArray> m_cachedSeekableRanges;
> +    RetainPtr<NSArray> m_cachedLoadedRanges;
> +    RetainPtr<NSArray> m_cachedTracks;
> +    bool m_cachedHasEnabledAudio;
> +    FloatSize m_cachedPresentationSize;
> +    double m_cachedDuration;
> +    double m_cachedRate;

Nit: sorting these by size will make the object slightly smaller.

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:249
> +    , m_cachedItemStatus(MediaPlayerAVPlayerItemStatusDoesNotExist)
> +    , m_cachedHasEnabledAudio(false)
> +    , m_cachedDuration(MediaPlayer::invalidTime())
> +    , m_cachedRate(0)

Shouldn't you initialize m_pendingStatusChanges, m_cachedLikelyToKeepUp,
m_cachedBufferEmpty, and m_cachedBufferFull?