[webkit-reviews] review granted: [Bug 49220] REGRESSION: transform matrix applied too many times when nested : [Attachment 75709] Patch (fix tabs in ChangeLog)

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Dave Hyatt
<hyatt at apple.com>'s request for review:
Bug 49220: REGRESSION: transform matrix applied too many times when nested
https://bugs.webkit.org/show_bug.cgi?id=49220

Attachment 75709: Patch (fix tabs in ChangeLog)
https://bugs.webkit.org/attachment.cgi?id=75709&action=review

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

> WebCore/ChangeLog:82
> +	   Blocks now have a computeOverflow function called at the end that
adds in all the types of overflow.  The addOverflowFromChildren

Called at the end of what?

> WebCore/ChangeLog:99
> +	   (WebCore::RenderBlock::addOverhangingFloats):
> +	   The propagation of float overflow has changed substantially.  The
basic rules are:
> +	       (1) The float must be in our floating objects list to contribute
to overflow.
> +	       (2) The float must be a descendant to contribute to overflow.
> +	       (3) The block must have the outermost list that contains the
float, or it has a self-painting layer and
> +		   so the float needs to be included in its overflow.

Are these covered by layout tests?

> WebCore/rendering/RenderTreeAsText.cpp:621
> +    // FIXME: Apply overflow to the root layer to not break every test. 
Complete hack.	Sigh.

Maybe file a bug on this and reference it in the comment?

> LayoutTests/ChangeLog:33
> +	   Fix for https://bugs.webkit.org/show_bug.cgi?id=49220
<<rdar://problem/8644849>, REGRESSION: transforms now
> +	   O(n^3) from pathological behavior in lowestPosition,
rightmostPosition, leftmostPosition and topmostPosition.
> +
> +	   This patch throws out the lowest/rightmost/leftmost/topmostPosition
functions and re-architects layout overflow
> +	   in the engine to cache all the information required to properly
handle scrolling.
> +
> +	   In the old code, there were two types of overflow: layout overflow
and visual overflow.  The former could
> +	   affect scrolling and the latter could not.  The distinction was
largely meaningless, since layout overflow
> +	   wasn't actually used to determine scroll width or scroll height.  It
didn't propagate across self-painting layer
> +	   boundaries either.  In the old code, the term visible overflow meant
the union of the layout overflow and
> +	   visual overflow rects.
> +
> +	   In the new code, the two types of overflow remain, but the
distinction between the two is now clear.  Visual overflow
> +	   is used purely for painting and hit testing checks and layout
overflow is used specifically for scrolling.  It has
> +	   been expanded to propagate across self-painting layers, to factor in
relative positioning and transforms, and to
> +	   work with writing modes.
> +
> +	   In order to minimize layout test changes, layers no longer
incorporate right/bottom overflow into their width/height members.
> +	   Doing so uncovered two bugs where left/top overflow was ignored
(proof that even having layer dimensions is harmful).
> +	   A render tree dump hack has been put into the code to keep this
overflow dumping for the RenderView's layer, since otherwise
> +	   a huge number of tests would change.
> +
> +	   Added fast/overflow/overflow-rtl-vertical.html to test vertical
writing-mode overflow.	Existing tests cover the rest.
> +
> +	   * page/FrameView.cpp:
> +	   (WebCore::FrameView::adjustViewSize):
> +	   (WebCore::FrameView::forceLayoutForPagination):
> +	   Changed to use RenderView's docTop/Left/Width/Height accessors,
which simply grab the overflow and properly flip it
> +	   to account for writing modes.

You can just describe the LayoutTests changes here.