[Webkit-unassigned] [Bug 91844] [EFL][WK2] Plugin process implementation

[bugzilla-daemon at webkit.org]

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





--- Comment #30 from Christophe Dumez <christophe.dumez at intel.com>  2012-09-14 06:14:31 PST ---
(In reply to comment #29)
> (From update of attachment 163889 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163889&action=review
> 
> > ChangeLog:3
> > +        [EFL][WK2] Plugin process implementation
> 
> New introduced approach makes this title out of data. What about: "[WK2][GTK][EFL] Share WebKit2-GTK plugin process implementation with EFL port" ?
> 
> > Source/WebKit2/ChangeLog:12
> > +        * PlatformEfl.cmake: Added files needed by plugin process.
> 
> Please keep the same tense in the ChangeLog file. Your are using Added, Adds, Add. You can choose the most suitable for you.
> 
> > Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:52
> > +#if PLATFORM(GTK)
> 
> This code seems duplicated for both port. Please consider common code:
> 1) gchar can be dropped here, char type is enough to save that message
> 2) Error message that is passed to g_warning and EINA_LOG_CRIT can be saved somewhere as it almost the same for both ports. If there is no any API to get program name what about using g_get_progname() in EFL with GLIB_SUPPORT macro.
> 3) Depend on port call g_warning(message) or  EINA_LOG_CRIT(message)
> 
> > Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:64
> > +#if PLATFORM(GTK)
> 
> This code might be merged too. Please consider
> 1) gint -> int
> 2) GOwnPtr -> WTF:OwnPtr
> After that we will have shared code.
> 
> > Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:84
> > +#if PLATFORM(GTK)
> 
> Won't be necessary if you apply above suggestion.

I agree with Grzegorz, you should really avoid as much as possible the #if PLATFORM() in the generic *Unix file. While it does not look too ugly now that only 2 ports are using it, it will get a lot worse when other ports start to use it. We should keep this code generic (Posix + WTF). Optimally, there should be no port specific code in there.

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