rdar://77565282
Created attachment 427937 [details] Patch
Comment on attachment 427937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427937&action=review > Source/WebKit/Platform/IPC/Connection.cpp:573 > The patch doesn't seem to apply to tip of tree. Could you please make sure we have an ASSERT(isMainRunLoop()) in Connection::waitForMessage() and Connection::dispatchSyncMessage() since we're using m_inDispatchSyncMessageCount in both? > Source/WebKit/Platform/IPC/Connection.cpp:940 > + m_inDispatchSyncMessageCount++; Would use prefixed version: ++m_inDispatchSyncMessageCount; > Source/WebKit/Platform/IPC/Connection.cpp:942 > + m_inDispatchSyncMessageCount--; Would use prefixed version and add an assertion for extra safety: ASSERT(m_inDispatchSyncMessageCount); --m_inDispatchSyncMessageCount; > Source/WebKit/Platform/IPC/Timeout.h:41 > + constexpr bool isInfinity() const { return !m_deadline; } Isn't m_deadline MonotonicTime::infinity() when we're infinity?
Comment on attachment 427937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427937&action=review >> Source/WebKit/Platform/IPC/Timeout.h:41 >> + constexpr bool isInfinity() const { return !m_deadline; } > > Isn't m_deadline MonotonicTime::infinity() when we're infinity? I *think* this should be `std::isnan(m_deadline)`
(In reply to Chris Dumez from comment #3) > Comment on attachment 427937 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=427937&action=review > > >> Source/WebKit/Platform/IPC/Timeout.h:41 > >> + constexpr bool isInfinity() const { return !m_deadline; } > > > > Isn't m_deadline MonotonicTime::infinity() when we're infinity? > > I *think* this should be `std::isnan(m_deadline)` I meant std::isinf().
Comment on attachment 427937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427937&action=review >> Source/WebKit/Platform/IPC/Connection.cpp:573 >> > > The patch doesn't seem to apply to tip of tree. Could you please make sure we have an ASSERT(isMainRunLoop()) in Connection::waitForMessage() and Connection::dispatchSyncMessage() since we're using m_inDispatchSyncMessageCount in both? That's odd — it seemed to apply when I just pulled. Will add the assertions! >> Source/WebKit/Platform/IPC/Connection.cpp:940 >> + m_inDispatchSyncMessageCount++; > > Would use prefixed version: ++m_inDispatchSyncMessageCount; 👍🏻 >> Source/WebKit/Platform/IPC/Connection.cpp:942 >> + m_inDispatchSyncMessageCount--; > > Would use prefixed version and add an assertion for extra safety: > ASSERT(m_inDispatchSyncMessageCount); > --m_inDispatchSyncMessageCount; Good call! >>>> Source/WebKit/Platform/IPC/Timeout.h:41 >>>> + constexpr bool isInfinity() const { return !m_deadline; } >>> >>> Isn't m_deadline MonotonicTime::infinity() when we're infinity? >> >> I *think* this should be `std::isnan(m_deadline)` > > I meant std::isinf(). Good catch — changing this to `std::isinf(m_deadline)`. I didn't see the private constructor below, and thought that it initialized m_deadline to the default MonotonicTime :(
For some reason, it looks like Timeout.h isn't in the Xcode project either. I'll go ahead and add it there while I'm making changes...
Created attachment 427966 [details] For EWS
Created attachment 427968 [details] Fix non-internal builds
Comment on attachment 427968 [details] Fix non-internal builds View in context: https://bugs.webkit.org/attachment.cgi?id=427968&action=review > Source/WebKit/Platform/IPC/Connection.cpp:575 > + RELEASE_LOG_ERROR(IPC, "Connection::waitForMessage(%s): Exiting immediately, since we're handling a sync message already", description(messageName)); %{public}s
Created attachment 427971 [details] Use %{public}s
Committed r277169 (237455@main): <https://commits.webkit.org/237455@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427971 [details].