Commit bb3517a9 authored by Rakina Zata Amni's avatar Rakina Zata Amni Committed by Commit Bot

Revert "Revert "Fix retargeting of result in elementFromPoint and elementsFromPoint""

crrev.com/c/808446 is reverted because of failure in ASAN Buildbot
Revert CL Link: crrev.com/c/880264
Failure link: https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Trusty%20ASAN/builds/8618

The failure is accessing *target_ancestor_iterator when it is out of bounds.
Link: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/TreeScope.cpp?q=Treescope.cpp&sq=package:chromium&rcl=dd944882a245a5117b50cb417138d92f32d931d6&l=393
as there were no bound checks for target_ancestor_iterator. It wasn't caught
by layout tests because it's still returning the correct results, because
it doesn't crash when getting *target_ancestor_iterator when it's out of bound.
It just stops the while-loop and returned at
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/TreeScope.cpp?q=Treescope.cpp&sq=package:chromium&rcl=dd944882a245a5117b50cb417138d92f32d931d6&l=398
Also, since the ASAN buildbot is not done before the CL is merged, this wasn't
caught by trybots prior to committing.

The fix is just adding a bound check for target_ancestor_riterator here:
https://chromium-review.googlesource.com/c/chromium/src/+/880741/2..3/third_party/WebKit/Source/core/dom/TreeScope.cpp
I have confirmed by using ASAN locally that it is fixed now.
Before the fix, running the failing tests with ASAN build fails.


