Bug 187143 – Make sure the WebResourceLoadStatisticsStore gets destroyed on the main thread

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

[Chris Dumez]

Make sure the WebResourceLoadStatisticsStore gets destroyed on the main thread. Currently, we keep capturing a Ref<> to the WebResourceLoadStatisticsStore whenever we dispatch a task to the work queue. This means that the WebResourceLoadStatisticsStore will get destroyed on a background thread if the work queue is the last one to hold a ref to the store, which would not be safe.

[Chris Dumez]

Created attachment 343857 [details]
Patch

[youenn fablet]

Comment on attachment 343857 [details]
Patch

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

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:188
> +    m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), origins = WTFMove(origins)]() mutable {

origins should be isolated except if we are sure they are coming straight from IPC.
We should add a comment saying why this is ok.
And we should probably make WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated private to ensure it does not get used out of IPC.

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:258
>  void WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener(String&& primaryDomainInNeedOfStorageAccess, uint64_t openerPageID, String&& openerPrimaryDomain, bool isTriggeredByUserGesture)

Ditto.

[Chris Dumez]

Created attachment 343868 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 343868 [details]
Patch

Clearing flags on attachment: 343868

Committed r233342: <https://trac.webkit.org/changeset/233342>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41609309>