Summary: | Use CheckedLock more in WebKit2 code | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, benjamin, cmarcelo, darin, ddkilzer, ews-watchlist, ggaren, kkinnunen, sam, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=226021 | ||||||||||
Attachments: |
|
Description
Chris Dumez
2021-05-19 19:29:07 PDT
Created attachment 429127 [details]
Patch
Comment on attachment 429127 [details]
Patch
I like WTF_GUARDED_BY_LOCK
Created attachment 429140 [details]
Patch
Comment on attachment 429140 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429140&action=review great work. maybe the lockable() could be reverted / not added. > Source/WTF/ChangeLog:8 > + Add lockable() getter to Locker<CheckedLock> to match the regular Locker<T>. I didn't see the use case to add that. That enables the anti-pattern that is being invoked also in this patch. > Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:177 > +void LibWebRTCCodecs::ensureGPUProcessConnectionOnMainThread(Locker<CheckedLock>& locker) For CheckedLock, this sort of unused parameter pattern is now an anti-pattern. The annotation in the header file would be : void LibWebRTCCodecs::ensureGPUProcessConnectionOnMainThread() WTF_REQUIRES_LOCK(m_connectionLock); Created attachment 429171 [details]
Patch
(In reply to Kimmo Kinnunen from comment #4) > Comment on attachment 429140 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=429140&action=review > > great work. > maybe the lockable() could be reverted / not added. > > > Source/WTF/ChangeLog:8 > > + Add lockable() getter to Locker<CheckedLock> to match the regular Locker<T>. > > I didn't see the use case to add that. > That enables the anti-pattern that is being invoked also in this patch. Yes, definitely not needed if we get rid of the "Passing Locker as parameter" pattern. I dropped this. > > Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:177 > > +void LibWebRTCCodecs::ensureGPUProcessConnectionOnMainThread(Locker<CheckedLock>& locker) > > For CheckedLock, this sort of unused parameter pattern is now an > anti-pattern. > > The annotation in the header file would be : > void LibWebRTCCodecs::ensureGPUProcessConnectionOnMainThread() > WTF_REQUIRES_LOCK(m_connectionLock); Fixed. Committed r277787 (237948@main): <https://commits.webkit.org/237948@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429171 [details]. |