Bug 92367 – Move region from HitTestResult to HitTestPoint

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Allan Sandfeld Jensen]

HitTestResult currently keeps track of a region in which the current hit-test is being performed, but the region is not really specific to the result, and is more closely tied to the current point being tested, therefore it makes more sense to store the region in the HitTestPoint instead of the HitTestResult.

The also makes it possible to get rid of setRegion() since it was only used to override and restore the original region in HitTestResult, since a new HitTestPoint is made when descending into a new region, this save/restore mechanic is no longer necessary, and region can be made a constant in HitTestPoint.

[Allan Sandfeld Jensen]

Created attachment 154610 [details]
Patch

[Antonio Gomes]

Comment on attachment 154610 [details]
Patch

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

r=me

> Source/WebCore/rendering/HitTestResult.cpp:107
> +HitTestPoint::HitTestPoint(const HitTestPoint& other, const LayoutSize& offset, RenderRegion* region)
> +    : m_point(other.m_point)
> +    , m_boundingBox(other.m_boundingBox)
> +    , m_transformedPoint(other.m_transformedPoint)
> +    , m_transformedRect(other.m_transformedRect)
> +    , m_region(region)

I wonder who owns this point. Can it be deleted elsewhere, and we get garbage here?

[Antonio Gomes]

> 
> I wonder who owns this point. Can it be deleted elsewhere, and we get garbage here?

s/point/pointer/g

[Allan Sandfeld Jensen]

(In reply to comment #2)
> (From update of attachment 154610 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154610&action=review
> 
> r=me
> 
> > Source/WebCore/rendering/HitTestResult.cpp:107
> > +HitTestPoint::HitTestPoint(const HitTestPoint& other, const LayoutSize& offset, RenderRegion* region)
> > +    : m_point(other.m_point)
> > +    , m_boundingBox(other.m_boundingBox)
> > +    , m_transformedPoint(other.m_transformedPoint)
> > +    , m_transformedRect(other.m_transformedRect)
> > +    , m_region(region)
> 
> I wonder who owns this point. Can it be deleted elsewhere, and we get garbage here?

It is owned by whoever created it, in this case in it just exists on the stack where it is created in renderflowthread, and is deleted automatically when that function returns.

[WebKit Review Bot]

Comment on attachment 154610 [details]
Patch

Clearing flags on attachment: 154610

Committed r123754: <http://trac.webkit.org/changeset/123754>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.