Bug 183925 – CacheStorage::Engine should not ref itself when hopping to a background thread

• Component: Service Workers
• Status: RESOLVED FIXED

[youenn fablet]

Instead we can use a weakThis

[youenn fablet]

<rdar://problem/38580483>

[youenn fablet]

Created attachment 336340 [details]
Patch

[youenn fablet]

Created attachment 336370 [details]
Patch

[Chris Dumez]

Comment on attachment 336370 [details]
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.

[youenn fablet]

Created attachment 336385 [details]
Patch

[youenn fablet]

(In reply to youenn fablet from comment #5)
> Created attachment 336385 [details]
> Patch

Thanks for the review.
Fixed the issues in this patch.

[WebKit Commit Bot]

Comment on attachment 336385 [details]
Patch

Clearing flags on attachment: 336385

Committed r229904: <https://trac.webkit.org/changeset/229904>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.