Bug 31690 – Make SocketStreamHandleCFNet work on Windows

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Alexey Proskuryakov]

We should not use main thread's CFRunLoop, because it doesn't have one.

[Alexey Proskuryakov]

Created attachment 43529 [details]
proposed patch

[Alexey Proskuryakov]

CC'ing Dave and Dmitry, because they dealt with callOnMainThreadAndWait before.

[Darin Adler]

Comment on attachment 43529 [details]
proposed patch

> +    bool needToSchedule = false;

Stray unused local variable.

> +// Blocks the thread until the call finishes on the main thread. This can easily cause deadlocks.
> +void callOnMainThreadAndWait(MainThreadFunction*, void* context);

I would reword the second sentence as "Misusing this can easily cause deadlocks."

> +    // Must add a source to the run loop to prevent CFRunLoopRun() from exiting

Need a period.

> +    CFRunLoopSourceContext ctxt = {0, (void*)1 /*must be non-NULL*/, 0, 0, 0, 0, 0, 0, 0, emptyPerform};

Can't we use something that's actually a pointer? We could use a local variable to name it. Like this, for example:

    void* arbitraryNonZeroPointer = loaderRL;

> +    CFRunLoopAddSource(loaderRL, bogusSource,kCFRunLoopDefaultMode);

Missing a space after the comma here.

> +        while (loaderRL == 0) {
> +            // FIXME: Sleep 10? that can't be right...
> +            Sleep(10);
> +        }

We would normally use !loaderRL, right?

How about calling the global variable loaderRunLoopObject instead of loaderRL?

I just realized I'm reviewing the moved code. Oops.

r=me

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/51222>. Addressed most of the above comments.