Bug 16351 – FontFallbackList.h doesn't include wtf/PassRefPtr.h

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Robert Norris]

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.

[David Kilzer (:ddkilzer)]

Please post a patch with a ChangeLog entry for review.  Thanks!

http://webkit.org/coding/contributing.html

[Robert Norris]

Created attachment 17791 [details]
Patch to include PassRefPtr.h in FontFallbackList.h

[Darin Adler]

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]

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]

Created attachment 17805 [details]
patch

[Robert Norris]

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]

Committed revision 28721.