[webkit-reviews] review granted: [Bug 200668] Event region collection should take clipping into account : [Attachment 376161] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 13 10:49:07 PDT 2019

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Antti Koivisto
<koivisto at iki.fi>'s request for review:
Bug 200668: Event region collection should take clipping into account

Attachment 376161: patch


--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 376161
  --> https://bugs.webkit.org/attachment.cgi?id=376161

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

> Source/WebCore/rendering/EventRegion.cpp:63
> +    m_clipStack.removeLast();

If we have mismatched push/pop, this will do shrink(size() - 1); which will
underflow and create a huge vector and probably crash. I think we should be
defensive here and check for m_clipStack not being empty.

This is especially true since we don't have save/restore, so you're doing
manual push/pop.

> Source/WebCore/rendering/RenderBox.cpp:1821
> +    if (paintInfo.phase == PaintPhase::EventRegion)
> +	   paintInfo.eventRegionContext->popClip();
> +
>      paintInfo.context().restore();

This is a manual pop, but the painting code is doing a save/restore. Maybe in
future it would be better to implement save/restore for the event region

If I'm changing painting code in future, it's going to be really hard to know
how to change the event region code.

Also, there may be other code paths that save/clip/restore that need to also do
something with the event region.

More information about the webkit-reviews mailing list