Make fragment-only URLs always be document-local references

Make fragment-only URLs always be document-local references

This implements the 'local url' handling per:

  https://drafts.csswg.org/css-values/#local-urls

which is also referenced from:

  https://svgwg.org/svg2-draft/linking.html#linkRefAttrs

Most of the logic is handled by a new helper class named
SVGURLReferenceResolver, which keeps state, resolves the URL
and extracts the fragment identifier as needed.

BUG=470608
Committed: https://crrev.com/e7d7225c33aa7fc42ee390125b01df9167fad106
Cr-Commit-Position: refs/heads/master@{#407299}

[fs, 2016-07-22 14:49:21]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-22 14:49:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 16:03:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-22 16:03:54]

Dry run: This issue passed the CQ dry run.

[fs, 2016-07-22 16:06:52]

fs@opera.com changed reviewers:
+ pdr@chromium.org

[fs, 2016-07-22 16:06:53]

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp
(right):

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp:128:
return ReferenceClipPathOperation::create(toCSSURIValue(value).value(),
resolver.fragmentIdentifier());
This has a (preexisting) bug (filed as crbug.com/629826), will send a dependent
CL to fix that.

[pdr., 2016-07-22 19:18:28]

Really nice! Just a few minor comments/questions.

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGURIReference.h (right):

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGURIReference.h:78: // flag' per
https://drafts.csswg.org/css-values/#local-urls .
Can you add a test (maybe like
http/tests/misc/input-image-fallback-content-crash.html) where the base href +
fragment is valid but we still choose the local object over the external one?

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGURIReference.h:79: class
SVGURLReferenceResolver {
I'm wondering if this belongs in SVG or somewhere more generic. The spec doesn't
specifically require that this only applies to SVG even though most cases are.
Could HTMLMapElement / HTMLImageElement::isServerMap also use this?

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGURIReference.h:84: bool isLocal() const;
Can you update isExternalURIReference to use this cool new technology? I think
it's doing the same logic.

[pdr., 2016-07-22 20:02:31]

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGURIReference.h (right):

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGURIReference.h:79: class
SVGURLReferenceResolver {
On 2016/07/22 at 19:18:28, pdr. wrote:
> I'm wondering if this belongs in SVG or somewhere more generic. The spec
doesn't specifically require that this only applies to SVG even though most
cases are. Could HTMLMapElement / HTMLImageElement::isServerMap also use this?

Answering my own question, image maps don't use the url syntax. My greps aren't
hitting any non-svg examples in our tests either. I suspect this is fine as
SVGURLReferenceResolver.

[fs, 2016-07-22 21:13:41]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-22 21:14:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2016-07-22 21:15:03]

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGURIReference.h (right):

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGURIReference.h:78: // flag' per
https://drafts.csswg.org/css-values/#local-urls .
On 2016/07/22 at 19:18:28, pdr. wrote:
> Can you add a test (maybe like
http/tests/misc/input-image-fallback-content-crash.html) where the base href +
fragment is valid but we still choose the local object over the external one?

Done.

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGURIReference.h:79: class
SVGURLReferenceResolver {
On 2016/07/22 at 19:18:28, pdr. wrote:
> I'm wondering if this belongs in SVG or somewhere more generic. The spec
doesn't specifically require that this only applies to SVG even though most
cases are. Could HTMLMapElement / HTMLImageElement::isServerMap also use this?

The spec is for CSS values, and subsequently referenced by SVG. I've made
provisions for adding a mechanism for this to CSSURIValue, with roughly the same
ergonomics. In the end I decided that it didn't fit very well together with this
CL. I've moved it to another CL (one about resolving against CSS stylesheets
when needed.) 

Usemap is specced by HTML [1][2], so we should probably not touch them unless
otherwise indicated.

[1]
https://html.spec.whatwg.org/multipage/infrastructure.html#rules-for-parsing-...
[2]
https://html.spec.whatwg.org/multipage/embedded-content.html#attr-hyperlink-u...

https://codereview.chromium.org/2174833002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGURIReference.h:84: bool isLocal() const;
On 2016/07/22 at 19:18:28, pdr. wrote:
> Can you update isExternalURIReference to use this cool new technology? I think
it's doing the same logic.

Yes, that's a good point. isExternalURIReference only had one user
(SVGUseElement), and that one could trivially be replaced with a simpler check,
so I dropped the helper method.

[pdr., 2016-07-22 21:53:11]

LGTM!

[fs, 2016-07-22 21:55:38]

Description was changed from

==========
Make fragment-only URLs always be document-local references

This implements the 'local url' handling per:

  https://drafts.csswg.org/css-values/#local-urls

which is also referenced from:

  https://svgwg.org/svg2-draft/linking.html#linkRefAttrs

Most of the logic is handled by a new helper class named
SVGURLReferenceResolver, which keeps state and resolves URL
and extracts the fragment identifier as needed.

BUG=470608
==========

to

==========
Make fragment-only URLs always be document-local references

This implements the 'local url' handling per:

  https://drafts.csswg.org/css-values/#local-urls

which is also referenced from:

  https://svgwg.org/svg2-draft/linking.html#linkRefAttrs

Most of the logic is handled by a new helper class named
SVGURLReferenceResolver, which keeps state, resolves the URL
and extracts the fragment identifier as needed.

BUG=470608
==========

[fs, 2016-07-22 22:57:22]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-07-22 22:57:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 23:33:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-22 23:35:38]

Description was changed from

==========
Make fragment-only URLs always be document-local references

This implements the 'local url' handling per:

  https://drafts.csswg.org/css-values/#local-urls

which is also referenced from:

  https://svgwg.org/svg2-draft/linking.html#linkRefAttrs

Most of the logic is handled by a new helper class named
SVGURLReferenceResolver, which keeps state, resolves the URL
and extracts the fragment identifier as needed.

BUG=470608
==========

to

==========
Make fragment-only URLs always be document-local references

This implements the 'local url' handling per:

  https://drafts.csswg.org/css-values/#local-urls

which is also referenced from:

  https://svgwg.org/svg2-draft/linking.html#linkRefAttrs

Most of the logic is handled by a new helper class named
SVGURLReferenceResolver, which keeps state, resolves the URL
and extracts the fragment identifier as needed.

BUG=470608

Committed: https://crrev.com/e7d7225c33aa7fc42ee390125b01df9167fad106
Cr-Commit-Position: refs/heads/master@{#407299}
==========

[commit-bot: I haz the power, 2016-07-22 23:35:39]

Patchset 2 (id:??) landed as
https://crrev.com/e7d7225c33aa7fc42ee390125b01df9167fad106
Cr-Commit-Position: refs/heads/master@{#407299}