Summary: | Allow DisplayRefreshMonitors to be more long-lived objects | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | cdumez, cmarcelo, ews-watchlist, gyuyoung.kim, japhet, luiz, ryuan.choi, sabouhallawa, sergio, simon.fraser, thorton, webkit-bug-importer, zeno | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2021-03-27 14:48:36 PDT
Created attachment 424470 [details]
Patch
Created attachment 424471 [details]
Patch
Created attachment 424472 [details]
Patch
Comment on attachment 424472 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424472&action=review R=me assuming bots are happy > Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:106 > + LockHolder lock(mutex()); I think we usually use locker for such variables, e.g: Auto locker = holdLock(lock()); > Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:111 > + auto started = startNotificationMechanism(); I don’t think we need this local. Could write: If(!startNotifocationMechanism()) return false; > Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:128 > + ++m_unscheduledFireCount; Could be merged into the next line. > Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:135 > LockHolder lock(m_mutex); Same comment about locker. > Source/WebCore/platform/graphics/DisplayRefreshMonitor.h:75 > + Lock& mutex() { return m_mutex; } Why don’t we call this lock instead of mutex since this is what we call it nowadays? Created attachment 424473 [details]
Patch
Created attachment 424477 [details]
Patch
Committed r275144: <https://commits.webkit.org/r275144> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424477 [details]. |