rdar://84399283
Created attachment 442099 [details] For EWS
Created attachment 442100 [details] Minor renaming
*** Bug 232139 has been marked as a duplicate of this bug. ***
Comment on attachment 442100 [details] Minor renaming View in context: https://bugs.webkit.org/attachment.cgi?id=442100&action=review > Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:152 > Ref<WebCore::ImageBuffer> m_imageBuffer; This backpointer should be weak, since RemoteImageBuffer (strongly) owns its RemoteDisplayListRecorder. Going to make this adjustment along with the rest of this patch.
Created attachment 442184 [details] Rebase on trunk
Created attachment 442189 [details] Add missing 'Reviewed by' line
Created attachment 442191 [details] Add missing 'Reviewed by' line
Created attachment 442206 [details] For EWS
Comment on attachment 442206 [details] For EWS View in context: https://bugs.webkit.org/attachment.cgi?id=442206&action=review > Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h:173 > + inline static uint64_t objectCountForTesting() { return gObjectCountForTesting; } inline seems redundant? > Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:424 > + m_renderingBackend->performWithMediaPlayerOnMainThread(identifier, [RefPtr imageBuffer = m_imageBuffer.get(), destination](MediaPlayer& player) { I think we usually use `imageBuffer = RefPtr { m_imageBuffer.get() }` ? > Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h:137 > + Lock m_remoteDisplayListsLock; That seems a bit fishy given that m_remoteDisplayLists is a WeakHashSet. How could it be used safely from multiple threads given that WeakPtr cannot?
(In reply to Chris Dumez from comment #9) > Comment on attachment 442206 [details] > For EWS > > View in context: > https://bugs.webkit.org/attachment.cgi?id=442206&action=review > > > Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h:173 > > + inline static uint64_t objectCountForTesting() { return gObjectCountForTesting; } > > inline seems redundant? Good point — removed the `inline`. > > > Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:424 > > + m_renderingBackend->performWithMediaPlayerOnMainThread(identifier, [RefPtr imageBuffer = m_imageBuffer.get(), destination](MediaPlayer& player) { > > I think we usually use `imageBuffer = RefPtr { m_imageBuffer.get() }` ? 👍🏻 > > > Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h:137 > > + Lock m_remoteDisplayListsLock; > > That seems a bit fishy given that m_remoteDisplayLists is a WeakHashSet. How > could it be used safely from multiple threads given that WeakPtr cannot? That's true — though, I'm curious why this isn't already causing all sorts of "!= constructed on main thread" assertions already, given that this is already an issue. I'm going to clean this up in https://bugs.webkit.org/show_bug.cgi?id=232183, and leave it as it is currently. I plan to turn this into a regular HashSet of either RefPtr or Ref, and manually remove entries when image buffers are released (based on rendering resource ID)
Created attachment 442207 [details] For EWS
Created attachment 442321 [details] Rebase on trunk
Created attachment 442330 [details] Try to fix layout test?
Comment on attachment 442330 [details] Try to fix layout test? r=me
Committed r284786 (243495@main): <https://commits.webkit.org/243495@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 442330 [details].