Join StyleBuilder with StyleResolver again. * In a first step we should move all properties back into a giant switch statement. * We should rename StyleResolver to StyleBuilder * Experiment with better ways to build style in branch after this move
As I mentioned in webkit-dev, I don't think joining them is a good idea. The problem is that the current style building implementation sucks, not that separating it is was a bad idea itself. I think this one should be WONTFIX.
Yeah, I think we can instead move the entire switch statement out of the StyleResolver and move it to StyleBuilder. In fact, we can just repurpose this bug to do that.
(In reply to comment #1) > As I mentioned in webkit-dev, I don't think joining them is a good idea. The problem is that the current style building implementation sucks, not that separating it is was a bad idea itself. I think this one should be WONTFIX. Oh, maybe I misunderstood you. (In reply to comment #2) > Yeah, I think we can instead move the entire switch statement out of the StyleResolver and move it to StyleBuilder. In fact, we can just repurpose this bug to do that. Yes, that would work as well. Antti, what do you think about that?
(In reply to comment #2) > Yeah, I think we can instead move the entire switch statement out of the StyleResolver and move it to StyleBuilder. In fact, we can just repurpose this bug to do that. I looked a bit in moving applyProperty(CSSPropertyID id, CSSValue* value) from StyleResolver to StyleBuilder. Doing that would require to * move some inline and static functions from StyleResolver to StyleBuilder * make the shared StyleBuilder to a not constant resource (atm: const StaticBuilder& in StyleResolver) * solve some circular dependencies All together, it is a quite complex move with a lot of refactoring. Therefore, we should be sure that this is the way to go. Further more, when the template and the switch are in one place, what is the next step?
If it was an easy refactoring it would have been done long time ago. :) I think this should be done in steps, something like this: 1) rename StyleBuilder -> DeprecatedStyleBuilder 2) create new StyleBuilder (it forwards to DeprecatedStyleBuilder for properties that are still implemented there) 3) move the giant switch and the related functions from StyleResolver to StyleBuilder 4) move individual properties from DeprecatedStyleBuilder to StyleBuilder until nothing remains 5) delete DeprecatedStyleBuilder We'll have DeprecatedStyleBuilder and the new StyleBuilder side by side for a while and can verify it is actually better.