Summary: | HTMLMediaElement: Adopt reportExtraMemoryVisited/Allocated | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||||||
Component: | Media | Assignee: | Jer Noble <jer.noble> | ||||||||||
Status: | NEW --- | ||||||||||||
Severity: | Normal | CC: | calvaris, cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, ggaren, glenn, gyuyoung.kim, jean-yves.avenard, kondapallykalyan, peng.liu6, philipj, sergio, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Jer Noble
2021-11-15 13:57:06 PST
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
|