Bug 132814 – [Stable] [GTK] GdiObject.h missing in WebKitGTK 2.4.1 tarball

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Milan Crha]

Revision r155454 introduces GdiObject.h (see bug #120778), but this is missing in the sources of tarball for 2.4.1. There are still references to this file, the same as used functions (adoptGDIObject) in the sources. I reverted those references locally, but it would be good to have this fixed in the future releases.

[Alberto Garcia]

I can see that the following files are missing from the tarball.

Source/WebCore/platform/win/GDIObjectCounter.h
Source/WebCore/platform/win/GDIObjectCounter.cpp
Source/WebCore/platform/graphics/win/SharedGDIObject.h
Source/WTF/wtf/win/GDIObject.h

Does it build fine if you add them manually?

[Milan Crha]

I have more other changes here, which I'm trying to fine-tune before offering to upstream. I'm building with msys/mingw32.

I can try to add the file into sources, but I do not think it's enough to just get them from anywhere and hope it'll work, I expect it needs also some Makefile change thus the build system knows about those files (at least for the .cpp file). My question then is:

- where do I get those files from, please? Exact link welcome.
- which Makefile should I change and in what way?

[Alberto Garcia]

(In reply to comment #2)
> - where do I get those files from, please? Exact link welcome.

You can browse the repository here:

   http://trac.webkit.org/browser/releases/WebKitGTK

Or you can also clone this git repository which contains the
webkit-2.4 branch with all the stable releases.

   https://github.com/bertogg/webkit

Here are the direct links to the files I mentioned earlier:

   https://raw.githubusercontent.com/bertogg/webkit/webkit-2.4/Source/WebCore/platform/win/GDIObjectCounter.h
   https://raw.githubusercontent.com/bertogg/webkit/webkit-2.4/Source/WebCore/platform/win/GDIObjectCounter.cpp
   https://raw.githubusercontent.com/bertogg/webkit/webkit-2.4/Source/WebCore/platform/graphics/win/SharedGDIObject.h
   https://raw.githubusercontent.com/bertogg/webkit/webkit-2.4/Source/WTF/wtf/win/GDIObject.h

> - which Makefile should I change and in what way?

You probably need to add those files to this Makefile:

   Source/WebCore/GNUmakefile.list.am

See the TARGET_WIN32 section. Run autogen.sh afterwards so the new
Makefiles are generated with your changes.

[Milan Crha]

Created attachment 231626 [details]
my changes to webkit

Thanks for the pointer. i took the files from tracker's trunk branch. The path Source/WTF/wtf/win/ doesn't exist in tarball, thus I created it and added there the file. there had been some issues with GdiObject itself, mingw/msys has a clash with deleteObject, thus I renamed it to deleteObjectGDI and there was also a problem with specialization of that template,, thus the later change in the GdiObject.h. Other related changes are attached. I hope it helps.

[WebKit Commit Bot]

Attachment 231626 [details] did not pass style-queue:


Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alberto Garcia]

(In reply to comment #4)
> there had been some issues with GdiObject itself, mingw/msys has a
> clash with deleteObject, thus I renamed it to deleteObjectGDI

Doesn't the WTF namespace prevent this problem from happening?

[Milan Crha]

(In reply to comment #6)
> (In reply to comment #4)
> > there had been some issues with GdiObject itself, mingw/msys has a
> > clash with deleteObject, thus I renamed it to deleteObjectGDI
> 
> Doesn't the WTF namespace prevent this problem from happening?

I did not investigate it into deep, thus I do not know from where the clash comes, but if it's a define, then the namespace wouldn't help.

The thing is, without rename I get a compiler error (I can regenerate it, if it helps), then I searched the Internet and one way to fix it was to rename the symbol, which I did and it helped.

[Alberto Garcia]

(In reply to comment #7)
> > > there had been some issues with GdiObject itself, mingw/msys has a
> > > clash with deleteObject, thus I renamed it to deleteObjectGDI
> >
> > Doesn't the WTF namespace prevent this problem from happening?

> The thing is, without rename I get a compiler error (I can
> regenerate it, if it helps), then I searched the Internet and one
> way to fix it was to rename the symbol, which I did and it helped.

It would be great if we can investigate this a bit more. If you can
regenerate the error and/or attach the relevant headers that produce
this clash then we can try to help.

Thanks!

[Milan Crha]

(In reply to comment #8)
> It would be great if we can investigate this a bit more. If you can
> regenerate the error and/or attach the relevant headers that produce
> this clash then we can try to help.

Weird, I reverted my changes in GdiObject.h (the rename from deleteObject to deleteObjectGDI) and the compile finished without stopping on this as before. I also didn't fine the 'deleteObject' defined anywhere in mingw32 headers (and even binaries), thus it might be some other kind of a clash. I changed one thing, I configured webkitgtk with --enable-jit this time.

In any case, I propose to not use my rename in the GdiObject.h, just feel free to use the other parts of the patch.

[Alberto Garcia]

Created attachment 232033 [details]
Patch

(In reply to comment #9)
> In any case, I propose to not use my rename in the GdiObject.h, just
> feel free to use the other parts of the patch.

Here's the updated patch. I made a couple of changes:

1) I renamed GdiObject.h to GDIObject.h in the patch. That's the
   correct name of the file.

2) I removed the if TARGET_WIN32 part around it, this is a header file
   so it's not really necessary.

We should also check if the master branch is affected by this, although
that would require a separate patch if it was the case.

[Carlos Garcia Campos]

Comment on attachment 232033 [details]
Patch

Thanks!

[Carlos Garcia Campos]

Pushed to the stable branch as <http://trac.webkit.org/changeset/169331>

[Milan Crha]

(In reply to comment #10)
> 1) I renamed GdiObject.h to GDIObject.h in the patch. That's the
>    correct name of the file.

Ahh, unnoticed by me, due to Windows being case insensitive in file names. I suppose, for a sake of consistency, the #include directives around the source code should be also changed to capitals on GDI.

> 2) I removed the if TARGET_WIN32 part around it, this is a header file
>    so it's not really necessary.

Are you sure? You definitely do not want to compile windows-specific headers on Linux (or basically any other than Windows system), the same as you do not compile posix related code on windows. I would keep there the check for TARGET_WIN32, the same as it is in Source/WebCore/GNUmakefile.list.am.

[Alberto Garcia]

(In reply to comment #13)

> > 2) I removed the if TARGET_WIN32 part around it, this is a header
> >    file so it's not really necessary.

> Are you sure? You definitely do not want to compile windows-specific
> headers on Linux

Well, headers are not compiled :)