[webkit-reviews] review granted: [Bug 182269] Use CompletionHandlers for DelayedReplies : [Attachment 337233] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 182269: Use CompletionHandlers for DelayedReplies
https://bugs.webkit.org/show_bug.cgi?id=182269

Attachment 337233: Patch

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




--- Comment #5 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 337233
  --> https://bugs.webkit.org/attachment.cgi?id=337233
Patch

Great patch!

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

> Source/WebKit/ChangeLog:9
> +	   called once.  This is what CompletionHandlers are for.

called once and always once.
Debug WK2 bots might tell us whether we always call these in any layout/api
test.

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:90
> +NetworkResourceLoader::NetworkResourceLoader(const
NetworkResourceLoadParameters& parameters, NetworkConnectionToWebProcess&
connection,
Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::DelayedReply&&
synchronousReply)

Since we are modifying NetworkResourceLoader constructor, maybe we could make
it take a NetworkResourceLoadParameters&&?

> Source/WebKit/Platform/IPC/Connection.h:57
> +template<typename> struct CodingType;

Why do we need this one?

> Source/WebKit/PluginProcess/PluginControllerProxy.cpp:102
> +    return WTFMove(m_initializationReply);

Maybe we should do ASSERT(m_initializationReply)?