Add a class for resolving style for a document tree. This will enable future features and optimizations.
Created attachment 268367 [details] patch
Comment on attachment 268367 [details] patch Attachment 268367 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/657930 New failing tests: printing/compositing-layer-printing.html printing/setPrinting.html printing/simple-lines-break.html printing/width-overflow.html printing/page-rule-in-media-query.html
Created attachment 268368 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 268367 [details] patch Attachment 268367 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/657932 New failing tests: printing/page-rule-in-media-query.html fast/css/scrollbar-dynamic-style-change.html printing/simple-lines-break.html printing/width-overflow.html printing/setPrinting.html printing/compositing-layer-printing.html
Created attachment 268369 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 268367 [details] patch Attachment 268367 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/657920 New failing tests: printing/compositing-layer-printing.html printing/page-rule-in-media-query.html printing/simple-lines-break.html printing/width-overflow.html printing/setPrinting.html
Created attachment 268370 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 268367 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=268367&action=review > Source/WebCore/style/StyleResolveTree.h:50 > +class TreeResolver { It's odd that the filename and class name don't match. It think you should rename one of them. > Source/WebCore/style/StyleResolveTree.h:59 > + void resolveShadowTree(Change, RenderStyle& inheritedStyle); Unclear from the signature whether inheritedStyle is 'in' or 'out'. > Source/WebCore/style/StyleResolveTree.h:61 > + Ref<RenderStyle> styleForElement(Element&, RenderStyle& inheritedStyle); Ditto. Can inheritedStyle be const? Can Element& be const? Same for most of the other functions.
> It's odd that the filename and class name don't match. It think you should > rename one of them. I'm going to do renames separately. > Ditto. Can inheritedStyle be const? Can Element& be const? Same for most of > the other functions. They should probably be const but that will require lots of changes (without introducing const_cast). It's better done separately.
Created attachment 268392 [details] patch
Weird that the file name is StyleResolveTree, but the class name is TreeResolver. These should match. I also don't think a Style namespace adds anything here. Just calling the class StyleTreeResolver is fine IMO.
(In reply to comment #9) > > It's odd that the filename and class name don't match. It think you should > > rename one of them. > > I'm going to do renames separately. > > > Ditto. Can inheritedStyle be const? Can Element& be const? Same for most of > > the other functions. > > They should probably be const but that will require lots of changes (without > introducing const_cast). It's better done separately. These seem like two good changes to make before doing this one.
> These seem like two good changes to make before doing this one. Does the order really matter? There are many many things to improve here. I'd like to make progress.
Comment on attachment 268392 [details] patch I think either order is fine.
Created attachment 268401 [details] now including file renames due to popular demand
Attachment 268401 [details] did not pass style-queue: ERROR: Source/WebCore/style/StyleTreeResolver.cpp:56: "CSSFontSelector.h" already included at Source/WebCore/style/StyleTreeResolver.cpp:32 [build/include] [4] ERROR: Source/WebCore/style/StyleTreeResolver.cpp:221: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/style/StyleTreeResolver.cpp:858: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/style/StyleTreeResolver.cpp:860: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/style/StyleTreeResolver.cpp:864: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/style/StyleTreeResolver.cpp:878: Missing space before { [whitespace/braces] [5] ERROR: Source/WebCore/style/StyleTreeResolver.h:26: #ifndef header guard has wrong style, please use: StyleTreeResolver_h [build/header_guard] [5] ERROR: Source/WebCore/style/StyleTreeResolver.h:92: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 8 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
https://trac.webkit.org/r194691