Change http://trac.webkit.org/changeset/43052 broke Chromium build -- we need to implement visitedURL in HashLinkChromium.cpp.
Gar. This looks messy. I wonder if it'll be okay if I just do: #if PLATFORM(CHROMIUM) LinkHash hash = visitedLinkHash(m_document->baseURL(), *attr); if (!hash) return PseudoLink; #endif Vector<UChar, 512> url;
Or something like that :)
It seems like _only_ the QT port requires the visitedURL call. Surely that is not a free call on other platforms, so maybe only the QT port should make that call.
Created attachment 29940 [details] Only use visitedURL in QT builds, v1. WebCore/ChangeLog | 13 +++++++++++++ WebCore/css/CSSStyleSelector.cpp | 25 +++++++++++++++---------- WebCore/platform/LinkHash.h | 3 ++- 3 files changed, 30 insertions(+), 11 deletions(-)
Ok, here's an ugly fix -- create separate code paths for QT and non-QT builds in checkPseudoState. The good news is that the separation is only on the high level, because visitedLinkHash still uses visitedURL.
Comment on attachment 29940 [details] Only use visitedURL in QT builds, v1. > + (WebCore::CSSStyleSelector::SelectorChecker::checkPseudoState): Moved guards around to > + provide separate code paths for QT and non-QT ports. It’s Qt, not QT. This mistake makes this especially confusing at Apple, because lots of people at Apple use QT to mean QuickTime. > +#if PLATFORM(QT) > // Resolves the potentially relative URL "attributeURL" relative to the given > // base URL, and returns the hash of the string that will be used for visited. > // It will return an empty Vector in case of errors. > void visitedURL(const KURL& base, const AtomicString& attributeURL, Vector<UChar, 512>&); > - > +#endif My general thought here is: 1) It doesn’t do harm to declare a function we plan to never define, so sometimes we just leave out the #if statements in headers for cases like this. 2) I believe adding this #if to the header but not the LinkHash.cpp file will break the Mac build, because we compile with a warning for functions that are defined with external linkage and not previously declared. So you need to add this #if to LinkHash.cpp, not just LinkHash.h. review- because of breaking the Mac build.
The direction here seems fine. It’s good not to burden the non-Qt platforms with Qt’s requirement here.
Created attachment 29941 [details] Only use visitedURL in QT builds, v1. WebCore/ChangeLog | 13 +++++++++++++ WebCore/css/CSSStyleSelector.cpp | 25 +++++++++++++++---------- WebCore/platform/LinkHash.h | 3 ++- 3 files changed, 30 insertions(+), 11 deletions(-)
Oops, sorry -- hair-trigger-finger :) Correct patch coming up.
Created attachment 29942 [details] Only use visitedURL in QT builds, v2. WebCore/ChangeLog | 12 ++++++++++++ WebCore/css/CSSStyleSelector.cpp | 25 +++++++++++++++---------- 2 files changed, 27 insertions(+), 10 deletions(-)
Fixed up -- I removed the guards in LinkHash.h and corrected the Qt whoopsie.
Comment on attachment 29942 [details] Only use visitedURL in QT builds, v2. You should mention restoring the order in your ChangeLog. This looks sane, so long as you know that no other build besides Qt needs the visitedURL call.
Landed as http://trac.webkit.org/changeset/43120.