RESOLVED FIXED 16351
FontFallbackList.h doesn't include wtf/PassRefPtr.h
https://bugs.webkit.org/show_bug.cgi?id=16351
Summary FontFallbackList.h doesn't include wtf/PassRefPtr.h
Robert Norris
Reported 2007-12-07 20:01:35 PST
WebCore/platform/graphics/FontFallbackList.h uses PassRefPtr but doesn't include wtf/PassRefPtr.h. This causes the following build error on my port: In file included from ../../../WebCore/platform/graphics/FontFallbackList.cpp:30: ../../../WebCore/platform/graphics/FontFallbackList.h:47: error: 'PassRefPtr' has not been declared ../../../WebCore/platform/graphics/FontFallbackList.h:47: error: expected ',' or '...' before '<' token ../../../WebCore/platform/graphics/FontFallbackList.cpp:45: error: prototype for 'void WebCore::FontFallbackList::invalidate(WTF::PassRefPtr<WebCore::FontSelector>)' does not match any in class 'WebCore::FontFallbackList' ../../../WebCore/platform/graphics/FontFallbackList.h:47: error: candidate is: void WebCore::FontFallbackList::invalidate(int) make[1]: *** [tmp/FontFallbackList.o] Error 1 Adding the include resolves this. On my port nearly all my platform files are currently no-op stubs, so I guess on other ports PassRefPtr.h is being brought in elsewhere. This at least occurs in r28509, but it doesn't seem to have been changed yet.
Attachments
Patch to include PassRefPtr.h in FontFallbackList.h (925 bytes, patch)
2007-12-08 14:55 PST, Robert Norris
darin: review-
patch (5.12 KB, patch)
2007-12-09 08:42 PST, Darin Adler
sam: review+
David Kilzer (:ddkilzer)
Comment 1 2007-12-08 14:18:18 PST
Please post a patch with a ChangeLog entry for review. Thanks! http://webkit.org/coding/contributing.html
Robert Norris
Comment 2 2007-12-08 14:55:04 PST
Created attachment 17791 [details] Patch to include PassRefPtr.h in FontFallbackList.h
Darin Adler
Comment 3 2007-12-09 06:57:52 PST
Comment on attachment 17791 [details] Patch to include PassRefPtr.h in FontFallbackList.h This change is wrong. When using PassRefPtr in a header, you don't need to include <wtf/PassRefPtr.h>. Instead you can include <wtf/Forward.h>. But also, we have a test that FontFallbackList.h already includes everything it needs to be compiled on its own. That's the fact that FontFallbackList.cpp includes it first. If there was any missing dependency, that would catch it because it would fail to compile. The way it gets Forward.h included is that it includes FontData.h, which includes FontPlatformData.h, which includes StringImpl.h, which includes Forward.h. If there was any other header needed for PassRefPtr, it would need to be included by StringImpl.h. In fact, there's no need for FontFallbackList.h to include RefCounted.h or Vector.h -- they are both included by StringImpl.h, so the correct patch would be to remove both of those wtf includes.
Darin Adler
Comment 4 2007-12-09 07:02:12 PST
Oh, I overlooked the fact that the FontPlatformData.h file in question was the Mac one. I suppose there's no guarantee that StringImpl.h will be included. So given that, it's fine to include <wtf/Forward.h> in FontFallbackList.h. But I would also think we would need to include RefPtr.h -- was PassRefPtr,h alone really enough in your testing?
Darin Adler
Comment 5 2007-12-09 08:42:36 PST
Robert Norris
Comment 6 2007-12-09 16:03:54 PST
RefPtr.h was being included by GlyphPageTreeNode.h, like so: GlyphPageTreeNode.h wtf/HashMap.h wtf/HashTable.h wtf/HashTraits.h wtf/HashFunctions.h wtf/RefPtr.h This is no longer a problem. At the time I had an empty FontPlatformData.h which was making this break. Now I don't (I include StringImpl.h and other things) and it doesn't happen. I filed the bug because I thought it odd that things missing from my own files would cause other files to break. If you don't think thats a problem then there is no bug on my end :) Patch looks good.
Darin Adler
Comment 7 2007-12-14 12:57:26 PST
Committed revision 28721.
Note You need to log in before you can comment on or make changes to this bug.