Bug 119497 - Move style recalculation out from Element
Summary: Move style recalculation out from Element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-05 15:14 PDT by Antti Koivisto
Modified: 2013-08-07 05:01 PDT (History)
13 users (show)

See Also:


Attachments
for bots (62.78 KB, patch)
2013-08-05 15:26 PDT, Antti Koivisto
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
bots2 (62.75 KB, patch)
2013-08-05 23:48 PDT, Antti Koivisto
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 (325.05 KB, application/zip)
2013-08-06 00:39 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (406.76 KB, application/zip)
2013-08-06 02:03 PDT, Build Bot
no flags Details
another (62.80 KB, patch)
2013-08-06 03:06 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
yet another (62.79 KB, patch)
2013-08-06 03:10 PDT, Antti Koivisto
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
now with all build systems (65.59 KB, patch)
2013-08-06 10:30 PDT, Antti Koivisto
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2013-08-05 15:14:20 PDT
Element currently does too much. The current Element::recalcStyle() and the related functions can be turned into standalone functions that operate on DOM tree. This will also give more freedom for future refactoring, for example making style recalculation non-recursive.
Comment 1 Antti Koivisto 2013-08-05 15:26:23 PDT
Created attachment 208153 [details]
for bots
Comment 2 WebKit Commit Bot 2013-08-05 15:29:21 PDT
Attachment 208153 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/css/StyleRecalculation.cpp', u'Source/WebCore/css/StyleRecalculation.h', u'Source/WebCore/css/StyleResolver.h', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/dom/Document.h', u'Source/WebCore/dom/Element.cpp', u'Source/WebCore/dom/Element.h', u'Source/WebCore/dom/ElementShadow.cpp', u'Source/WebCore/dom/ElementShadow.h', u'Source/WebCore/dom/Node.cpp', u'Source/WebCore/dom/Node.h', u'Source/WebCore/dom/PseudoElement.cpp', u'Source/WebCore/dom/PseudoElement.h', u'Source/WebCore/dom/ShadowRoot.cpp', u'Source/WebCore/dom/ShadowRoot.h', u'Source/WebCore/dom/Text.cpp', u'Source/WebCore/dom/Text.h', u'Source/WebCore/html/HTMLFormControlElement.cpp', u'Source/WebCore/html/HTMLFormControlElement.h', u'Source/WebCore/html/HTMLFrameSetElement.cpp', u'Source/WebCore/html/HTMLFrameSetElement.h', u'Source/WebCore/html/HTMLIFrameElement.cpp', u'Source/WebCore/html/HTMLIFrameElement.h', u'Source/WebCore/html/HTMLMediaElement.cpp', u'Source/WebCore/html/HTMLMediaElement.h', u'Source/WebCore/html/HTMLOptionElement.cpp', u'Source/WebCore/html/HTMLOptionElement.h', u'Source/WebCore/html/HTMLPlugInImageElement.cpp', u'Source/WebCore/html/HTMLPlugInImageElement.h', u'Source/WebCore/inspector/InspectorOverlay.cpp', u'Source/WebCore/loader/DocumentWriter.cpp', u'Source/WebCore/page/Frame.cpp', u'Source/WebCore/page/Page.cpp', u'Source/WebCore/rendering/RenderBlock.cpp', u'Source/WebCore/svg/SVGElement.cpp', u'Source/WebCore/svg/SVGElement.h', u'Source/WebCore/svg/SVGUseElement.cpp', u'Source/WebCore/svg/SVGUseElement.h']" exit_code: 1
Source/WebCore/css/StyleRecalculation.cpp:123:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WebCore/dom/Document.h:49:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Early Warning System Bot 2013-08-05 15:50:32 PDT
Comment on attachment 208153 [details]
for bots

Attachment 208153 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1351018
Comment 4 Early Warning System Bot 2013-08-05 15:53:20 PDT
Comment on attachment 208153 [details]
for bots

Attachment 208153 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1392007
Comment 5 Build Bot 2013-08-05 16:08:43 PDT
Comment on attachment 208153 [details]
for bots

Attachment 208153 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1392010
Comment 6 EFL EWS Bot 2013-08-05 16:19:07 PDT
Comment on attachment 208153 [details]
for bots

Attachment 208153 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1343084
Comment 7 Build Bot 2013-08-05 16:26:03 PDT
Comment on attachment 208153 [details]
for bots

Attachment 208153 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1396011
Comment 8 EFL EWS Bot 2013-08-05 16:51:14 PDT
Comment on attachment 208153 [details]
for bots

Attachment 208153 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1402001
Comment 9 Antti Koivisto 2013-08-05 23:48:24 PDT
Created attachment 208169 [details]
bots2
Comment 10 Early Warning System Bot 2013-08-06 00:09:37 PDT
Comment on attachment 208169 [details]
bots2

Attachment 208169 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1358190
Comment 11 Early Warning System Bot 2013-08-06 00:12:40 PDT
Comment on attachment 208169 [details]
bots2

Attachment 208169 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1352091
Comment 12 Ryosuke Niwa 2013-08-06 00:31:56 PDT
Comment on attachment 208169 [details]
bots2

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

