Bug 226001 – Use CheckedLock more in WebKit2 code

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

Use CheckedLock more in WebKit2 code to benefit for Clang Thread Safety Analysis.

[Chris Dumez]

Created attachment 429127 [details]
Patch

[Alex Christensen]

Comment on attachment 429127 [details]
Patch

I like WTF_GUARDED_BY_LOCK

[Chris Dumez]

Created attachment 429140 [details]
Patch

[Kimmo Kinnunen]

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);

[Chris Dumez]

Created attachment 429171 [details]
Patch

[Chris Dumez]

(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.

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/78261485>