Bug 133708 - Make it possible for waitForAndDispatchImmediately to bail if a sync message comes in from the other direction
Summary: Make it possible for waitForAndDispatchImmediately to bail if a sync message ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-10 16:57 PDT by Tim Horton
Modified: 2014-06-11 13:38 PDT (History)
4 users (show)

See Also:


Attachments
patch (10.71 KB, patch)
2014-06-10 17:38 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
patch (10.67 KB, patch)
2014-06-10 17:40 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
anders changes (11.18 KB, patch)
2014-06-11 12:57 PDT, Tim Horton
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2014-06-10 16:57:56 PDT
We should have a flag on waitForAndDispatchImmediately that makes it bail immediately if a sync message comes in from the other direction. For example:

1. UI process is waitForAndDispatchImmediatelying, waiting for message X from the Web process
2. Web process timer fires, sends sync message Y to the UI process. Y won't return until the UI process main thread gets a chance to run, but it's currently waiting.
<a long time goes by with both processes doing nothing useful>
3. *Eventually* we hit the waitForAndDispatchImmediately timeout, and everything proceeds normally from there.

Instead:

1. UI process is waitForAndDispatchImmediatelying, waiting for message X from the Web process
2. Web process timer fires, sends sync message Y to the UI process.
3. The waitForAndDispatchImmediately on message X bails immediately.
4. Message Y is handled.
<a long time didn't have to go by>
Comment 1 Tim Horton 2014-06-10 17:38:59 PDT
Created attachment 232833 [details]
patch
Comment 2 Tim Horton 2014-06-10 17:40:48 PDT
Created attachment 232834 [details]
patch
Comment 3 Anders Carlsson 2014-06-11 12:19:22 PDT
Comment on attachment 232834 [details]
patch

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

> Source/WebKit2/Platform/IPC/Connection.cpp:392
> +    WaitForMessageInfo waitingForMessage(messageReceiverName, messageName, destinationID, flags);

There's still no run loop assertion here!

> Source/WebKit2/Platform/IPC/Connection.h:172
> +    template<typename T> bool waitForAndDispatchImmediately(uint64_t destinationID, std::chrono::milliseconds timeout, WaitForMessageFlags = (WaitForMessageFlags)0);

Please use an unsigned instead of the cast.

> Source/WebKit2/Platform/IPC/Connection.h:280
> +    struct WaitForMessageInfo {
> +        WaitForMessageInfo(StringReference messageReceiverName, StringReference messageName, uint64_t destinationID, WaitForMessageFlags flags)
> +            : messageReceiverName(messageReceiverName)
> +            , messageName(messageName)
> +            , destinationID(destinationID)
> +            , waitForMessageFlags(flags)
> +        { }
> +
> +        StringReference messageReceiverName;
> +        StringReference messageName;
> +        uint64_t destinationID;
> +
> +        WaitForMessageFlags waitForMessageFlags;
> +        bool messageWaitingInterrupted = false;
> +
> +        std::unique_ptr<MessageDecoder> decoder;
> +    };
> +
> +    WaitForMessageInfo* m_waitingForMessage;

I think you can forward declare this and just stick it in the .cpp file. Maybe call it WaitForMessageState?
Comment 4 Tim Horton 2014-06-11 12:57:43 PDT
Created attachment 232891 [details]
anders changes
Comment 5 Anders Carlsson 2014-06-11 13:35:45 PDT
Comment on attachment 232891 [details]
anders changes

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

> Source/WebKit2/Platform/IPC/Connection.cpp:45
> +    { }

Newline after {

> Source/WebKit2/Platform/IPC/Connection.cpp:428
> +            std::unique_ptr<MessageDecoder> decoder = std::move(m_waitingForMessage->decoder);

auto decoder =
Comment 6 Tim Horton 2014-06-11 13:38:44 PDT
http://trac.webkit.org/changeset/169825