[webkit-reviews] review granted: [Bug 108164] Coordinated Graphics : Remove CoordinatedLayerTreeHostProxy dependency from LayerTreeRenderer : [Attachment 185322] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Jae Hyun Park
<jae.park at company100.net>'s request for review:
Bug 108164: Coordinated Graphics : Remove CoordinatedLayerTreeHostProxy
dependency from LayerTreeRenderer
https://bugs.webkit.org/show_bug.cgi?id=108164

Attachment 185322: Patch
https://bugs.webkit.org/attachment.cgi?id=185322&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=185322&action=review


Okay from me given Noam already reviewed.

>
Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.h:98

> +    // LayerTreeRendererClient Methods

Period.

> Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.h:98
> -    // The painting thread must lock the main thread to use below two
methods, because two methods access members that the main thread manages. See
m_coordinatedLayerTreeHostProxy.
> +    // The painting thread must lock the main thread to use below two
methods, because two methods access members that the main thread manages. See
m_client.
>      // Currently, QQuickWebPage::updatePaintNode() locks the main thread
before calling both methods.
>      void purgeGLResources();
>      void setActive(bool);

This is backward. You should not document weird thread consistency issues, you
should encapsulate them.