Bug 114715 - Move applyProperty from StyleResolver to StyleBuilder.
Summary: Move applyProperty from StyleResolver to StyleBuilder.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 114508
  Show dependency treegraph
 
Reported: 2013-04-16 17:22 PDT by Dirk Schulze
Modified: 2014-03-02 09:05 PST (History)
13 users (show)

See Also:


Attachments
Patch for testing (110.90 KB, patch)
2013-04-16 17:35 PDT, Dirk Schulze
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (118.38 KB, patch)
2013-04-16 18:39 PDT, Dirk Schulze
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (118.36 KB, patch)
2013-04-16 19:38 PDT, Dirk Schulze
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch - with CSS Variables fix (118.94 KB, patch)
2013-04-16 21:57 PDT, Dirk Schulze
koivisto: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Schulze 2013-04-16 17:22:14 PDT
Create a new class StyleBuilder, make it a member of StyleResolver and move applyProperty to this class.
Comment 1 Dirk Schulze 2013-04-16 17:35:12 PDT
Created attachment 198454 [details]
Patch for testing

Test patch on EWS.
Comment 2 WebKit Commit Bot 2013-04-16 17:38:56 PDT
Attachment 198454 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.vcproj/WebCore.vcproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/css/CSSAllInOne.cpp', u'Source/WebCore/css/SVGCSSStyleSelector.cpp', u'Source/WebCore/css/StyleBuilder.cpp', u'Source/WebCore/css/StyleBuilder.h', u'Source/WebCore/css/StyleResolver.cpp', u'Source/WebCore/css/StyleResolver.h', u'Source/WebCore/rendering/style/RenderStyle.h']" exit_code: 1
Source/WebCore/css/SVGCSSStyleSelector.cpp:39:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 EFL EWS Bot 2013-04-16 17:54:33 PDT
Comment on attachment 198454 [details]
Patch for testing

Attachment 198454 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/170112
Comment 4 Dirk Schulze 2013-04-16 18:39:50 PDT
Created attachment 198457 [details]
Patch
Comment 5 WebKit Commit Bot 2013-04-16 18:43:31 PDT
Attachment 198457 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.vcproj/WebCore.vcproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/css/CSSAllInOne.cpp', u'Source/WebCore/css/SVGCSSStyleSelector.cpp', u'Source/WebCore/css/StyleBuilder.cpp', u'Source/WebCore/css/StyleBuilder.h', u'Source/WebCore/css/StyleResolver.cpp', u'Source/WebCore/css/StyleResolver.h', u'Source/WebCore/rendering/style/RenderStyle.h']" exit_code: 1
Source/WebCore/css/StyleBuilder.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/StyleBuilder.cpp:91:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 EFL EWS Bot 2013-04-16 18:44:51 PDT
Comment on attachment 198457 [details]
Patch

Attachment 198457 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/9303
Comment 7 Dirk Schulze 2013-04-16 19:38:04 PDT
Created attachment 198460 [details]
Patch
Comment 8 EFL EWS Bot 2013-04-16 19:43:25 PDT
Comment on attachment 198460 [details]
Patch

Attachment 198460 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/8292
Comment 9 Build Bot 2013-04-16 21:36:34 PDT
Comment on attachment 198460 [details]
Patch

Attachment 198460 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/56179
Comment 10 Dirk Schulze 2013-04-16 21:57:28 PDT
Created attachment 198466 [details]
Patch - with CSS Variables fix
Comment 11 Antti Koivisto 2013-04-17 06:53:36 PDT
Comment on attachment 198466 [details]
Patch - with CSS Variables fix

From IRC discussion it sounds like the goals and the end state of this work is unclear. We shouldn't do large refactoring like this until there is good understanding where we are going.
Comment 12 Yuki Sekiguchi 2013-05-28 03:19:57 PDT
We came to the conclusion in the contributors meeting[1].

What is the status of this bug?
If no one have time to do single switch statement refactoring, I want to do this refactoring.

[1]: https://docs.google.com/document/d/1EsMhk1X9FjPGWByp33NxKqOOJaMwuGP8FzFl19o4L1M/edit?pli=1