[Webkit-unassigned] [Bug 22039] Update LayoutTests/platforms/gtk/Skipped - get the LayoutTests going

[bugzilla-daemon at webkit.org]

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





------- Comment #24 from zecke at selfish.org  2008-11-23 11:40 PDT -------
(From update of attachment 25398)

> +void FrameLoaderClient::addExtraPluginDirectory(const String& directory)
> +{
> +    PluginDatabase::installedPlugins()->addExtraPluginDirectory(directory);
> +}
> +

why do you add this level of abstraction? Calling PluginDatabase directly
from webkitwebview is not doing any harm.



> +void webkit_web_view_add_extra_plugin_directory(WebKitWebView* webView, const gchar* directory)
> +{
> +    g_return_if_fail(WEBKIT_IS_WEB_VIEW(webView));
> +
> +    Frame* coreFrame = core(webkit_web_view_get_main_frame(webView));
> +    WebKit::FrameLoaderClient* client = static_cast<WebKit::FrameLoaderClient*>(coreFrame->loader()->client());
> +
> +    client->addExtraPluginDirectory(String(directory));

PluginDatabase::installedPlugins()->addExtraPluginDirectory(filenameToString(directory));

the constructor you pick will convert from ASCII to the string which is almost
never correct.


> Index: WebKit/gtk/webkit/webkitwebview.h
> ===================================================================
> --- WebKit/gtk/webkit/webkitwebview.h	(revision 38694)
> +++ WebKit/gtk/webkit/webkitwebview.h	(working copy)
> @@ -277,6 +277,10 @@ WEBKIT_API void
>  webkit_web_view_set_full_content_zoom           (WebKitWebView        *web_view,
>                                                   gboolean              full_content_zoom);
>  
> +WEBKIT_API void
> +webkit_web_view_add_extra_plugin_directory      (WebKitWebView        *web_view,
> +                                                 const gchar          *directory);
> +

Could you move that to webkitprivate.h? I think the API should be added to
WebKitWebSettings together with being able to completely control the paths.
As this will require discussion let us start with the semi public interface.



> ===================================================================
> --- WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp	(revision 38694)
> +++ WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp	(working copy)
> @@ -23,12 +23,18 @@
>   * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> +#include "config.h"

^^^ unlikely that we want that here. The plugin should not know anything about
WebCore/JSC so including config.h is certainly wrong.


> +#if PLATFORM(GTK)
> +#include <stdlib.h>
> +#include <string.h>
> +#endif
> +

we can include that unconditionally.


> +#if PLATFORM(GTK)
> +#include "npfunctions.h"
> +#else
>  #include <WebKit/npfunctions.h>
> +#endif

Two options:
   a) As you proposed on IRC add a Forwarding Header to the
DumpRenderTree/gtk/TestNetscapePlugin...
   b) change the CPP flags to define something and use that for the condition.
config.h may not be included.


> +#include "config.h"
>  #include "TestObject.h"
> +
>  #include "PluginObject.h"
>  
> +#if PLATFORM(GTK)
> +#include <stdlib.h>
> +#include <string.h>
> +#endif
> +

same comments.


> +#if PLATFORM(GTK)
> +#include "npapi.h"
> +#include "npruntime.h"
> +#else
>  #include <WebKit/npapi.h>
>  #include <WebKit/npruntime.h>
> +#endif

same options.

> +
> +    webkit_web_view_add_extra_plugin_directory(webView, "/tmp/webkit-test-plugin");

Can't we point to the build directory? Add that to the CPPFLAGS and use that
here?

>  
> +# On GTK port, copy libtestnetscapeplugin.so into temporary directory
> +if (isGtk()) {
> +    my $testPluginDir = catdir(productDir(), "TestNetscapePlugin/.libs");
> +    my $testPluginFile = "$testPluginDir/libtestnetscapeplugin.so";
> +
> +    my $tmpPluginDir = "/tmp/webkit-test-plugin";
> +    my $tmpPluginFile = "$tmpPluginDir/libtestnetscapeplugin.so";
> +    mkdir($tmpPluginDir, 0755);
> +
> +    copy($testPluginFile, $tmpPluginFile);
> +    chmod(0755, $tmpPluginFile);
> +}

with the above I think it is avoidable. Alternatively introduce an environment
var for DRT to read and set the plugin dir.


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