Bug 44069 – [GTK] Clean up WebCore/platform/graphics/gtk/ImageGtk.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

There are a good number of style violations in ImageGtk.cpp as well as some general badness.

[Martin Robinson]

Created attachment 64516 [details]
Cleanup patch

[WebKit Review Bot]

Attachment 64516 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3756311

[Martin Robinson]

Created attachment 64530 [details]
Patch with build fixes

[Gustavo Noronha (kov)]

Comment on attachment 64530 [details]
Patch with build fixes

WebCore/platform/graphics/gtk/ImageGtk.cpp:58
 +      if (!GetModuleFileName(hmodule, (CHAR *) dataDirectory, sizeof(retval) - 10))
Should this (CHAR *) be turned into a static_cast<CHAR*>(dataDirectory) for greater style guide followage?

WebCore/platform/graphics/gtk/ImageGtk.cpp:141
 +          GOwnPtr<gchar> glibFileName(g_build_filename(getWebKitDataDirectory(), "webkit-1.0", "images", imageName.get(), NULL));
Data directories should follow the "soname" of the library. We have done it in InspectoClientGtk:

http://trac.webkit.org/browser/trunk/WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp#L90

Can you then also please change WebCore/GNUmakefile.am to add 'gtk' after 'webkit' here? You'll notice the inspector one is already correct, but we missed images =(:

http://trac.webkit.org/browser/trunk/WebCore/GNUmakefile.am#L4575

As we discussed on IRC, it would be great to move the getWebKitDataDirectory function to, say, FileSystemGtk, and then use it in InspectorClient as well, so it starts actually working on Windows heh. Let's follow up on that later!

[Martin Robinson]

Committed r65529: <http://trac.webkit.org/changeset/65529>

[WebKit Review Bot]

http://trac.webkit.org/changeset/65529 might have broken GTK Linux 64-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/65529
http://trac.webkit.org/changeset/65530