[webkit-reviews] review denied: [Bug 42046] connect-compositing-iframe2.html test sometimes shows blank iframe content : [Attachment 63024] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 29 23:30:31 PDT 2010


Darin Adler <darin at apple.com> has denied Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 42046: connect-compositing-iframe2.html test sometimes shows blank iframe
content
https://bugs.webkit.org/show_bug.cgi?id=42046

Attachment 63024: Patch
https://bugs.webkit.org/attachment.cgi?id=63024&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Two thoughts on how to refine this further:

    1) setNeedsStyleRecalc no longer seems like a great name to me since this
doesn't set a flag named "setNeedsStyleRecalc". It's still a bit like
setNeedsDisplayInRect, so I guess we can leave the name along. But maybe we can
find a better name.

    2) setNeedsStyleRecalc should assert that changeType is != NoStyleChange,
in case we missed any callers who need to call the new function.

The setStyleChange function can't be called outside of Node.cpp because it's
defined inside the file and marked inline. Thus if you want to call it from
clearNeedsStyleRecalc you need to either move Node::setStyleChange to the
header or move the body of clearNeedsStyleRecalc into the .cpp file and make it
non-inline.

Or you could write this:

    void clearNeedsStyleRecalc() { m_nodeFlags &= ~StyleChangeMask; }

Which would have the added benefit of being more efficient.

review- since it breaks the build as-is


More information about the webkit-reviews mailing list