[webkit-reviews] review granted: [Bug 189684] Clear persistent storage between tests for resourceLoadStatistics : [Attachment 349986] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Woodrow Wang
<wwang153 at stanford.edu>'s request for review:
Bug 189684: Clear persistent storage between tests for resourceLoadStatistics
https://bugs.webkit.org/show_bug.cgi?id=189684

Attachment 349986: Patch

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




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

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

r=me with fixes.

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:775
> +	       m_memoryStore->clear([this, shouldGrandfather,
callCompletionHandlerOnMainThread = WTFMove(callCompletionHandlerOnMainThread)]
() mutable {

this, protectedThis = makeRef(*this)

I think you need to protect this here since there is nothing that guarantees
that |this| stays alive AFAICT.

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:777
> +		      
m_memoryStore->grandfatherExistingWebsiteData(WTFMove(callCompletionHandlerOnMa
inThread));

You likely need to null-check m_memoryStore here. It would have been cleared in
between.