Bug 235730 – Begin using coroutines in testing code

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Begin using coroutines in testing code

[Alex Christensen]

Created attachment 450171 [details]
Patch

[Darin Adler]

Comment on attachment 450171 [details]
Patch

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

> Tools/TestWebKitAPI/cocoa/HTTPServer.h:65
> +    enum class UseCoroutines : bool { Yes };
> +    HTTPServer(UseCoroutines, Function<CoroutineHandle(Connection)>&&, Protocol = Protocol::Http);

I don’t think we need this UseCoroutines::Yes argument. The Function argument already takes CoroutineHandle, so this should be found by normal overloading.

> Tools/TestWebKitAPI/cocoa/HTTPServer.mm:53
> +        for (auto& handle : coroutineHandles)
> +            handle.destroy();

Maybe we should wrap the coroutine handle in a type of our own that does a destroy in the handle’s destructor. Then we would not need this. Like CoroutineHandle could do this.

> Tools/TestWebKitAPI/cocoa/HTTPServer.mm:452
> +    return Awaitable(*this);

This return statement slices the Awaitable, copying out only the suspend_never base class part of the object. I don’t think that’s what you wanted to do here.

[Darin Adler]

Comment on attachment 450171 [details]
Patch

review- because of the slice problem; not sure how the test works!

[Radar WebKit Bug Importer]

<rdar://problem/88452783>

[Alex Christensen]

Comment on attachment 450171 [details]
Patch

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

>> Tools/TestWebKitAPI/cocoa/HTTPServer.h:65
>> +    HTTPServer(UseCoroutines, Function<CoroutineHandle(Connection)>&&, Protocol = Protocol::Http);
> 
> I don’t think we need this UseCoroutines::Yes argument. The Function argument already takes CoroutineHandle, so this should be found by normal overloading.

The Function returns a CoroutineHandle but takes the same parameter, a Connection.  We currently do need UseCoroutines because otherwise it thinks the constructor is ambiguous.

[Alex Christensen]

Created attachment 459553 [details]
Patch

[Alex Christensen]

Created attachment 459554 [details]
Patch

[EWS]

Committed r294492 (250751@main): <https://commits.webkit.org/250751@main>

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