Bug 98322 – Add render object that paints plugin snapshots

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Jon Lee]

Add a new render class that knows how to paint poster images when the plugin is inactive.

[Radar WebKit Bug Importer]

<rdar://problem/12426546>

[Jon Lee]

Created attachment 167559 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 167559 [details]
Patch

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

> Source/WebCore/html/HTMLPlugInImageElement.cpp:55
>      , m_needsDocumentActivationCallbacks(false)
>  {
>      setHasCustomCallbacks();
> +
> +    if (document->page() && document->page()->settings()->plugInSnapshottingEnabled())
> +        setDisplayState(WaitingForSnapshot);
>  }

Are you missing initialization of m_displayState here in other cases?

> Source/WebCore/html/HTMLPlugInImageElement.cpp:273
> +    if (image) {
> +        toRenderSnapshottedPlugin(renderer())->snapshotResource()->setCachedImage(new CachedImage(image));
> +        renderer()->repaint();
> +    }

What happens if the plugin is display:none (or became display:none during the waiting phase)? renderer() could be null.

> Source/WebCore/rendering/RenderSnapshottedPlugin.cpp:77
> +void RenderSnapshottedPlugin::layout()
> +{
> +    ASSERT(needsLayout());
> +
> +    updateLogicalWidth();
> +    updateLogicalHeight();
> +
> +    if (plugInImageElement()->displayState() < HTMLPlugInElement::Playing)
> +        RenderBox::layout();
> +    else
> +        RenderPart::layout();
> +
> +    m_overflow.clear();
> +    addVisualEffectOverflow();
> +
> +    updateLayerTransform();
> +
> +    if (!widget() && frameView())
> +        frameView()->addWidgetToUpdate(this);
> +
> +    setNeedsLayout(false);
> +}

It's a bit odd that you need special layout behavior. Are there cases where the layout of the element changes when it goes from live to snapshot, or vice versa?

[Build Bot]

Comment on attachment 167559 [details]
Patch

Attachment 167559 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14214391

[Jon Lee]

Comment on attachment 167559 [details]
Patch

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

>> Source/WebCore/html/HTMLPlugInImageElement.cpp:55
>>  }
> 
> Are you missing initialization of m_displayState here in other cases?

No. In HTMLPlugInElement it sets the default state to Playing. Only this subclass overrides that setting.

>> Source/WebCore/html/HTMLPlugInImageElement.cpp:273
>> +    }
> 
> What happens if the plugin is display:none (or became display:none during the waiting phase)? renderer() could be null.

Hmm. With display:none we could still obtain the snapshot from the plugin. Does that mean I should hold the snapshotResource in the HTML element instead of the renderer?

>> Source/WebCore/rendering/RenderSnapshottedPlugin.cpp:77
>> +}
> 
> It's a bit odd that you need special layout behavior. Are there cases where the layout of the element changes when it goes from live to snapshot, or vice versa?

No. This code is exactly the same as RenderEmbeddedObject::layout(), except that if we are snapshotting I use RenderBox::layout() instead of RenderPart::layout(). I didn't know how to factor that one part out without making this function needlessly complicated. Any suggestion?

> Source/WebCore/rendering/RenderSnapshottedPlugin.cpp:130
> +void RenderSnapshottedPlugin::paintReplacedOverlay(PaintInfo& paintInfo, const LayoutPoint& paintOffset)

This needs to be moved to RenderTheme. Will post a new patch.

[Jon Lee]

Created attachment 167595 [details]
Patch

[Jon Lee]

Comment on attachment 167595 [details]
Patch

Moved the overlay rendering code out to RenderTheme. Fixed, hopefully, Windows build error.

[Jon Lee]

Created attachment 167603 [details]
Removed layout() call, renamed setSnapshot to updateSnapshot, and moved setting the snapshot into RenderSnapshottedPlugin to make cleaner code

[Jon Lee]

Comment on attachment 167603 [details]
Removed layout() call, renamed setSnapshot to updateSnapshot, and moved setting the snapshot into RenderSnapshottedPlugin to make cleaner code

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

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:7823
> +		2981CAA5131822EC00D12F2A /* AccessibilityObject.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; path = AccessibilityObject.cpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };

Not sure what happened here. Will remove this when checked in.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:11265
> +		A81872140977D3C0005826D9 /* ContainerNode.cpp */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; path = ContainerNode.cpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };

Ditto.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:12812
> +		BC772B370C4EA91E0083285F /* CSSParser.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; path = CSSParser.cpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };

And here too.

[Jon Lee]

Committed r130688: <http://trac.webkit.org/changeset/130688>