Bug 122959 - [CSSRegions] Move regions auto-size code into RenderNamedFlowFragment
Summary: [CSSRegions] Move regions auto-size code into RenderNamedFlowFragment
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andrei Bucur
Keywords: AdobeTracked
Depends on:
Blocks: 57312
  Show dependency treegraph
Reported: 2013-10-17 05:28 PDT by Mihnea Ovidenie
Modified: 2014-01-09 04:02 PST (History)
7 users (show)

See Also:

Patch (31.38 KB, patch)
2014-01-08 07:59 PST, Andrei Bucur
no flags Details | Formatted Diff | Diff
Patch (29.58 KB, patch)
2014-01-09 02:04 PST, Andrei Bucur
no flags Details | Formatted Diff | Diff
Patch (28.71 KB, patch)
2014-01-09 03:26 PST, Andrei Bucur
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihnea Ovidenie 2013-10-17 05:28:26 PDT
After https://bugs.webkit.org/show_bug.cgi?id=119135, the region auto-size code should be moved from RenderRegion class to the new RenderNamedFlowFragment class. RenderNamedFlowFragment will contain functionality specific to CSSRegions only, as it is the case for region auto-size.
Comment 1 Andrei Bucur 2014-01-08 07:59:23 PST
Created attachment 220633 [details]
Comment 2 Mihnea Ovidenie 2014-01-08 08:23:47 PST
(In reply to comment #1)
> Created an attachment (id=220633) [details]
> Patch

Would not have been easier to make *hasAutoLogicalHeight* a virtual which returns false in RenderRegion and do the check in RenderNamedFlowFragment? The static implementation in RenderFlowThread calls isRenderNamedFlowFragment() which is already a virtual method.
Comment 3 Andrei Bucur 2014-01-09 02:04:09 PST
Created attachment 220704 [details]
Comment 4 Mihnea Ovidenie 2014-01-09 02:15:45 PST
Comment on attachment 220704 [details]

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

r=me, please take a look at the comments before landing.

> Source/WebCore/rendering/RenderNamedFlowFragment.cpp:159
> +    ASSERT(newLogicalHeight < LayoutUnit::max() / 2);

I would use RenderFlowThread::maxLogicalHeight() instead of LayoutUnit::max() / 2.

> Source/WebCore/rendering/RenderTreeAsText.cpp:671
> +            if (isRenderNamedFlowFragment && toRenderNamedFlowFragment(renderRegion)->hasAutoLogicalHeight())

Since hasAutoLogicalHeight always returns false for RenderRegion, you can still use the previous condition.
Comment 5 Andrei Bucur 2014-01-09 03:26:22 PST
Created attachment 220709 [details]
Comment 6 WebKit Commit Bot 2014-01-09 04:02:22 PST
Comment on attachment 220709 [details]

Clearing flags on attachment: 220709

Committed r161553: <http://trac.webkit.org/changeset/161553>
Comment 7 WebKit Commit Bot 2014-01-09 04:02:25 PST
All reviewed patches have been landed.  Closing bug.