Bug 195208 – [ThreadedCompositor] Simplify the compositing run loop worker thread

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

We can remove the WorkQueuePool, since we never really supported more than one thread, and now that single process model non longer exists it doesn't even make sense. We can simply use a RunLoop instead of a WorkQueue so that the implementation is not specific to the generic WorkQueue implementation.

[Carlos Garcia Campos]

Created attachment 363328 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 363328 [details]
Patch

Clearing flags on attachment: 363328

Committed r242266: <https://trac.webkit.org/changeset/242266>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48513504>

[Don Olmstead]

*** Bug 186444 has been marked as a duplicate of this bug. ***

[Fujii Hironori]

Comment on attachment 363328 [details]
Patch

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

> Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:76
> +    });

Why do you dispatch twice?
Can this code be simplified like the following?

    m_runLoop->dispatch([] {
        RunLoop::current().stop();
    });

[Carlos Garcia Campos]

Comment on attachment 363328 [details]
Patch

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

>> Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:76
>> +    });
> 
> Why do you dispatch twice?
> Can this code be simplified like the following?
> 
>     m_runLoop->dispatch([] {
>         RunLoop::current().stop();
>     });

No, we used to get a crash if the run loop was stopped before the timer was destroyed. So, we stop the run loop in the next main frame iteration. See bug #167545. Maybe it's no longer needed now that we don't use a WorkQueue.

[Fujii Hironori]

It seems the original issue of Bug 167545 was a race condition of:
1. Main threaed is deleting the WorkQueue
2. The WorkQueue's thread is executing a task

I think my change (comment 6) works.
I'm going to file a ticket for the change.

This code is causing a issue for WinCairo (Bug 186364 Comment 11).