Bug 209992 – [ macOS ] TestWebKitAPI.WKHTTPCookieStore.WithoutProcessPoolDuplicates is failing

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Sihui Liu]

...

[Sihui Liu]

Created attachment 395417 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 395417 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:641
> +            [cookieStore deleteCookie:cookie completionHandler:nil];

What makes it so that we don’t need to wait for completion?

[Sihui Liu]

Created attachment 395716 [details]
Patch

[Sihui Liu]

(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 395417 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=395417&action=review
> 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:641
> > +            [cookieStore deleteCookie:cookie completionHandler:nil];
> 
> What makes it so that we don’t need to wait for completion?

We need to wait for the completion. Updated the patch.

[Alexey Proskuryakov]

Comment on attachment 395716 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:649
> +                if (++deletedCount == cookies.count)

Are all of these completion handlers called on the same thread? I thought that they just go on a dispatch queue, and can be executed simultaneously, in which case this should use an atomic operation instead of a plain "++".

Also, please take cookies.count function call out of the loop. This array should be thread safe, but there is no need to recalculate its size.

[Sihui Liu]

Comment on attachment 395716 [details]
Patch

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

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:649
>> +                if (++deletedCount == cookies.count)
> 
> Are all of these completion handlers called on the same thread? I thought that they just go on a dispatch queue, and can be executed simultaneously, in which case this should use an atomic operation instead of a plain "++".
> 
> Also, please take cookies.count function call out of the loop. This array should be thread safe, but there is no need to recalculate its size.

All completion handlers will be called on the main runloop. Will move cookies.count out of the loop.

[Sihui Liu]

Created attachment 395749 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/61458442>

[Geoffrey Garen]

Comment on attachment 395749 [details]
Patch

r=me

[EWS]

Committed r259745: <https://trac.webkit.org/changeset/259745>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395749 [details].