[webkit-reviews] review granted: [Bug 183925] CacheStorage::Engine should not ref itself when hopping to a background thread : [Attachment 336370] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 23 08:47:58 PDT 2018


Chris Dumez <cdumez at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 183925: CacheStorage::Engine should not ref itself when hopping to a
background thread
https://bugs.webkit.org/show_bug.cgi?id=183925

Attachment 336370: Patch

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




--- Comment #4 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 336370
  --> https://bugs.webkit.org/attachment.cgi?id=336370
Patch

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

r=me with comments

> Source/WebKit/ChangeLog:10
> +	   Use weak pointer when hopping ot background threads.

typo: ot

> Source/WebKit/ChangeLog:14
> +	   Made sure to use just oen Engine for all private sessions.

typo: oen

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:330
> +	       RunLoop::main().dispatch([this, weakThis = makeWeakPtr(this),
identifier]() mutable {

I do not think we should be calling makeWeakPtr() from a background thread. I
doubt it is thread safe (In particular, createWeakPtr() that is being
implicitly called). I'd suggest WTFMove() ing weakThis here.


More information about the webkit-reviews mailing list