Bug 118265 – [GTK][ThreadedCompositor] Add support for threaded compositor

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Gwang Yoon Hwang]

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.

[Gwang Yoon Hwang]

Created attachment 205842 [details]
Patch

[Gwang Yoon Hwang]

Created attachment 206079 [details]
Patch

[Gwang Yoon Hwang]

Created attachment 243287 [details]
Patch

[Gwang Yoon Hwang]

Created attachment 244059 [details]
Patch

[Gwang Yoon Hwang]

Created attachment 244065 [details]
Patch

[Gwang Yoon Hwang]

Created attachment 244066 [details]
Patch

[Gwang Yoon Hwang]

(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

[Martin Robinson]

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.

[Gwang Yoon Hwang]

Created attachment 244129 [details]
Patch

[Gwang Yoon Hwang]

(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.

[Anders Carlsson]

Comment on attachment 244066 [details]
Patch

Cross platform files look fine to me.

[WebKit Commit Bot]

Comment on attachment 244129 [details]
Patch

Clearing flags on attachment: 244129

Committed r178095: <http://trac.webkit.org/changeset/178095>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.