[webkit-reviews] review denied: [Bug 112095] [Texmap] Synchronise layers only if the layer has been changed. : [Attachment 192633] Proposal patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam at webkit.org> has denied JungJik Lee
<jungjik.lee at samsung.com>'s request for review:
Bug 112095: [Texmap] Synchronise layers only if the layer has been changed.
https://bugs.webkit.org/show_bug.cgi?id=112095

Attachment 192633: Proposal patch
https://bugs.webkit.org/attachment.cgi?id=192633&action=review

------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=192633&action=review


Please correct nitpicks, otherwise patch looks ok.

> Source/WebCore/ChangeLog:14
> +	   We always append m_layerState to coordinator in
CoordinatedGraphicsLayer::flushCompositingStateForThisLayerOnly.
> +	   This causes renderNext-circular calls continuously. 
> +	   So this patch checks whether the layer needs to be updated or not,
> +	   if we need to update the layer, then we append layer state to
coordinator.
> +	   This will reduce the needless update and IPC calls.
> +
> +	   No new tests (OOPS!).

CoordinatedGraphicsLayer always sets the layer state, which results in an
infinite commit/renderNextFrame loop between the web process in the UI process.

This patch breaks the infinite loop by avoiding commit if the layer state
hasn't changed.

There is no current facility for automatically testing this, however it doesn't
break existing tests.

>
Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cp
p:750
> +    bool needToSyncThisLayer = hasChangedLayerState();

bool layerNeedsSync = needsSync();

>
Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cp
p:775
> +bool CoordinatedGraphicsLayer::hasChangedLayerState()

rename to needsSync()