[webkit-reviews] review granted: [Bug 21427] Implement NSAPI support for WebKit/Mac inside WebCore/plugins : [Attachment 25095] Updated patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Tor Arne Vestbø
<tavestbo at trolltech.com>'s request for review:
Bug 21427: Implement NSAPI support for WebKit/Mac inside WebCore/plugins
https://bugs.webkit.org/show_bug.cgi?id=21427

Attachment 25095: Updated patch
https://bugs.webkit.org/attachment.cgi?id=25095&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
>  public:
>	   PlatformPluginWidget platformPluginWidget() const { return m_window;
}
> +	   void setPlatformPluginWidget(PlatformPluginWidget widget) {
> +		   m_window = widget;
> +	   }

>From the looks of it other code in WebCore puts simple inline setters in one
line of code.

> +    if (!dispatchNPEvent(record)) {
> +	   LOG(Events, "PluginView::setFocus(): Get-focus event not accepted");

> +    }
[...]
> +    if (!m_isStarted) {
> +	   // TODO: Draw the "missing plugin" image
> +	   return;
> +    }
[...]
> +    if (!dispatchNPEvent(event)) {
> +	   LOG(Events, "PluginView::paint(): Paint event not accepted");
> +    }
[...]
> +    if (!dispatchNPEvent(record)) {
> +	   LOG(Events, "PluginView::handleKeyboardEvent(): Keyboard event type
%d not accepted",
> +		   record.what);
> +    } else {
> +	   event->setDefaultHandled();
> +    }
[...]
> +    if (!dispatchNPEvent(record)) {
> +	   LOG(Events, "PluginView::nullEventTimerFired(): Null event not
accepted");
> +    }

All of these one line control clauses should not use braces.

I'm really nitpicking here now :-), so r+ under the condition that you fix them
before landing.