[webkit-reviews] review granted: [Bug 102150] Pass clicks through to the restarted plugin : [Attachment 175946] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Jon Lee
<jonlee at apple.com>'s request for review:
Bug 102150: Pass clicks through to the restarted plugin
https://bugs.webkit.org/show_bug.cgi?id=102150

Attachment 175946: Patch
https://bugs.webkit.org/attachment.cgi?id=175946&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
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* ?