[webkit-reviews] review denied: [Bug 20947] Refactor RenderObject::setStyle code : [Attachment 24194] Remove all but one setStyle override with styleWillChange/styleDidChange.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Simon Fraser
<simon.fraser at apple.com>'s request for review:
Bug 20947: Refactor RenderObject::setStyle code
https://bugs.webkit.org/show_bug.cgi?id=20947

Attachment 24194: Remove all but one setStyle override with
styleWillChange/styleDidChange.
https://bugs.webkit.org/attachment.cgi?id=24194&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
This is better than making render objects bigger, but it would be nice to avoid
the global variables.

I have two ideas:

1) We probably don't need a dynamic open ended system for this, since we don't
host arbitrary renderer classes from add-ons or anything like that.

I propse a structure with some preallocated slots for the data needed by
various RenderObject derived classes. We can comment it to make it clear which
slots are for which derived classes. Something like this:

    struct CachedStateFromBeforeStyleChange {
	// RenderObject
	bool affectsParentBlock;

	// RenderBox
	bool boxWasFloating;
	bool boxHadOverflowClip;
    }

We'd pass a CachedStateFromBeforeStyleChange& into styleWillChange and a const
CachedStateFromBeforeStyleChange& into styleDidChange.

2) A more-complex but possibly more elegant option would be to make versions of
the functions that answer these key questions such as affectsParentBlock,
isFloating, and hasOverflowClip take a RenderStyle* argument so that we could
use them in the "did" functions to answer the questions about the old state.
That might be tricky to get right and to maintain, but I think it's a cleaner
solution so worth considering.

Lets try one of the non-global-variable solutions, so review- on this one.

If after thinking it over you want to stick with the global variable approach,
feel free to set the review flag to review? again; I'm not strongly opposed.