[webkit-reviews] review denied: [Bug 25485] Implement visitedURL to Re-unfork Chromium. : [Attachment 29940] Only use visitedURL in QT builds, v1.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 25485: Implement visitedURL to Re-unfork Chromium.
https://bugs.webkit.org/show_bug.cgi?id=25485

Attachment 29940: Only use visitedURL in QT builds, v1.
https://bugs.webkit.org/attachment.cgi?id=29940&action=review

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