Bug 112432 – RenderSnapshottedPlugIn can't be a RenderBlock (what if it's display: inline?)

• Component: Plug-ins
• Status: RESOLVED FIXED

[Tim Horton]

RenderSnapshottedPlugIn can break the layout of pages, because it replaces a RenderReplaced (the plugin's RenderEmbeddedObject) but is a RenderBlock. If the element is inline, madness breaks loose and the page layout goes downhill quickly.

We can make RenderSnapshottedPlugIn a RenderEmbeddedObject subclass with a little bit of care, and rescue ourselves from layout peril.

<rdar://problem/13187211>

[Tim Horton]

Created attachment 193282 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 193282 [details]
patch

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?

[Tim Horton]

Created attachment 193361 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 193361 [details]
patch

Nice!

[Tim Horton]

Comment on attachment 193361 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193361&action=review

> Source/WebCore/page/FrameView.cpp:2532
> +                HTMLPlugInImageElement* pluginElement = static_cast<HTMLPlugInImageElement*>(ownerElement);

I’ll make sure this uses the to* helper.

[Dean Jackson]

Comment on attachment 193361 [details]
patch

I love this. But I wonder why we're still tearing down and swapping renderers now. We should go back to the original idea of using RSPI all the time (until we un-snapshot). That could be a followup patch I guess.

PS. I think we can remove some InlineBox exports too now.

[Tim Horton]

http://trac.webkit.org/changeset/145934