[webkit-reviews] review denied: [Bug 83360] [BlackBerry] Upstreaming BlackBerry-specific changes to PluginView : [Attachment 136659] Patch

[bugzilla-daemon at webkit.org]

Antonio Gomes <tonikitoo at webkit.org> has denied Charles Wei
<charles.wei at torchmobile.com.cn>'s request for review:
Bug 83360: [BlackBerry] Upstreaming BlackBerry-specific changes to PluginView
https://bugs.webkit.org/show_bug.cgi?id=83360

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

------- Additional Comments from Antonio Gomes <tonikitoo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=136659&action=review


Lets upstream it by parts: first the event handling stuff, then networking,
etc.

> Source/WebCore/ChangeLog:8
> +	   No new testsr, initial checkin of BlackBerry porting of PluginView,

testsr*

> Source/WebCore/plugins/PluginView.cpp:460
> +    // Don't let a plugin start any loads if it no longer has a page
associated with it.
> +    if (!m_parentFrame->page())
> +	   return;
> +

this is hard to accept without a test case of failure or a crash. no tests...

> Source/WebCore/plugins/PluginView.h:88
> +#if (PLATFORM(QT) && USE(ACCELERATED_COMPOSITING) &&
ENABLE(NETSCAPE_PLUGIN_API) && (defined(XP_UNIX) )) || PLATFORM(BLACKBERRY)

I think it should be like:

#if (PLATFORM(QT) || PLATFORM(BLACKBERRY) && USE(ACCELERATED_COMPOSITING) &&
ENABLE(NETSCAPE_PLUGIN_API) && defined(XP_UNIX)