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

[bugzilla-daemon at webkit.org]

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
https://bugs.webkit.org/show_bug.cgi?id=200668

Attachment 376161: patch

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




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

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
context.

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.