HTMLMediaElement: adopt Adopt reportExtraMemoryVisited/Allocated
<rdar://problem/85533233>
Created attachment 444637 [details] Patch
Created attachment 444638 [details] Patch
Comment on attachment 444638 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444638&action=review > Source/WebCore/platform/graphics/MediaPlayer.cpp:1001 > + if (m_private && !m_private->supportsMemoryCostChanged()) When m_private does not support MemoryCostChanged?
Comment on attachment 444638 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444638&action=review > Source/WebCore/html/HTMLMediaElement.cpp:866 > + if (newMemoryCost == m_reportedExtraMemoryCost) on which thread could this be running that makes a requirement that m_reportedExtraMemoruCost be atomic? Could a note be added on the member declaration explaining on why this requirement? It would be ideal if this method had an assert like Assert(isOnMainthread() || isOnTheThreadWeExpect()) unfortunately, there's very little infrastructure for that
(In reply to Peng Liu from comment #4) > Comment on attachment 444638 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444638&action=review > > > Source/WebCore/platform/graphics/MediaPlayer.cpp:1001 > > + if (m_private && !m_private->supportsMemoryCostChanged()) > > When m_private does not support MemoryCostChanged? Every MediaPlayerPrivate that isn't MediaPlayerPrivateAVFoundation just uses the default implementation of memoryCost(), and never explicitly pushes memory cost information upwards.
(In reply to Jean-Yves Avenard [:jya] from comment #5) > Comment on attachment 444638 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444638&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:866 > > + if (newMemoryCost == m_reportedExtraMemoryCost) > > on which thread could this be running that makes a requirement that > m_reportedExtraMemoruCost be atomic? memoryCost() can be called from a GC thread, through the JSHTMLMediaElement wrapper object, by JSC. > Could a note be added on the member declaration explaining on why this > requirement? I'll add a note in ::memoryCost().
Created attachment 444693 [details] Patch
Created attachment 444722 [details] Patch