[webkit-reviews] review denied: [Bug 112432] RenderSnapshottedPlugIn can't be a RenderBlock (what if it's display: inline?) : [Attachment 193282] patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Tim Horton
<timothy_horton at apple.com>'s request for review:
Bug 112432: RenderSnapshottedPlugIn can't be a RenderBlock (what if it's
display: inline?)
https://bugs.webkit.org/show_bug.cgi?id=112432

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=193282&action=review


I like it but think it would benefit from some minor cleanup.

> Source/WebCore/rendering/RenderEmbeddedObject.cpp:295
> +    if (!widget() && frameView() && !isSnapshottedPlugIn())
>	   frameView()->addWidgetToUpdate(this);

It's not clear why a snapshotted plugin should not have to addWidgetToUpdate().
Is it because it will never have a widget? Perhaps we should add a member
function that describes this more clearly?

> Source/WebCore/rendering/RenderEmbeddedObject.cpp:302
> +#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO)
> +    if (!isSnapshottedPlugIn())
> +	   return;
> +#endif

Why not just call canHaveChildren() here?