Bug 79478 – LayerTreeHostProxy uses Functional instead of MessageQueue for accumulating tasks.

• Component: WebKit2
• Status: RESOLVED FIXED

[Dongseong Hwang]

Add TaskQueue and apply it into LayerTreeHostProxy.

Both TaskQueue and MessageQueue are useful.
TaskQueue can reduces codes about boilerplate message definition.
MessageQueue can provide more functionality like remove.
LayerTreeHostProxy dose not require special funcionality of MessageQueue, so
TaskQueue can make LayerTreeHostProxy code more succinct.

[Dongseong Hwang]

Created attachment 128713 [details]
patch

[Dongseong Hwang]

I will apply TaskQueue to RunLoop and etc.

[WebKit Commit Bot]

Attachment 128713 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/wtf/TaskQueue.h:38:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dongseong Hwang]

Created attachment 128715 [details]
patch v.2

[Dongseong Hwang]

Chrome defines Function<void()> to Closure, and defines Deque<Closure> to TaskQueue in message_loop.h in chromium project.
I mimic that.

I will bring some functionalities from chrome to WebKit when I need them.
http://dev.chromium.org/developers/design-documents/threading

[Noam Rosenthal]

I'm OK with the changes to LayerTreeHostProxy, but I don't get to review additions to WTF.

[Dongseong Hwang]

Thanks, Rosenthal.
I think it is hard to persuade to patch TaskQueue.
I postpone it next.

[Dongseong Hwang]

Created attachment 128851 [details]
patch v.3

[Dongseong Hwang]

I focus on only LayerTreeHostProxy without unreasonable invasion to WTF.
I understand only one thread using LayerTreeHostProxy, so I do not use Mutex.

[Dongseong Hwang]

Created attachment 128853 [details]
patch v.4

[Noam Rosenthal]

Comment on attachment 128853 [details]
patch v.4

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

Awesome!
Some touch ups and we're good.

> Source/WebKit2/ChangeLog:8
> +        It is succinct to collect tasks using Functional because of reducing codes about
> +        boilerplate message definition.

Please reword changelog (just an English touch-up):
Use Functional instead of a MessageQueue for messages to the LayerTreeHostProxy renderer. This makes a lot of the broilerplate code for message-passing unnecessary, and results in a much more succinct implementation.

> Source/WebKit2/UIProcess/LayerTreeHostProxy.h:92
> +    Vector<Function<void()> > m_functionQueue;

m_functionQueue -> m_renderQueue

[Dongseong Hwang]

Created attachment 128932 [details]
patch v.5

Thanks for kind review. :)

[WebKit Review Bot]

The commit-queue encountered the following flaky tests while processing attachment 128932 [details]:

css3/filters/effect-contrast-hw.html bug 79618 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

Comment on attachment 128932 [details]
patch v.5

Clearing flags on attachment: 128932

Committed r108951: <http://trac.webkit.org/changeset/108951>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.