[webkit-reviews] review granted: [Bug 57130] Relative mouse coordinates recalculated for each target : [Attachment 87003] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Emil A Eklund
<eae at chromium.org>'s request for review:
Bug 57130: Relative mouse coordinates recalculated for each target
https://bugs.webkit.org/show_bug.cgi?id=57130

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=87003&action=review

Looks great with nits. Love the test.

> Source/WebCore/ChangeLog:10
> +	   structures significantly.

Probably should mention that this is changes O(N^2) to O(N). Don't be shy :)

> Source/WebCore/dom/MouseRelatedEvent.cpp:141
> +    m_hasCachedRelativePosition = false;

You can optimize this even further by checking (just outside receivedTarget
callsite) to see if target is actually changing.

> Source/WebCore/dom/MouseRelatedEvent.h:44
> +	   int layerX() const;
> +	   int layerY() const;
> +	   int offsetX() const;
> +	   int offsetY() const;

These aren't const anymore, right? They will change the object.

> Source/WebCore/dom/MouseRelatedEvent.h:67
> +	   void computeRelativePosition() const;

Ditto.

> Source/WebCore/dom/MouseRelatedEvent.h:84
> +	   mutable int m_layerX;
> +	   mutable int m_layerY;
> +	   mutable int m_offsetX;
> +	   mutable int m_offsetY;
> +	   mutable bool m_hasCachedRelativePosition;

And once you remove consts, these don't need to be mutables.