[webkit-reviews] review granted: [Bug 173762] [ThreadedCompositor] Update and retrieve scene attributes under a Lock : [Attachment 313704] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 173762: [ThreadedCompositor] Update and retrieve scene attributes under a
Lock
https://bugs.webkit.org/show_bug.cgi?id=173762

Attachment 313704: Patch

https://bugs.webkit.org/attachment.cgi?id=313704&action=review




--- Comment #2 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 313704
  --> https://bugs.webkit.org/attachment.cgi?id=313704
Patch

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

>
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor
.cpp:208
> +    WebCore::IntSize viewportSize;
> +    WebCore::IntPoint scrollPosition;

We don't need the WebCore::

>
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor
.cpp:221
> +    float scaleFactor;
> +    bool drawsBackground;
> +    bool needsResize;
> +    {
> +	   LockHolder locker(m_attributes.lock);
> +	   viewportSize = m_attributes.viewportSize;
> +	   scrollPosition = m_attributes.scrollPosition;
> +	   scaleFactor = m_attributes.scaleFactor;
> +	   drawsBackground = m_attributes.drawsBackground;
> +	   needsResize = m_attributes.needsResize;
> +
> +	   // Reset the needsResize attribute to false.
> +	   m_attributes.needsResize = false;

Could we simply all this using WTFMove()? Maybe leaving the lock out of the
struct and overriding the move in the struct to ensure a reset state after the
move.