[webkit-reviews] review denied: [Bug 32086] Repaint issues in text input under a scale transform : [Attachment 45809] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has denied Simon Fraser (smfr) <simon.fraser at apple.com>'s
request for review:
Bug 32086: Repaint issues in text input under a scale transform
https://bugs.webkit.org/show_bug.cgi?id=32086

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

------- Additional Comments from mitz at webkit.org
> +	   (WebCore::RenderView::enableLayoutStateForSubtreeLayout):

This sounds like a method that enables layout state, but it isn’t. How about
“canUseLayoutStateForSubtree()” (I don’t think Layout is necessary) or better
yet “shouldDisableLayoutStateForSubtree()” (with the opposite return value of
course)?

> +    bool enableLayoutState;

If you rename the method to shouldDisable… then you can rename the variable
too.

> +    if (subtree) {
> +	   RenderView* view = root->view();
> +	   enableLayoutState = view->enableLayoutStateForSubtreeLayout(root);
> +	   view->pushLayoutState(root);

Do you have to push if you disable?

> +    bool enableLayoutStateForSubtreeLayout(RenderObject* root) const;

Can drop “root”.

r- because I think the above name is too confusing.