Summary: | Move RenderObject::layout() to RenderElement. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||
Component: | Layout and Rendering | Assignee: | Andreas Kling <kling> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, dbarton, d-r, esprehn+autocc, fmalita, fred.wang, glenn, gyuyoung.kim, kondapallykalyan, mrobinson, pdr, schenney | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Andreas Kling
2013-10-08 18:04:57 PDT
Created attachment 213738 [details]
Patch
Created attachment 213739 [details]
Patch
Eh that was not at all this patch.
Attachment 213739 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/page/FrameView.cpp', u'Source/WebCore/rendering/RenderBlock.cpp', u'Source/WebCore/rendering/RenderBlockLineLayout.cpp', u'Source/WebCore/rendering/RenderBox.cpp', u'Source/WebCore/rendering/RenderElement.cpp', u'Source/WebCore/rendering/RenderElement.h', u'Source/WebCore/rendering/RenderObject.cpp', u'Source/WebCore/rendering/RenderObject.h', u'Source/WebCore/rendering/RenderTable.cpp', u'Source/WebCore/rendering/RenderText.h', u'Source/WebCore/rendering/mathml/RenderMathMLRoot.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLRow.cpp', u'Source/WebCore/rendering/svg/SVGRenderSupport.cpp']" exit_code: 1
Source/WebCore/rendering/RenderElement.h:96: More than one command on the same line in if [whitespace/parens] [4]
Total errors found: 1 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 213739 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213739&action=review > Source/WebCore/rendering/RenderBox.cpp:458 > while (child) { > - child->layoutIfNeeded(); > + if (child->needsLayout()) > + toRenderElement(child)->layout(); > ASSERT(!child->needsLayout()); > child = child->nextSibling(); RenderBoxes can have text children. The reason this works is that everyone who has them also overriders layout(). Rather than relying on this a more stylish solution would be to remove RenderBox::layout() completely and move the relevant portions code to the (probably) few subclasses that actually use it. I did this when moving paint(). > Source/WebCore/rendering/RenderElement.cpp:1088 > +void RenderElement::layout() > +{ > + StackStats::LayoutCheckPoint layoutCheckPoint; > + ASSERT(needsLayout()); > + RenderObject* child = firstChild(); > + while (child) { > + if (child->needsLayout()) > + toRenderElement(child)->layout(); > + ASSERT(!child->needsLayout()); > + child = child->nextSibling(); > + } > + clearNeedsLayout(); > +} Same comment as for RenderBox. RenderElement::paint() is pure virtual. Maybe this can be too? > Source/WebCore/rendering/RenderElement.h:96 > + /* This function performs a layout only if one is needed. */ > + void layoutIfNeeded() { if (needsLayout()) layout(); } This comment adds no useful information and uses non-standard format too. > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:275 > - child->layout(); > + toRenderElement(child)->layout(); How do we know this is a RenderElement? Comment on attachment 213739 [details] Patch Clearing flags on attachment: 213739 Committed r157222: <http://trac.webkit.org/changeset/157222> All reviewed patches have been landed. Closing bug. |