Summary: | Make SocketStreamHandleCFNet work on Windows | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||
Component: | Platform | Assignee: | Alexey Proskuryakov <ap> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dimich, levin | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | Windows XP | ||||||
Attachments: |
|
Description
Alexey Proskuryakov
2009-11-19 14:52:32 PST
Created attachment 43529 [details]
proposed patch
CC'ing Dave and Dmitry, because they dealt with callOnMainThreadAndWait before. 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 Committed <http://trac.webkit.org/changeset/51222>. Addressed most of the above comments. |