Bug 118265

Summary: [GTK][ThreadedCompositor] Add support for threaded compositor
Product: WebKit Reporter: Gwang Yoon Hwang <yoon>
Component: Layout and RenderingAssignee: Gwang Yoon Hwang <yoon>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, cgarcia, changseok, commit-queue, jaepark, mrobinson, noam, sergio, skyul, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 117227, 118383    
Bug Blocks: 100341    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Gwang Yoon Hwang 2013-07-01 15:33:10 PDT
By implementing ThreadedCoordinatedLayerTreeHost, this patch supports
    threaded compositor for WK2. In this initial implementation, threaded
    compositor only supports fixedLayout.
    
    COORDINATED_GRAPHICS_IPC is introduced to classify IPC specific codes
    from Coordinated Graphics.
    
    Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp :
        Calls XInitThreads to use thread-safe xlib.
    
    No new tests, this is an experimental feature.
Comment 1 Gwang Yoon Hwang 2013-07-01 16:10:03 PDT
Created attachment 205842 [details]
Patch
Comment 2 Gwang Yoon Hwang 2013-07-04 05:05:50 PDT
Created attachment 206079 [details]
Patch
Comment 3 Gwang Yoon Hwang 2014-12-15 04:17:34 PST
Created attachment 243287 [details]
Patch
Comment 4 Gwang Yoon Hwang 2015-01-06 06:58:13 PST
Created attachment 244059 [details]
Patch
Comment 5 Gwang Yoon Hwang 2015-01-06 09:05:08 PST
Created attachment 244065 [details]
Patch
Comment 6 Gwang Yoon Hwang 2015-01-06 09:14:50 PST
Created attachment 244066 [details]
Patch
Comment 7 Gwang Yoon Hwang 2015-01-06 09:17:02 PST
(In reply to comment #6)
> Created attachment 244066 [details]
> Patch

Sorry for noise.
I made a little typo in previous patch.

Changes:

* Change COORDINATED_GRAPHICS_IPC to COORDINATED_GRAPHICS_MULITIPROCESS
* Rebase for current HEAD
Comment 8 Martin Robinson 2015-01-06 11:39:18 PST
Comment on attachment 244066 [details]
Patch

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

Looks good to me, thought the changes to platform independent files will need to be approved by Apple.

> Source/WebKit2/ChangeLog:8
> +        This patch introduces the Threaded Compositor for WebKitGTK+.

Nit: the Threaded Compositor -> a threaded compositor

> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:527
> +#if USE(COORDINATED_GRAPHICS_MULTIPROCESS)
>      m_page->send(Messages::WebPageProxy::DidChangeContentSize(size));
>  #endif
> +#endif

These blocks do not have to be nested. I think it's clearer to not nest them actually.

> Source/cmake/WebKitFeatures.cmake:152
> +    WEBKIT_OPTION_DEFINE(ENABLE_THREADED_COMPOSITOR "Toggle Toggle Threaded Compositor support" OFF)

Nit: Threaded Compositor -> threaded compositor

> Tools/Scripts/webkitperl/FeatureList.pm:386
> +    { option => "threaded-compositor", desc => "Toggle Threaded Compositor support",

Ditto.
Comment 9 Gwang Yoon Hwang 2015-01-06 20:07:00 PST
Created attachment 244129 [details]
Patch
Comment 10 Gwang Yoon Hwang 2015-01-06 20:14:23 PST
(In reply to comment #8)
> Comment on attachment 244066 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244066&action=review
> 
> Looks good to me, thought the changes to platform independent files will
> need to be approved by Apple.
> 
Okay, I'll ask for approval later.

> > Source/WebKit2/ChangeLog:8
> > +        This patch introduces the Threaded Compositor for WebKitGTK+.
> 
> Nit: the Threaded Compositor -> a threaded compositor
> 
> > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:527
> > +#if USE(COORDINATED_GRAPHICS_MULTIPROCESS)
> >      m_page->send(Messages::WebPageProxy::DidChangeContentSize(size));
> >  #endif
> > +#endif
> 
> These blocks do not have to be nested. I think it's clearer to not nest them
> actually.
> 

Good point. Modified.

> > Source/cmake/WebKitFeatures.cmake:152
> > +    WEBKIT_OPTION_DEFINE(ENABLE_THREADED_COMPOSITOR "Toggle Toggle Threaded Compositor support" OFF)
> 
> Nit: Threaded Compositor -> threaded compositor
> 

Fixed. :)

> > Tools/Scripts/webkitperl/FeatureList.pm:386
> > +    { option => "threaded-compositor", desc => "Toggle Threaded Compositor support",
> 
> Ditto.

Fixed.
Comment 11 Anders Carlsson 2015-01-07 12:39:16 PST
Comment on attachment 244066 [details]
Patch

Cross platform files look fine to me.
Comment 12 WebKit Commit Bot 2015-01-07 20:29:25 PST
Comment on attachment 244129 [details]
Patch

Clearing flags on attachment: 244129

Committed r178095: <http://trac.webkit.org/changeset/178095>
Comment 13 WebKit Commit Bot 2015-01-07 20:29:31 PST
All reviewed patches have been landed.  Closing bug.