Once a timer fires after the plugin initializes, make a snapshot of the plugin offscreen to display as a placeholder.
<rdar://problem/12426658>
Created attachment 167616 [details] Patch
Comment on attachment 167616 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167616&action=review > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:271 > + , m_pluginSnapshotTimer(this, &PluginView::pluginSnapshotTimerFired, 3) Should this be a named constant? > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:1381 > +{ Do you want to do something like: ASSERT_UNUSED(timer, timer == m_pluginSnapshotTimer?
Comment on attachment 167616 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167616&action=review >> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:271 >> + , m_pluginSnapshotTimer(this, &PluginView::pluginSnapshotTimerFired, 3) > > Should this be a named constant? Done. >> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:1381 >> +{ > > Do you want to do something like: ASSERT_UNUSED(timer, timer == m_pluginSnapshotTimer? Done.
Created attachment 167782 [details] Patch
Comment on attachment 167782 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167782&action=review > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:292 > + // Null out the plug-in element explicitly so we'll crash earlier if we try to use > + // the plug-in view after it's been destroyed. > + m_pluginElement = nullptr; There doesn't seem to be much point in doing this in the destructor. > Source/WebKit2/WebProcess/Plugins/PluginView.h:235 > + // The snapshot is used in two scenarios: > + // 1) If plugin snapshotting is enabled, this snapshot represents the poster image. > + // 2) Otherwise, it represents the first image displayed when the plugin is run, to avoid > + // side effects should the plugin perform DOM manipulation on initialization. > RefPtr<ShareableBitmap> m_snapshot; > + RefPtr<WebCore::Image> m_snapshotImage; This comment is ambiguous now; is it referring to m_snapshot, or m_snapshotImage. Do we still need both of these? Can they be renamed to make their purposes more obvious?
(In reply to comment #6) > (From update of attachment 167782 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167782&action=review > > > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:292 > > + // Null out the plug-in element explicitly so we'll crash earlier if we try to use > > + // the plug-in view after it's been destroyed. > > + m_pluginElement = nullptr; > > There doesn't seem to be much point in doing this in the destructor. This already existed, and seems to have been added as a sanity check, so I will leave this in. > > > Source/WebKit2/WebProcess/Plugins/PluginView.h:235 > > + // The snapshot is used in two scenarios: > > + // 1) If plugin snapshotting is enabled, this snapshot represents the poster image. > > + // 2) Otherwise, it represents the first image displayed when the plugin is run, to avoid > > + // side effects should the plugin perform DOM manipulation on initialization. > > RefPtr<ShareableBitmap> m_snapshot; > > + RefPtr<WebCore::Image> m_snapshotImage; > > This comment is ambiguous now; is it referring to m_snapshot, or m_snapshotImage. Do we still need both of these? Can they be renamed to make their purposes more obvious? Yes, I believe I can get rid of the image variable and make this all happen locally. For readability, this means that instead of giving an Image* to WebCore, I will be using PassRefPtr<Image> instead. Unfortunately, there is still a naming issue, because there is m_snapshot, and m_pluginSnapshotTimer, which at first glance would appear related. I am appending comments to explain that they are unrelated, and will rename m_snapshot to m_transientPaintingSnapshot. New patch forthcoming.
Created attachment 167813 [details] Patch Rename m_snapshot to m_transientPaintingSnapshot, refactor pluginSnapshotTimerFired to use local variables.
Committed r130810: <http://trac.webkit.org/changeset/130810>