Bug 225481 – [iOS] Safari sometimes hangs underneath `WebKit::UIDelegate::UIClient::createNewPage`

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Wenson Hsieh]

rdar://77565282

[Wenson Hsieh]

Created attachment 427937 [details]
Patch

[Chris Dumez]

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?

[Chris Dumez]

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)`

[Chris Dumez]

(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().

[Wenson Hsieh]

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 :(

[Wenson Hsieh]

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...

[Wenson Hsieh]

Created attachment 427966 [details]
For EWS

[Wenson Hsieh]

Created attachment 427968 [details]
Fix non-internal builds

[Chris Dumez]

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

[Wenson Hsieh]

Created attachment 427971 [details]
Use %{public}s

[EWS]

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].