Bug 110495 – Plug-in snapshotting code always accepts first snapshot

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Dean Jackson]

An HTMLPlugInImageElement swaps to a different renderer when it receives a snapshot from its embedding API (currently only WK2). This is a regression - previously we'd take a number of snapshots waiting for an image that had useful data, and there was a common renderer for the plugin and snapshot.

This is the first part of the fix, which is to have the embedder tell the plugin when it thinks it has enough to snapshot. Only then will we swap to the other renderer.

A later fix will be to make sure that we show intermediate (if mostly empty) snapshots.

[Dean Jackson]

Created attachment 189570 [details]
Patch

[Tim Horton]

Comment on attachment 189570 [details]
Patch

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

> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:74
> -static const unsigned maximumSnapshotRetries = 60;
> +static const unsigned maximumSnapshotRetries = 4;

I don't see an explanation for this change.

> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:1609
> +    m_pluginElement->setDisplayState(HTMLPlugInElement::DisplayingSnapshot);

When does the tear-down happen now?

[Dean Jackson]

Comment on attachment 189570 [details]
Patch

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

>> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:74
>> +static const unsigned maximumSnapshotRetries = 4;
> 
> I don't see an explanation for this change.

I will add this to the changelog:

I also reduced the number of snapshot attempts we will make before giving up. We don't want to sit around for 66 seconds displaying nothing.

>> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:1609
>> +    m_pluginElement->setDisplayState(HTMLPlugInElement::DisplayingSnapshot);
> 
> When does the tear-down happen now?

Oh, that happens anyway, because as soon as I set the displayState it will swap renderers. It was the existing renderer that had the Widget, and the reference to the PluginView, and so we tear things down in the destructor.

[Dean Jackson]

Committed r143650: <http://trac.webkit.org/changeset/143650>