Report the memory cost of HTMLMediaElements to GC.
Created attachment 204642 [details] Patch
Comment on attachment 204642 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204642&action=review > Source/WebCore/html/HTMLMediaElement.cpp:622 > + double extraMemoryCost = m_player->extraMemoryCost(); > + double extraMemoryCostDelta = extraMemoryCost - m_reportedExtraMemoryCost; > + m_reportedExtraMemoryCost = extraMemoryCost; Why the mix of double and size_t here? It seems like extraMemoryCost is returning size_t, so this should just be size_t. > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:883 > + return totalBytes() * buffered()->totalDuration() / duration; Is there something that prevents this from overflowing a size_t? > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:269 > + virtual size_t extraMemoryCost() const OVERRIDE; Either this class or this function should be marked FINAL too.
Comment on attachment 204642 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204642&action=review > Source/WebCore/platform/graphics/MediaPlayerPrivate.h:209 > + virtual size_t extraMemoryCost() const { return 0; } Most (all?) ports will want the simple implementation you added to MediaPlayerPrivateAVFoundation.cpp - why not put it here?
(In reply to comment #2) > (From update of attachment 204642 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204642&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:622 > > + double extraMemoryCost = m_player->extraMemoryCost(); > > + double extraMemoryCostDelta = extraMemoryCost - m_reportedExtraMemoryCost; > > + m_reportedExtraMemoryCost = extraMemoryCost; > > Why the mix of double and size_t here? It seems like extraMemoryCost is returning size_t, so this should just be size_t. No good reason; these should indeed be size_t. > > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:883 > > + return totalBytes() * buffered()->totalDuration() / duration; > > Is there something that prevents this from overflowing a size_t? Since totalBytes() is an unsigned, and (totalDuration() / duration) will be in the range [0..1], this can never overflow a size_t. > > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:269 > > + virtual size_t extraMemoryCost() const OVERRIDE; > > Either this class or this function should be marked FINAL too. Sure thing. Thanks!
(In reply to comment #3) > (From update of attachment 204642 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204642&action=review > > > Source/WebCore/platform/graphics/MediaPlayerPrivate.h:209 > > + virtual size_t extraMemoryCost() const { return 0; } > > Most (all?) ports will want the simple implementation you added to MediaPlayerPrivateAVFoundation.cpp - why not put it here? I guess that makes sense. If any port wants their own implementation, they can just override it.
(In reply to comment #5) > (In reply to comment #3) > > (From update of attachment 204642 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=204642&action=review > > > > > Source/WebCore/platform/graphics/MediaPlayerPrivate.h:209 > > > + virtual size_t extraMemoryCost() const { return 0; } > > > > Most (all?) ports will want the simple implementation you added to MediaPlayerPrivateAVFoundation.cpp - why not put it here? > > I guess that makes sense. If any port wants their own implementation, they can just override it. Ah, it can't go into MediaPlayerPrivate because MediaPlayerPrivate does not have a "totalBytes()" method.
Committed r151609: <http://trac.webkit.org/changeset/151609>