In WebCore/platform/graphics/cg/GraphicsContextCG.cpp, GraphicsContext::setURLForRect takes a KURL argument and converts it to a CFURLRef, to pass to CGPDFContextSetURLForRect. This can only work when the KURL-to-CFURLRef conversion is available in KURL. In Chromium, we're using a different implementation of KURL, based on GoogleURL, and ours doesn't do CFURLRef conversions. We run into a problem, then, when GraphicsContext::setURLForRect tries to do this conversion. All implementations of GraphicsContext other than the CG one just do notImplemented() in setURLForRect. The attached patch makes GraphicsContextCG sensitive to whether the GoogleURL library is in use based on USE(GOOGLEURL), which was proposed in bug 21807. Because KURL support for CFURLRef conversions is also dependent on PLATFORM(CF), I've included that for completeness. If PLATFORM(CF) is not defined or USE(GOOGLEURL) is, GraphicsContextCG::setURLForRect is a simple notImplemented(), as it currently is on other platforms.
Created attachment 24623 [details] Patch
Comment on attachment 24623 [details] Patch Having a dependency on USE(GOOGLEURL) in GraphicsContext feels deeply wrong.
Having a dependency on URLs in general in a GraphicsContext feels pretty wrong...
Comment on attachment 24623 [details] Patch A better way to handle this is would be to put the notImplemented() call inside the GOOGLEURL version of CFURLRef. There's no good reason that KURL when PLATFORM(CF) is true shouldn't implement conversion to a CFURL, regardless of whether it's based on the Google URL library; putting the workaround for that in the GraphicsContext file seems wrong. > +#if PLATFORM(CF) && !USE(GOOGLEURL) The !USE(GOOGLEURL) part is fine, but the PLATFORM(CF) check here is unnecessary and should be removed. CoreGraphics is built on top of CoreFoundation. You can't compile GraphicsContextCG.cpp if PLATFORM(CF) is false. I'm going to say review- because I think this check is in the wrong file, and even if you disagree and think this is the right file. it's not done exactly right.
Created attachment 24625 [details] Alternate patch Takes out PLATFORM(CF) check as suggested.
Comment on attachment 24625 [details] Alternate patch Please fix this in KURL.h as I requested.
(In reply to comment #4) > (From update of attachment 24623 [details] [edit]) > A better way to handle this is would be to put the notImplemented() call inside > the GOOGLEURL version of CFURLRef. I meant the GOOGLEURL version of KURL::createCFURL().
(In reply to comment #0) > In Chromium, we're using a different implementation of KURL, based on > GoogleURL, and ours doesn't do CFURLRef conversions. That seems wrong. There's no reason to not implement conversions from a GoogleURL object to the underlying platform's URL object, CFURL. It's analogous to saying "we have our own string class and have chosen to not implement conversion to CFString".
Retitling based on my understanding of what the bug is.
I'll just fix this directly in GURL then.