Bug: 759947,805039
Change-Id: I9934af8131f285045e0eb80923f190b6d88cef7d
Reviewed-on: https://chromium-review.googlesource.com/880741
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: default avatarHayato Ito <hayato@chromium.org>
Reviewed-by: default avatarTakayoshi Kochi <kochi@chromium.org>
Reviewed-by: default avatarDmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531839}
parent 3bc515d5
PASS shadowRoot.elementsFromPoint() threw exception TypeError: Failed to execute 'elementsFromPoint' on 'ShadowRoot': 2 arguments required, but only 0 present.. PASS shadowRoot.elementsFromPoint() threw exception TypeError: Failed to execute 'elementsFromPoint' on 'ShadowRoot': 2 arguments required, but only 0 present..
PASS shadowRoot.elementsFromPoint(0) threw exception TypeError: Failed to execute 'elementsFromPoint' on 'ShadowRoot': 2 arguments required, but only 1 present.. PASS shadowRoot.elementsFromPoint(0) threw exception TypeError: Failed to execute 'elementsFromPoint' on 'ShadowRoot': 2 arguments required, but only 1 present..
PASS nodeListToString(shadowRoot.elementsFromPoint(x12, y12)) is "DIV" PASS nodeListToString(shadowRoot.elementsFromPoint(x12, y12)) is "DIV, DIV#host, BODY, HTML"
PASS nodeListToString(nestedShadowRoot.elementsFromPoint(x22, y22)) is "DIV" PASS nodeListToString(nestedShadowRoot.elementsFromPoint(x22, y22)) is "DIV, DIV, DIV#host, BODY, HTML"
PASS nodeListToString(shadowRoot.elementsFromPoint(x22, y22)) is "DIV" PASS nodeListToString(shadowRoot.elementsFromPoint(x22, y22)) is "DIV, DIV#host, BODY, HTML"
PASS nodeListToString(document.elementsFromPoint(x22, y22)) is "DIV#host, BODY, HTML" PASS nodeListToString(document.elementsFromPoint(x22, y22)) is "DIV#host, BODY, HTML"
PASS nodeListToString(root3.elementsFromPoint(centerX(blockHost), centerY(blockHost))) is "" PASS nodeListToString(root3.elementsFromPoint(centerX(blockHost), centerY(blockHost))) is "DIV#blockHost, BODY, HTML"
PASS nodeListToString(document.elementsFromPoint(centerX(blockHost), centerY(blockHost))) is "DIV#blockHost, BODY, HTML" PASS nodeListToString(document.elementsFromPoint(centerX(blockHost), centerY(blockHost))) is "DIV#blockHost, BODY, HTML"
PASS nodeListToString(root4.elementsFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))) is "" PASS nodeListToString(root4.elementsFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))) is "SPAN#inlineBlockHost, BODY, HTML"
PASS nodeListToString(document.elementsFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))) is "SPAN#inlineBlockHost, BODY, HTML" PASS nodeListToString(document.elementsFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))) is "SPAN#inlineBlockHost, BODY, HTML"
PASS nodeListToString(document.elementsFromPoint(centerX(submit), centerY(submit))) is "INPUT#submit, BODY, HTML" PASS nodeListToString(document.elementsFromPoint(centerX(submit), centerY(submit))) is "INPUT#submit, BODY, HTML"
PASS successfullyParsed is true PASS successfullyParsed is true
......
...@@ -63,12 +63,12 @@ shouldThrow('shadowRoot.elementsFromPoint()'); ...@@ -63,12 +63,12 @@ shouldThrow('shadowRoot.elementsFromPoint()');
shouldThrow('shadowRoot.elementsFromPoint(0)'); shouldThrow('shadowRoot.elementsFromPoint(0)');
assertElementsFromPoint( assertElementsFromPoint(
'shadowRoot.elementsFromPoint(x12, y12)', [box12]); 'shadowRoot.elementsFromPoint(x12, y12)', [box12, host, document.body, document.documentElement]);
assertElementsFromPoint( assertElementsFromPoint(
'nestedShadowRoot.elementsFromPoint(x22, y22)', [box22]); 'nestedShadowRoot.elementsFromPoint(x22, y22)', [box22, nestedHost, host, document.body, document.documentElement]);
assertElementsFromPoint( assertElementsFromPoint(
'shadowRoot.elementsFromPoint(x22, y22)', [nestedHost]); 'shadowRoot.elementsFromPoint(x22, y22)', [nestedHost, host, document.body, document.documentElement]);
assertElementsFromPoint( assertElementsFromPoint(
'document.elementsFromPoint(x22, y22)', 'document.elementsFromPoint(x22, y22)',
[host, document.body, document.documentElement]); [host, document.body, document.documentElement]);
...@@ -80,13 +80,13 @@ root4.appendChild(document.createTextNode('text2')); ...@@ -80,13 +80,13 @@ root4.appendChild(document.createTextNode('text2'));
assertElementsFromPoint( assertElementsFromPoint(
'root3.elementsFromPoint(centerX(blockHost), centerY(blockHost))', 'root3.elementsFromPoint(centerX(blockHost), centerY(blockHost))',
[]); [blockHost, document.body, document.documentElement]);
assertElementsFromPoint( assertElementsFromPoint(
'document.elementsFromPoint(centerX(blockHost), centerY(blockHost))', 'document.elementsFromPoint(centerX(blockHost), centerY(blockHost))',
[blockHost, document.body, document.documentElement]); [blockHost, document.body, document.documentElement]);
assertElementsFromPoint( assertElementsFromPoint(
'root4.elementsFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))', 'root4.elementsFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))',
[]); [inlineBlockHost, document.body, document.documentElement]);
assertElementsFromPoint( assertElementsFromPoint(
'document.elementsFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))', 'document.elementsFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))',
[inlineBlockHost, document.body, document.documentElement]); [inlineBlockHost, document.body, document.documentElement]);
......
...@@ -5,9 +5,9 @@ PASS shadowRoot.elementFromPoint(x12, y12) is box12 ...@@ -5,9 +5,9 @@ PASS shadowRoot.elementFromPoint(x12, y12) is box12
PASS nestedShadowRoot.elementFromPoint(x22, y22) is box22 PASS nestedShadowRoot.elementFromPoint(x22, y22) is box22
PASS shadowRoot.elementFromPoint(x22, y22) is nestedHost PASS shadowRoot.elementFromPoint(x22, y22) is nestedHost
PASS document.elementFromPoint(x22, y22) is host PASS document.elementFromPoint(x22, y22) is host
PASS root3.elementFromPoint(centerX(blockHost), centerY(blockHost)) is null PASS root3.elementFromPoint(centerX(blockHost), centerY(blockHost)) is blockHost
PASS document.elementFromPoint(centerX(blockHost), centerY(blockHost)) is blockHost PASS document.elementFromPoint(centerX(blockHost), centerY(blockHost)) is blockHost
PASS root4.elementFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost)) is null PASS root4.elementFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost)) is inlineBlockHost
PASS document.elementFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost)) is inlineBlockHost PASS document.elementFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost)) is inlineBlockHost
PASS document.elementFromPoint(centerX(submit), centerY(submit)) is submit PASS document.elementFromPoint(centerX(submit), centerY(submit)) is submit
PASS successfullyParsed is true PASS successfullyParsed is true
......
...@@ -65,9 +65,9 @@ var root3 = blockHost.createShadowRoot(); ...@@ -65,9 +65,9 @@ var root3 = blockHost.createShadowRoot();
root3.appendChild(document.createTextNode('text1')); root3.appendChild(document.createTextNode('text1'));
var root4 = inlineBlockHost.createShadowRoot(); var root4 = inlineBlockHost.createShadowRoot();
root4.appendChild(document.createTextNode('text2')); root4.appendChild(document.createTextNode('text2'));
shouldBeNull('root3.elementFromPoint(centerX(blockHost), centerY(blockHost))'); shouldBe('root3.elementFromPoint(centerX(blockHost), centerY(blockHost))', 'blockHost');
shouldBe('document.elementFromPoint(centerX(blockHost), centerY(blockHost))', 'blockHost'); shouldBe('document.elementFromPoint(centerX(blockHost), centerY(blockHost))', 'blockHost');
shouldBeNull('root4.elementFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))'); shouldBe('root4.elementFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))', 'inlineBlockHost');
shouldBe('document.elementFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))', 'inlineBlockHost'); shouldBe('document.elementFromPoint(centerX(inlineBlockHost), centerY(inlineBlockHost))', 'inlineBlockHost');
shouldBe('document.elementFromPoint(centerX(submit), centerY(submit))', 'submit'); shouldBe('document.elementFromPoint(centerX(submit), centerY(submit))', 'submit');
</script> </script>
......
...@@ -251,16 +251,20 @@ Element* TreeScope::HitTestPoint(double x, ...@@ -251,16 +251,20 @@ Element* TreeScope::HitTestPoint(double x,
const HitTestRequest& request) const { const HitTestRequest& request) const {
HitTestResult result = HitTestResult result =
HitTestInDocument(&RootNode().GetDocument(), x, y, request); HitTestInDocument(&RootNode().GetDocument(), x, y, request);
Node* node = result.InnerNode(); return HitTestPointInternal(result.InnerNode());
}
Element* TreeScope::HitTestPointInternal(Node* node) const {
if (!node || node->IsDocumentNode()) if (!node || node->IsDocumentNode())
return nullptr; return nullptr;
Element* element;
if (node->IsPseudoElement() || node->IsTextNode()) if (node->IsPseudoElement() || node->IsTextNode())
node = node->ParentOrShadowHostNode(); element = node->ParentOrShadowHostElement();
DCHECK(!node || node->IsElementNode() || node->IsShadowRoot()); else
node = AncestorInThisScope(node); element = ToElement(node);
if (!node || !node->IsElementNode()) if (!element)
return nullptr; return nullptr;
return ToElement(node); return Retarget(*element);
} }
HeapVector<Member<Element>> TreeScope::ElementsFromHitTestResult( HeapVector<Member<Element>> TreeScope::ElementsFromHitTestResult(
...@@ -270,13 +274,11 @@ HeapVector<Member<Element>> TreeScope::ElementsFromHitTestResult( ...@@ -270,13 +274,11 @@ HeapVector<Member<Element>> TreeScope::ElementsFromHitTestResult(
Node* last_node = nullptr; Node* last_node = nullptr;
for (const auto rect_based_node : result.ListBasedTestResult()) { for (const auto rect_based_node : result.ListBasedTestResult()) {
Node* node = rect_based_node.Get(); Node* node = rect_based_node.Get();
if (!node || !node->IsElementNode() || node->IsDocumentNode()) // In some cases the hit test doesn't return slot elements, so we can only
// get it through its child and can't skip it.
if (!node->IsElementNode() && !IsHTMLSlotElement(node->parentNode()))
continue; continue;
node = HitTestPointInternal(node);
if (node->IsPseudoElement() || node->IsTextNode())
node = node->ParentOrShadowHostNode();
node = AncestorInThisScope(node);
// Prune duplicate entries. A pseduo ::before content above its parent // Prune duplicate entries. A pseduo ::before content above its parent
// node should only result in a single entry. // node should only result in a single entry.
if (node == last_node) if (node == last_node)
...@@ -366,7 +368,43 @@ void TreeScope::AdoptIfNeeded(Node& node) { ...@@ -366,7 +368,43 @@ void TreeScope::AdoptIfNeeded(Node& node) {
adopter.Execute(); adopter.Execute();
} }
// This method corresponds to the Retarget algorithm specified in
// https://dom.spec.whatwg.org/#retarget
// This retargets |target| against the root of |this|.
// The steps are different with the spec for performance reasons,
// but the results should be the same.
Element* TreeScope::Retarget(const Element& target) const { Element* TreeScope::Retarget(const Element& target) const {
const TreeScope& target_scope = target.GetTreeScope();
if (!target_scope.RootNode().IsShadowRoot())
return const_cast<Element*>(&target);
HeapVector<Member<const TreeScope>> target_ancestor_scopes;
HeapVector<Member<const TreeScope>> context_ancestor_scopes;
for (const TreeScope* tree_scope = &target_scope; tree_scope;
tree_scope = tree_scope->ParentTreeScope())
target_ancestor_scopes.push_back(tree_scope);
for (const TreeScope* tree_scope = this; tree_scope;
tree_scope = tree_scope->ParentTreeScope())
context_ancestor_scopes.push_back(tree_scope);
auto target_ancestor_riterator = target_ancestor_scopes.rbegin();
auto context_ancestor_riterator = context_ancestor_scopes.rbegin();
while (context_ancestor_riterator != context_ancestor_scopes.rend() &&
target_ancestor_riterator != target_ancestor_scopes.rend() &&
*context_ancestor_riterator == *target_ancestor_riterator) {
++context_ancestor_riterator;
++target_ancestor_riterator;
}
if (target_ancestor_riterator == target_ancestor_scopes.rend())
return const_cast<Element*>(&target);
Node& first_different_scope_root =
(*target_ancestor_riterator).Get()->RootNode();
return &ToShadowRoot(first_different_scope_root).host();
}
Element* TreeScope::AdjustedFocusedElementInternal(
const Element& target) const {
for (const Element* ancestor = &target; ancestor; for (const Element* ancestor = &target; ancestor;
ancestor = ancestor->OwnerShadowHost()) { ancestor = ancestor->OwnerShadowHost()) {
if (this == ancestor->GetTreeScope()) if (this == ancestor->GetTreeScope())
...@@ -385,7 +423,7 @@ Element* TreeScope::AdjustedFocusedElement() const { ...@@ -385,7 +423,7 @@ Element* TreeScope::AdjustedFocusedElement() const {
return nullptr; return nullptr;
if (RootNode().IsInV1ShadowTree()) { if (RootNode().IsInV1ShadowTree()) {
if (Element* retargeted = Retarget(*element)) { if (Element* retargeted = AdjustedFocusedElementInternal(*element)) {
return (this == &retargeted->GetTreeScope()) ? retargeted : nullptr; return (this == &retargeted->GetTreeScope()) ? retargeted : nullptr;
} }
return nullptr; return nullptr;
......
...@@ -92,6 +92,8 @@ class CORE_EXPORT TreeScope : public GarbageCollectedMixin { ...@@ -92,6 +92,8 @@ class CORE_EXPORT TreeScope : public GarbageCollectedMixin {
Element* Retarget(const Element& target) const; Element* Retarget(const Element& target) const;
Element* AdjustedFocusedElementInternal(const Element& target) const;
// Find first anchor with the given name. // Find first anchor with the given name.
// First searches for an element with the given ID, but if that fails, then // First searches for an element with the given ID, but if that fails, then
// looks for an anchor with the given name. ID matching is always case // looks for an anchor with the given name. ID matching is always case
...@@ -141,6 +143,8 @@ class CORE_EXPORT TreeScope : public GarbageCollectedMixin { ...@@ -141,6 +143,8 @@ class CORE_EXPORT TreeScope : public GarbageCollectedMixin {
void SetNeedsStyleRecalcForViewportUnits(); void SetNeedsStyleRecalcForViewportUnits();
private: private:
Element* HitTestPointInternal(Node*) const;
Member<ContainerNode> root_node_; Member<ContainerNode> root_node_;
Member<Document> document_; Member<Document> document_;
Member<TreeScope> parent_tree_scope_; Member<TreeScope> parent_tree_scope_;
......
...@@ -1211,6 +1211,25 @@ ...@@ -1211,6 +1211,25 @@
if (window.location.search.indexOf('remoteFrontend') === -1) if (window.location.search.indexOf('remoteFrontend') === -1)
return; return;
// Support for legacy (<M65) frontends.
/** @type {(!function(number, number):Element|undefined)} */
ShadowRoot.prototype.__originalShadowRootElementFromPoint;
if (!ShadowRoot.prototype.__originalShadowRootElementFromPoint) {
ShadowRoot.prototype.__originalShadowRootElementFromPoint = ShadowRoot.prototype.elementFromPoint;
/**
* @param {number} x
* @param {number} y
* @return {Element}
*/
ShadowRoot.prototype.elementFromPoint = function(x, y) {
var originalResult = ShadowRoot.prototype.__originalShadowRootElementFromPoint.apply(this, arguments);
if (this.host && originalResult === this.host)
return null;
return originalResult;
};
}
// Support for legacy (<M53) frontends. // Support for legacy (<M53) frontends.
if (!window.KeyboardEvent.prototype.hasOwnProperty('keyIdentifier')) { if (!window.KeyboardEvent.prototype.hasOwnProperty('keyIdentifier')) {
Object.defineProperty(window.KeyboardEvent.prototype, 'keyIdentifier', { Object.defineProperty(window.KeyboardEvent.prototype, 'keyIdentifier', {
......
...@@ -755,7 +755,7 @@ Document.prototype.deepElementFromPoint = function(x, y) { ...@@ -755,7 +755,7 @@ Document.prototype.deepElementFromPoint = function(x, y) {
var node = null; var node = null;
while (container) { while (container) {
var innerNode = container.elementFromPoint(x, y); var innerNode = container.elementFromPoint(x, y);
if (!innerNode) if (!innerNode || node === innerNode)
break; break;
node = innerNode; node = innerNode;
container = node.shadowRoot; container = node.shadowRoot;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment