[webkit-reviews] review granted: [Bug 44069] [GTK] Clean up WebCore/platform/graphics/gtk/ImageGtk.cpp : [Attachment 64530] Patch with build fixes

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 44069: [GTK] Clean up WebCore/platform/graphics/gtk/ImageGtk.cpp
https://bugs.webkit.org/show_bug.cgi?id=44069

Attachment 64530: Patch with build fixes
https://bugs.webkit.org/attachment.cgi?id=64530&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
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/InspectorClientG
tk.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!