Bug 79478 - LayerTreeHostProxy uses Functional instead of MessageQueue for accumulating tasks.
Summary: LayerTreeHostProxy uses Functional instead of MessageQueue for accumulating t...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-24 05:01 PST by Dongseong Hwang
Modified: 2012-02-26 20:13 PST (History)
8 users (show)

See Also:


Attachments
patch (26.98 KB, patch)
2012-02-24 05:05 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
patch v.2 (26.80 KB, patch)
2012-02-24 05:09 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
patch v.3 (15.29 KB, patch)
2012-02-24 21:38 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
patch v.4 (15.32 KB, patch)
2012-02-24 21:47 PST, Dongseong Hwang
noam: review-
noam: commit-queue-
Details | Formatted Diff | Diff
patch v.5 (15.35 KB, patch)
2012-02-26 15:18 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2012-02-24 05:01:42 PST
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.
Comment 1 Dongseong Hwang 2012-02-24 05:05:12 PST
Created attachment 128713 [details]
patch
Comment 2 Dongseong Hwang 2012-02-24 05:06:07 PST
I will apply TaskQueue to RunLoop and etc.
Comment 3 WebKit Commit Bot 2012-02-24 05:07:00 PST
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.
Comment 4 Dongseong Hwang 2012-02-24 05:09:52 PST
Created attachment 128715 [details]
patch v.2
Comment 5 Dongseong Hwang 2012-02-24 05:19:05 PST
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
Comment 6 Noam Rosenthal 2012-02-24 07:16:35 PST
I'm OK with the changes to LayerTreeHostProxy, but I don't get to review additions to WTF.
Comment 7 Dongseong Hwang 2012-02-24 21:09:27 PST
Thanks, Rosenthal.
I think it is hard to persuade to patch TaskQueue.
I postpone it next.
Comment 8 Dongseong Hwang 2012-02-24 21:38:47 PST
Created attachment 128851 [details]
patch v.3
Comment 9 Dongseong Hwang 2012-02-24 21:43:04 PST
I focus on only LayerTreeHostProxy without unreasonable invasion to WTF.
I understand only one thread using LayerTreeHostProxy, so I do not use Mutex.
Comment 10 Dongseong Hwang 2012-02-24 21:47:52 PST
Created attachment 128853 [details]
patch v.4
Comment 11 Noam Rosenthal 2012-02-25 07:55:01 PST
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
Comment 12 Dongseong Hwang 2012-02-26 15:18:15 PST
Created attachment 128932 [details]
patch v.5

Thanks for kind review. :)
Comment 13 WebKit Review Bot 2012-02-26 20:11:48 PST
The commit-queue encountered the following flaky tests while processing attachment 128932 [details]:

css3/filters/effect-contrast-hw.html bug 79618 (author: cmarrin@apple.com)
The commit-queue is continuing to process your patch.
Comment 14 WebKit Review Bot 2012-02-26 20:13:48 PST
Comment on attachment 128932 [details]
patch v.5

Clearing flags on attachment: 128932

Committed r108951: <http://trac.webkit.org/changeset/108951>
Comment 15 WebKit Review Bot 2012-02-26 20:13:56 PST
All reviewed patches have been landed.  Closing bug.