Nested <use>s can resolve against an external resource

Nested <use>s can resolve against an external resource

Rather than using TargetElementFromIRIString(...) when resolving nested
<use> references, we should be using ResolveTargetElement(), since the
latter also considers a possible external resource.

BUG=721196
Review-Url: https://codereview.chromium.org/2875303002
Cr-Commit-Position: refs/heads/master@{#471632}
Committed: https://chromium.googlesource.com/chromium/src/+/a0de45f1fc217b5e308e5d58ef1ed43064f40ac4

[fs, 2017-05-12 12:54:18]

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

[commit-bot: I haz the power, 2017-05-12 12:54:36]

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, 2017-05-12 13:48:59]

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

[commit-bot: I haz the power, 2017-05-12 13:49:00]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[fs, 2017-05-12 14:15:18]

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

[commit-bot: I haz the power, 2017-05-12 14:15:36]

Dry run: CQ is trying da patch.

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

[fs, 2017-05-12 14:57:35]

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

[commit-bot: I haz the power, 2017-05-12 17:43:15]

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

[commit-bot: I haz the power, 2017-05-12 17:43:16]

Dry run: This issue passed the CQ dry run.

[pdr., 2017-05-13 03:10:51]

https://codereview.chromium.org/2875303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right):

https://codereview.chromium.org/2875303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGUseElement.cpp:562: Element*
target_element = use.ResolveTargetElement(kDontAddObserver);
Can you help me understand why we don't want to add an observer? Isn't an
observer used to catch changes in the used element? If so, would we fail to get
updates if #rect-external changes?

[fs, 2017-05-13 08:24:07]

https://codereview.chromium.org/2875303002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right):

https://codereview.chromium.org/2875303002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGUseElement.cpp:562: Element*
target_element = use.ResolveTargetElement(kDontAddObserver);
On 2017/05/13 at 03:10:50, pdr. wrote:
> Can you help me understand why we don't want to add an observer? Isn't an
observer used to catch changes in the used element? If so, would we fail to get
updates if #rect-external changes?

We don't want to add an observer, because the element itself (also a <use>) will
have done that already (or will) when it receives its "primary" (non-nested)
expansion (BuildPendingResource gets called on it.)

[pdr., 2017-05-13 23:09:39]

On 2017/05/13 at 08:24:07, fs wrote:
>
https://codereview.chromium.org/2875303002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right):
> 
>
https://codereview.chromium.org/2875303002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/svg/SVGUseElement.cpp:562: Element*
target_element = use.ResolveTargetElement(kDontAddObserver);
> On 2017/05/13 at 03:10:50, pdr. wrote:
> > Can you help me understand why we don't want to add an observer? Isn't an
observer used to catch changes in the used element? If so, would we fail to get
updates if #rect-external changes?
> 
> We don't want to add an observer, because the element itself (also a <use>)
will have done that already (or will) when it receives its "primary"
(non-nested) expansion (BuildPendingResource gets called on it.)

Ah, LGTM

[fs, 2017-05-14 08:58:53]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2017-05-14 08:59:02]

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, 2017-05-14 16:39:18]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494752333724820,
"parent_rev": "1363aa20f15439186c0650ebe9d7e9f271f0269b", "commit_rev":
"a0de45f1fc217b5e308e5d58ef1ed43064f40ac4"}

[commit-bot: I haz the power, 2017-05-14 16:39:29]

Description was changed from

==========
Nested <use>s can resolve against an external resource

Rather than using TargetElementFromIRIString(...) when resolving nested
<use> references, we should be using ResolveTargetElement(), since the
latter also considers a possible external resource.

BUG=721196
==========

to

==========
Nested <use>s can resolve against an external resource

Rather than using TargetElementFromIRIString(...) when resolving nested
<use> references, we should be using ResolveTargetElement(), since the
latter also considers a possible external resource.

BUG=721196

Review-Url: https://codereview.chromium.org/2875303002
Cr-Commit-Position: refs/heads/master@{#471632}
Committed:
https://chromium.googlesource.com/chromium/src/+/a0de45f1fc217b5e308e5d58ef1e...
==========

[commit-bot: I haz the power, 2017-05-14 16:39:30]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/a0de45f1fc217b5e308e5d58ef1e...