> Source/WebCore/css/StyleRecalculation.h:35
> +namespace Style {

Style might be too generic? How about StyleRecalc or RenderStyleChange?
Comment 13 EFL EWS Bot 2013-08-06 00:33:11 PDT
Comment on attachment 208169 [details]
bots2

Attachment 208169 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1364093
Comment 14 Build Bot 2013-08-06 00:39:08 PDT
Comment on attachment 208169 [details]
bots2

Attachment 208169 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1370410

New failing tests:
css2.1/20110323/at-import-004.htm
css2.1/t040304-c64-uri-00-a-g.html
css2.1/t0603-c11-import-00-b.html
css2.1/20110323/at-import-005.htm
css2.1/20110323/at-import-007.htm
css2.1/t060401-c32-cascading-00-b.html
css2.1/20110323/at-import-009.htm
css1/basic/class_as_selector.html
css1/box_properties/border.html
css1/units/urls.html
css2.1/20110323/at-import-002.htm
css2.1/t040105-atkeyw-00-b.html
compositing/geometry/bounds-ignores-hidden-dynamic-negzindex.html
css2.1/20110323/at-import-006.htm
css1/basic/containment.html
compositing/geometry/bounds-ignores-hidden-dynamic.html
css1/cascade/cascade_order.html
Comment 15 Build Bot 2013-08-06 00:39:11 PDT
Created attachment 208170 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 16 Andreas Kling 2013-08-06 00:55:07 PDT
Comment on attachment 208169 [details]
bots2

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

>> Source/WebCore/css/StyleRecalculation.h:35
>> +namespace Style {
> 
> Style might be too generic? How about StyleRecalc or RenderStyleChange?

I like Style. We can then start moving more things to the namespace and migrate a large chunk of css/ to style/
Comment 17 EFL EWS Bot 2013-08-06 01:25:50 PDT
Comment on attachment 208169 [details]
bots2

Attachment 208169 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1361099
Comment 18 Antti Koivisto 2013-08-06 01:38:32 PDT
(In reply to comment #16)
> (From update of attachment 208169 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208169&action=review
> 
> >> Source/WebCore/css/StyleRecalculation.h:35
> >> +namespace Style {
> > 
> > Style might be too generic? How about StyleRecalc or RenderStyleChange?
> 
> I like Style. We can then start moving more things to the namespace and migrate a large chunk of css/ to style/

That would be the plan.

StyleRule -> Style::Rule
StyleResolver -> Style::Resolver

etc.
Comment 19 Build Bot 2013-08-06 02:03:46 PDT
Comment on attachment 208169 [details]
bots2

Attachment 208169 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1354115

New failing tests:
css2.1/20110323/at-import-005.htm
css1/basic/class_as_selector.html
css2.1/20110323/at-import-006.htm
fast/box-sizing/table-cell.html
css2.1/20110323/at-import-009.htm
fast/block/float/015.html
css3/flexbox/flex-no-flex.html
css1/box_properties/border.html
css2.1/20110323/at-import-002.htm
css3/filters/filter-property-computed-style.html
css1/units/urls.html
css1/cascade/cascade_order.html
fast/block/sticky-position-containing-block-crash.html
http/tests/cache/cancel-during-revalidation-succeeded.html
css2.1/20110323/at-import-007.htm
fast/block/margin-collapse/103.html
fast/body-propagation/background-image/008.html
css2.1/t0603-c11-import-00-b.html
compositing/geometry/bounds-ignores-hidden-dynamic-negzindex.html
css1/basic/containment.html
compositing/geometry/bounds-ignores-hidden-dynamic.html
css2.1/t040304-c64-uri-00-a-g.html
css2.1/20110323/at-import-004.htm
http/tests/css/css-imports-url-fragment.html
css2.1/t060401-c32-cascading-00-b.html
fast/body-propagation/background-color/008.html
fast/block/float/031.html
css2.1/20110323/eof-005.htm
css2.1/t040105-atkeyw-00-b.html
css1/classification/display.html
Comment 20 Build Bot 2013-08-06 02:03:49 PDT
Created attachment 208176 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.4
Comment 21 Antti Koivisto 2013-08-06 03:06:35 PDT
Created attachment 208180 [details]
another

- renamed Style::recalculate -> Style::resolveTree
- fix Force inheritance
Comment 22 Antti Koivisto 2013-08-06 03:10:57 PDT
Created attachment 208181 [details]
yet another
Comment 23 Early Warning System Bot 2013-08-06 03:36:03 PDT
Comment on attachment 208181 [details]
yet another

Attachment 208181 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1354146
Comment 24 Early Warning System Bot 2013-08-06 03:40:36 PDT
Comment on attachment 208181 [details]
yet another

Attachment 208181 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1353143
Comment 25 EFL EWS Bot 2013-08-06 04:21:47 PDT
Comment on attachment 208181 [details]
yet another

Attachment 208181 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1354152
Comment 26 kov's GTK+ EWS bot 2013-08-06 04:30:01 PDT
Comment on attachment 208181 [details]
yet another

Attachment 208181 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1404140
Comment 27 EFL EWS Bot 2013-08-06 04:36:09 PDT
Comment on attachment 208181 [details]
yet another

Attachment 208181 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1351177
Comment 28 Antti Koivisto 2013-08-06 10:30:13 PDT
Created attachment 208200 [details]
now with all build systems
Comment 29 Andreas Kling 2013-08-07 01:57:37 PDT
Comment on attachment 208200 [details]
now with all build systems

r=me
Comment 30 Antti Koivisto 2013-08-07 05:01:17 PDT
https://trac.webkit.org/r153783