[webkit-reviews] review granted: [Bug 188265] Worker should support unhandled promise rejections : [Attachment 346992] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yusuke Suzuki
<yusukesuzuki at slowstart.org>'s request for review:
Bug 188265: Worker should support unhandled promise rejections
https://bugs.webkit.org/show_bug.cgi?id=188265

Attachment 346992: Patch

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




--- Comment #24 from Darin Adler <darin at apple.com> ---
Comment on attachment 346992
  --> https://bugs.webkit.org/attachment.cgi?id=346992
Patch

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

> Source/WebCore/dom/Microtasks.h:73
> +    WEBCORE_EXPORT static MicrotaskQueue&
queueForScriptExecutionContext(ScriptExecutionContext*);

This should take a reference, not a pointer. I believe it does not allow
passing a nullptr.

I’m not a huge fan of writing out the entire class name of an argument as part
of a function name: it’s common in Objective-C but doesn’t seem right in C++
where we have much stronger type checking. I could imagine just naming this
function "queue" or "contextQueue" or maybe "queueForContext" or
"queueForExecutionContext".

> Source/WebCore/workers/WorkerThread.cpp:304
> +	       workerGlobalScope.removeRejectedPromiseTracker();

I’m starting to think that this long list of things to stop in
WorkerGlobalScope might belong inside that class, not in this WorkerThread
function.