[webkit-reviews] review granted: [Bug 135227] [WK2] Fixed/Sticky layers can get mispositioned when the layer tree commit change their position or size : [Attachment 235404] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 135227: [WK2] Fixed/Sticky layers can get mispositioned when the layer tree
commit change their position or size
https://bugs.webkit.org/show_bug.cgi?id=135227

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=235404&action=review


> Source/WebCore/page/scrolling/ScrollingTree.h:134
> +    void fixedOrStickyNodeRemoved() { --m_fixedOrStickyNodeCount; }

Should assert that m_fixedOrStickyNodeCount isn't going to underflow.

> Source/WebCore/page/scrolling/mac/ScrollingTreeFixedNode.mm:50
> +    scrollingTree().fixedOrStickyNodeRemoved();

Not a huge fan of doing work like this in the destructor. I think it would be
better to make more explicit didAddNode/didRemoveNode (which also enter the
node in m_nodeMap), but maybe that's too risky at this point.