Clicking on the plugin enables snapshotted plugins.
<rdar://problem/12695575>
Created attachment 174845 [details] Patch
Comment on attachment 174845 [details] Patch Attachment 174845 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14888423
Comment on attachment 174845 [details] Patch Attachment 174845 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14888421
Comment on attachment 174845 [details] Patch Attachment 174845 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14876563
Comment on attachment 174845 [details] Patch Attachment 174845 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14890096
Comment on attachment 174845 [details] Patch Attachment 174845 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14868803
Comment on attachment 174845 [details] Patch Attachment 174845 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14875599
Comment on attachment 174845 [details] Patch Attachment 174845 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14873744
Comment on attachment 174845 [details] Patch Attachment 174845 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/14872784
Comment on attachment 174845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174845&action=review > Source/WebCore/html/HTMLPlugInImageElement.cpp:297 > + dispatchSimulatedClick(m_pendingClickEventFromSnapshot.get(), true, true, false); EWS says that’s too many booleans there.
Comment on attachment 174845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174845&action=review >> Source/WebCore/html/HTMLPlugInImageElement.cpp:297 >> + dispatchSimulatedClick(m_pendingClickEventFromSnapshot.get(), true, true, false); > > EWS says that’s too many booleans there. I see now. This patch depends on that other patch.
Created attachment 175943 [details] Patch
Comment on attachment 175943 [details] Patch Attachment 175943 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14988533
Created attachment 175946 [details] Patch
Comment on attachment 175946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175946&action=review > Source/WebCore/html/HTMLPlugInImageElement.cpp:278 > + ASSERT_UNUSED(timer, timer == &m_simulatedMouseClickTimer); We don't normally assert this. Why do you think you need to? > Source/WebCore/html/HTMLPlugInImageElement.cpp:282 > + dispatchSimulatedClick(m_pendingClickEventFromSnapshot.get(), SendMouseOverUpDownEvents, DoNotShowPressedLook); What if this event runs JS that destroys |this|? Have you tested that? > Source/WebKit2/WebProcess/Plugins/PDF/SimplePDFPlugin.mm:928 > + ASSERT_NOT_REACHED(); > + return IntPoint(); Can't you just call the superclass? > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:71 > -static const double pluginSnapshotTimerDelay = 1; > +static const double pluginSnapshotTimerDelay = 1.1; Should you comment here about the dependency with simulatedMouseClickTimerDelay? > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:735 > +PassOwnPtr<WebEvent> PluginView::createWebEvent(MouseEvent* event) const const MouseEvent* ?
Comment on attachment 175946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175946&action=review >> Source/WebCore/html/HTMLPlugInImageElement.cpp:278 >> + ASSERT_UNUSED(timer, timer == &m_simulatedMouseClickTimer); > > We don't normally assert this. Why do you think you need to? It is extraneous. I'll remove it. >> Source/WebCore/html/HTMLPlugInImageElement.cpp:282 >> + dispatchSimulatedClick(m_pendingClickEventFromSnapshot.get(), SendMouseOverUpDownEvents, DoNotShowPressedLook); > > What if this event runs JS that destroys |this|? Have you tested that? I have not tested this case. I've been testing with live sites, and have not run into a case where the plugin destroys itself immediately (since |this| would have to be deleted before the 1.1 second timer fired). >> Source/WebKit2/WebProcess/Plugins/PDF/SimplePDFPlugin.mm:928 >> + return IntPoint(); > > Can't you just call the superclass? Superclass uses pure virtual. I can move this up to WebKit::Plugin as a default impl. >> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:71 >> +static const double pluginSnapshotTimerDelay = 1.1; > > Should you comment here about the dependency with simulatedMouseClickTimerDelay? Done. >> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:735 >> +PassOwnPtr<WebEvent> PluginView::createWebEvent(MouseEvent* event) const > > const MouseEvent* ? No. offsetX() and offsetY() are calculated dynamically and then cached, so the event should not be const.
Committed r135767: <http://trac.webkit.org/changeset/135767>