[Webkit-unassigned] [Bug 57617] [Qt][WK2] Implement PLUGIN_PROCESS

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=57617


Andreas Kling <kling at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89585|review?                     |review-
               Flag|                            |




--- Comment #13 from Andreas Kling <kling at webkit.org>  2011-04-26 16:47:12 PST ---
(From update of attachment 89585)
View in context: https://bugs.webkit.org/attachment.cgi?id=89585&action=review

> Source/WebKit2/Platform/CoreIPC/Attachment.h:53
> -    Attachment(int fileDescriptor, size_t);
> +    Attachment(int fileDescriptor, size_t = 0);

This API look rather strange, it should be split into two constructors.

> Source/WebKit2/Platform/unix/SharedMemoryUnix.cpp:-72
> -    ASSERT(!isNull());
> -

Benjamin: we are in SharedMemoryUnix, so the assert totally make sense.
So Either you should have a new encoder for Arguments (with new asserts depending on the type), or you need to fix the broken assertions.

> Source/WebKit2/PluginProcess/PluginProcess.cpp:48
> +// FIXME: the LOG macro don't work without that. Should be fixed there.
> +using namespace WebCore;

Benjamin: Should you rely add a dependency to WebCore here? You don't want to link to WebCore.

> Source/WebKit2/PluginProcess/PluginProcess.cpp:190
> +            ASSERT_NOT_REACHED();
> +            return;

Benjamin: you have cleaning code to close the socket on the previous call to fcntl, but not on this one.

> Source/WebKit2/PluginProcess/PluginProcess.cpp:200
> -    ASSERT_NOT_REACHED();
> +    notImplemented();

With your change it does not assert in debug. The notImplemented() instead of the comment is a good idea.

> Source/WebKit2/PluginProcess/qt/PluginControllerProxyQt.cpp:48
> -void PluginControllerProxy::platformGeometryDidChange(const IntRect& frameRect, const IntRect&)
> +void PluginControllerProxy::platformGeometryDidChange()

Benjamin: ???

> Source/WebKit2/PluginProcess/qt/PluginProcessMainQt.cpp:36
> +#include <QDebug>

Benjamin: ???

> Source/WebKit2/PluginProcess/qt/PluginProcessMainQt.cpp:38
> +#include <QtGlobal>

Benjamin: ???

> Source/WebKit2/PluginProcess/qt/PluginProcessMainQt.cpp:50
> +    QApplication* app = new QApplication(argc, argv);

Benjamin: why are you creating that on the heap (and leaking it?)?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.