Bug 67848

Summary:

[chromium] Workaround for String/HashSet/StringHash #include order glitch

Product:

WebKit

Reporter:

Iain Merrick <husky>

Component:

New Bugs

Assignee:

Iain Merrick <husky>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

husky, jamesr, kbr, nduca

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	kbr: review+

Description Iain Merrick 2011-09-09 07:50:41 PDT

[chromium] Workaround for String/HashSet/StringHash #include order glitch

Comment 1 Iain Merrick 2011-09-09 07:55:42 PDT

Created attachment 106869 [details]
Patch

Comment 2 Iain Merrick 2011-09-09 07:56:55 PDT

Kind of a long explanation for a one-line change, but it's bit of a head-scratcher.

Comment 3 Nat Duca 2011-09-09 08:13:05 PDT

+kbr who knows this file pretty well.

Comment 4 Iain Merrick 2011-09-09 08:39:42 PDT

Simple version: nobody actually includes StringHash.h! So this doesn't compile:

#include <wtf/HashSet.h>
#include <wtf/text/WTFString.h>

class ScoobyDoo {
  HashSet<String> scrappy;
};

#include <wtf/text/StringHash.h>

I think this *does* mean we were using the wrong hash traits.

The only core header that includes StringHash.h is WTFThreadData.h, which is why it showed up when I had thread-related code and GraphicsContext3D in the same .cpp file.

Comment 5 Kenneth Russell 2011-09-09 14:48:27 PDT

Comment on attachment 106869 [details]
Patch

This seems awfully suspicious because there are some files (like Extensions3DChromium.cpp) which compile just fine and include only their header, GraphicsContext3D.h and GraphicsContext3DPrivate.h -- and GraphicsContext3DPrivate.h already includes GraphicsContext3D.h. Could you please continue to investigate the underlying problem rather than committing this workaround, which isn't necessary for the sources currently in the tree?

Comment 6 James Robinson 2011-09-09 14:50:16 PDT

Comment on attachment 106869 [details]
Patch

If there's a problem with the string headers, fix the string headers.

Comment 7 Iain Merrick 2011-09-12 04:15:54 PDT

Here's the fix for the string headers: https://bugs.webkit.org/show_bug.cgi?id=67851

Comment 8 Kenneth Russell 2011-09-12 11:51:22 PDT

(In reply to comment #7)
> Here's the fix for the string headers: https://bugs.webkit.org/show_bug.cgi?id=67851

Thanks. After looking at the comments on that other bug it seems that there was a deliberate decision to avoid including the hash traits from all files which include WTFString.h. Given that, I think this fix to GraphicsContext3DPrivate.h is the right thing to do. I'll r+ this and you can close the other one as WontFix.

Comment 9 Iain Merrick 2011-09-14 09:28:25 PDT

Obsoleted by https://bugs.webkit.org/show_bug.cgi?id=67851