Make TreeScope::baseURL pure virtual

Make TreeScope::baseURL pure virtual

This allows completeURL and baseURL work correctly when called on a ShadowRoot.
They should fall back to the ShadowRoot's Document's baseURL unless the ShadowRoot
itself has a baseURL specified.

Since no code in Blink yet calls either on a ShadowRoot, there's no
test I can write that triggers this, but once we start calling
treeScope().completeURL() this will be well-tested.

R=esprehn@chromium.org
BUG=311682
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165025

[adamk, 2014-01-14 00:19:33]

+abarth

[esprehn, 2014-01-14 00:22:12]

LGTM, but why isn't this handled inside the getter for baseURL()?

https://codereview.chromium.org/135643004/diff/1/Source/core/dom/TreeScope.cpp
File Source/core/dom/TreeScope.cpp (right):

https://codereview.chromium.org/135643004/diff/1/Source/core/dom/TreeScope.cp...
Source/core/dom/TreeScope.cpp:500: return
document().completeURLWithOverride(url, m_baseURL.isEmpty() ?
document().baseURL() : m_baseURL);
Hmm, shouldn't the baseURL getter on ShadowRoot do this for you?

shadowRoot.baseURI from JS should return the Document's URL I think if you
haven't assigned anything yet?

[adamk, 2014-01-14 00:34:03]

https://codereview.chromium.org/135643004/diff/1/Source/core/dom/TreeScope.cpp
File Source/core/dom/TreeScope.cpp (right):

https://codereview.chromium.org/135643004/diff/1/Source/core/dom/TreeScope.cp...
Source/core/dom/TreeScope.cpp:500: return
document().completeURLWithOverride(url, m_baseURL.isEmpty() ?
document().baseURL() : m_baseURL);
On 2014/01/14 00:22:12, esprehn wrote:
> Hmm, shouldn't the baseURL getter on ShadowRoot do this for you?
>
> shadowRoot.baseURI from JS should return the Document's URL I think if you
> haven't assigned anything yet?

Very good points. I've added the logic to baseURL as suggested, and will update
the CL description. Please take another look, as I'm not sure you'll like the
way I'm avoiding infinite recursion.

[esprehn, 2014-01-14 00:36:36]

Couldn't you just make this virtual too?

https://codereview.chromium.org/135643004/diff/70001/Source/core/dom/TreeScop...
File Source/core/dom/TreeScope.cpp (right):

https://codereview.chromium.org/135643004/diff/70001/Source/core/dom/TreeScop...
Source/core/dom/TreeScope.cpp:500: if (!m_baseURL.isEmpty() || document() ==
this)
Why not just make this virtual and let document override it?

[adamk, 2014-01-14 00:51:15]

Okay, made baseURL virtual, now this is quite a bit different. Not sure why I
didn't make it virtual in the first place, but there you have it.

[esprehn, 2014-01-14 00:52:22]

lgtm, beautiful.

[commit-bot: I haz the power, 2014-01-14 00:55:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/135643004/30002

[commit-bot: I haz the power, 2014-01-14 04:28:08]

Change committed as 165025