Bug 79478

Summary: LayerTreeHostProxy uses Functional instead of MessageQueue for accumulating tasks.
Product: WebKit Reporter: Dongseong Hwang <dongseong.hwang>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, levin+threading, menard, noam, rakuco, skyul, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch v.2
none
patch v.3
none
patch v.4
noam: review-, noam: commit-queue-
patch v.5 none

Dongseong Hwang
Reported 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.
Attachments
patch (26.98 KB, patch)
2012-02-24 05:05 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
patch v.2 (26.80 KB, patch)
2012-02-24 05:09 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
patch v.3 (15.29 KB, patch)
2012-02-24 21:38 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
patch v.4 (15.32 KB, patch)
2012-02-24 21:47 PST, Dongseong Hwang 		noam: review-
noam: commit-queue-
DetailsFormatted DiffDiff
patch v.5 (15.35 KB, patch)
2012-02-26 15:18 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Dongseong Hwang
Comment 1 2012-02-24 05:05:12 PST
Created attachment 128713 [details] patch
Dongseong Hwang
Comment 2 2012-02-24 05:06:07 PST
I will apply TaskQueue to RunLoop and etc.
WebKit Commit Bot
Comment 3 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.
Dongseong Hwang
Comment 4 2012-02-24 05:09:52 PST
Created attachment 128715 [details] patch v.2
Dongseong Hwang
Comment 5 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
Noam Rosenthal
Comment 6 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.
Dongseong Hwang
Comment 7 2012-02-24 21:09:27 PST
Thanks, Rosenthal. I think it is hard to persuade to patch TaskQueue. I postpone it next.
Dongseong Hwang
Comment 8 2012-02-24 21:38:47 PST
Created attachment 128851 [details] patch v.3
Dongseong Hwang
Comment 9 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.
Dongseong Hwang
Comment 10 2012-02-24 21:47:52 PST
Created attachment 128853 [details] patch v.4
Noam Rosenthal
Comment 11 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
Dongseong Hwang
Comment 12 2012-02-26 15:18:15 PST
Created attachment 128932 [details] patch v.5 Thanks for kind review. :)
WebKit Review Bot
Comment 13 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.
WebKit Review Bot
Comment 14 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>
WebKit Review Bot
Comment 15 2012-02-26 20:13:56 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.