Summary: | [ThreadedCompositor] Simplify the compositing run loop worker thread | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, don.olmstead, Hironori.Fujii, webkit-bug-importer, zan | ||||
Priority: | P2 | Keywords: | Gtk, InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=186444 | ||||||
Attachments: |
|
Description
Carlos Garcia Campos
2019-03-01 06:54:35 PST
Created attachment 363328 [details]
Patch
Comment on attachment 363328 [details] Patch Clearing flags on attachment: 363328 Committed r242266: <https://trac.webkit.org/changeset/242266> All reviewed patches have been landed. Closing bug. *** Bug 186444 has been marked as a duplicate of this bug. *** Comment on attachment 363328 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363328&action=review > Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:76 > + }); Why do you dispatch twice? Can this code be simplified like the following? m_runLoop->dispatch([] { RunLoop::current().stop(); }); Comment on attachment 363328 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363328&action=review >> Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:76 >> + }); > > Why do you dispatch twice? > Can this code be simplified like the following? > > m_runLoop->dispatch([] { > RunLoop::current().stop(); > }); No, we used to get a crash if the run loop was stopped before the timer was destroyed. So, we stop the run loop in the next main frame iteration. See bug #167545. Maybe it's no longer needed now that we don't use a WorkQueue. It seems the original issue of Bug 167545 was a race condition of: 1. Main threaed is deleting the WorkQueue 2. The WorkQueue's thread is executing a task I think my change (comment 6) works. I'm going to file a ticket for the change. This code is causing a issue for WinCairo (Bug 186364 Comment 11). |