[webkit-reviews] review denied: [Bug 16351] FontFallbackList.h doesn't include wtf/PassRefPtr.h : [Attachment 17791] Patch to include PassRefPtr.h in FontFallbackList.h

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Dec 9 06:57:52 PST 2007


Darin Adler <darin at apple.com> has denied Robert Norris <rob at cataclysm.cx>'s
request for review:
Bug 16351: FontFallbackList.h doesn't include wtf/PassRefPtr.h
http://bugs.webkit.org/show_bug.cgi?id=16351

Attachment 17791: Patch to include PassRefPtr.h in FontFallbackList.h
http://bugs.webkit.org/attachment.cgi?id=17791&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
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.


More information about the webkit-reviews mailing list