AuxiliaryProcessProxy::sendWithAsyncReply should queue up messages if sent while the process is starting like it does messages without replies
Created attachment 378696 [details] Patch
Comment on attachment 378696 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378696&action=review > Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:152 > + if (auto&& info = WTFMove(asyncReplyInfo)) Not sure why this isn't simply: if (asyncReplyInfo) > Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:222 > + if (auto&& info = WTFMove(pendingMessage.asyncReplyInfo)) ditto. > Source/WebKit/UIProcess/AuxiliaryProcessProxy.h:175 > + sendMessage(WTFMove(encoder), sendOptions, {{ [completionHandler = WTFMove(completionHandler)] (IPC::Decoder* decoder) mutable { If sendMessage() returns false (because the process has been terminated, it looks to me that the completion handler will never be called. We should probably T::cancelReply.
Comment on attachment 378696 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378696&action=review > Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:153 > + IPC::addAsyncReplyHandler(*connection(), info->second, WTFMove(info->first)); and then WTFMove(asyncReplyInfo->first) here.
Created attachment 378716 [details] Patch
Comment on attachment 378716 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378716&action=review > Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:213 > + OptionSet<IPC::SendOption> sendOptions = pendingMessage.sendOptions; both can be auto. > Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:230 > m_connection->sendMessage(WTFMove(message), sendOptions); I guess sendMessage can only return true here. I would assert it though. In theory, we could add the async reply handler that would only complete once the connection is destroyed.
Comment on attachment 378716 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378716&action=review > Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:211 > + for (auto&& pendingMessage : std::exchange(m_pendingMessages, { })) { Why auto&& and not auto? You're doing a std::exchange().
(In reply to Chris Dumez from comment #6) > Comment on attachment 378716 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=378716&action=review > > > Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:211 > > + for (auto&& pendingMessage : std::exchange(m_pendingMessages, { })) { > > Why auto&& and not auto? You're doing a std::exchange(). Otherwise I get this: error: call to implicitly-deleted copy constructor of 'WebKit::AuxiliaryProcessProxy::PendingMessage'
http://trac.webkit.org/r249835
<rdar://problem/55340747>
Comment on attachment 378716 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378716&action=review >>> Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:211 >>> + for (auto&& pendingMessage : std::exchange(m_pendingMessages, { })) { >> >> Why auto&& and not auto? You're doing a std::exchange(). > > Otherwise I get this: > error: call to implicitly-deleted copy constructor of 'WebKit::AuxiliaryProcessProxy::PendingMessage' Ah, should probably be auto& then. I missed that it was a for() and not an if().