[webkit-reviews] review canceled: [Bug 185757] Don't create the SubimageCache just to clear an image from it : [Attachment 340676] Patch v1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 21 10:44:52 PDT 2018


David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has canceled David Kilzer
(:ddkilzer) <ddkilzer at webkit.org>'s request for review:
Bug 185757: Don't create the SubimageCache just to clear an image from it
https://bugs.webkit.org/show_bug.cgi?id=185757

Attachment 340676: Patch v1

https://bugs.webkit.org/attachment.cgi?id=340676&action=review




--- Comment #6 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 340676
  --> https://bugs.webkit.org/attachment.cgi?id=340676
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=340676&action=review

>> Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp:38
>> +std::once_flag SubimageCacheWithTimer::s_onceFlag;
> 
> Why are these here and in the header?

They are 'static' class variables, so they're declared within the class in the
header, but space is allocated (just once) in the *.cpp file.

>> Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp:119
>> +	std::call_once(SubimageCacheWithTimer::s_onceFlag, [] {
> 
> Why? This is WebCore code that’s main-thread-only, we can just null-check.

Wil fix.  Wasn't sure if breaking this out from the WebCore::Image constructor
could cause any issues with it being called from multiple threads.  (I guess if
that was the case, I'd need locking in ::cacheExists() as well.)


More information about the webkit-reviews mailing list