[Webkit-unassigned] [Bug 57068] [GTK] [WebKit2] Implement a basic WebKitTestRunner

[bugzilla-daemon at webkit.org]

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


Adam Roben (:aroben) <aroben at apple.com> changed:

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




--- Comment #20 from Adam Roben (:aroben) <aroben at apple.com>  2011-04-26 16:52:04 PST ---
(From update of attachment 90371)
View in context: https://bugs.webkit.org/attachment.cgi?id=90371&action=review

> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.h:61
> -typedef void* PlatformBundle;
> +typedef ::GModule* PlatformBundle;

I'm surprised the :: is needed.

> Source/WebKit2/WebProcess/InjectedBundle/gtk/InjectedBundleGtk.cpp:41
> +    m_platformBundle = g_module_open(fileSystemRepresentation(m_path).data(), G_MODULE_BIND_LOCAL);

Do we need to close this ever?

> Tools/Scripts/old-run-webkit-tests:1710
> +    } elsif (isGtk()) {
> +        push @extraPlatforms, "gtk" if $platform eq "gtk-wk2";
>      }

This doesn't look right. You should modify buildPlatformHierarchy instead, similar to what is done for win-wk2 and mac-wk2.

Do you want to modify readSkippedFiles, too, to fall back to mac-wk2 like win-wk2 and qt-wk2 do?

> Tools/Scripts/run-launcher:73
>      if (isGtk()) {
> -        $launcherPath = catdir($launcherPath, "Programs", "GtkLauncher");
> +        if (isWK2()) {
> +            $launcherPath = catdir($launcherPath, "Programs", "MiniBrowser");
> +        } else {
> +            $launcherPath = catdir($launcherPath, "Programs", "GtkLauncher");
> +        }
>      }

This seems somewhat orthogonal.

> Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp:43
> +{
> +
> +    GtkSettings* settings = gtk_settings_get_default();

Extra newline in here.

> Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:80
> +    const char* bundlePath = g_getenv("TEST_RUNNER_INJECTED_BUNDLE_FILENAME");
> +    if (!bundlePath) {
> +        fprintf(stderr, "TEST_RUNNER_INJECTED_BUNDLE_FILENAME environment variable not found\n");
> +        exit(0);
> +    }
> +
> +    gsize bytesWritten;
> +    GOwnPtr<char> utf8BundlePath(g_filename_to_utf8(bundlePath, -1, 0, &bytesWritten, 0));
> +    m_injectedBundlePath = WKStringCreateWithUTF8CString(utf8BundlePath.get());

Is it not possible to find the injected bundle using a path relative to the test runner?

I think you're leaking m_injectedBundlePath.

> Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:86
> +    // This is called after initializeInjectedBundlePath.
> +    m_testPluginDirectory = m_injectedBundlePath;

You could assert that m_injectedBundlePath is initialized.

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