Commit d65c5aca authored by Dominic Mazzoni's avatar Dominic Mazzoni Committed by Commit Bot

AccessibleNode shouldn't cache an Element's Document.

AccessibleNode needs an owner document. If it's constructed without an
element, we get the document from the JavaScript context. That part is
fine. But if we construct it from an Element, we were caching its
Document - but elements can be reparented to a different document,
leading to a crash if the Element is reparented and then its original
document is deleted.

The solution is to not cache the Document if the AccessibleNode is associated
with an Element. Just get it from the Element.

Bug: 785802
Change-Id: I8d67f9117a5d9a9c9efbd978ac4d1965e4c0eb2a
Reviewed-on: https://chromium-review.googlesource.com/775616Reviewed-by: default avatarAlice Boxhall <aboxhall@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518387}
parent 9e97add1
...@@ -204,7 +204,7 @@ QualifiedName GetCorrespondingARIAAttribute(AOMIntProperty property) { ...@@ -204,7 +204,7 @@ QualifiedName GetCorrespondingARIAAttribute(AOMIntProperty property) {
} // namespace } // namespace
AccessibleNode::AccessibleNode(Element* element) AccessibleNode::AccessibleNode(Element* element)
: element_(element), document_(element->GetDocument()) { : element_(element), document_(nullptr) {
DCHECK(RuntimeEnabledFeatures::AccessibilityObjectModelEnabled()); DCHECK(RuntimeEnabledFeatures::AccessibilityObjectModelEnabled());
} }
...@@ -220,6 +220,16 @@ AccessibleNode* AccessibleNode::Create(Document& document) { ...@@ -220,6 +220,16 @@ AccessibleNode* AccessibleNode::Create(Document& document) {
return new AccessibleNode(document); return new AccessibleNode(document);
} }
Document* AccessibleNode::GetDocument() const {
if (document_)
return document_;
if (element_)
return &element_->GetDocument();
NOTREACHED();
return nullptr;
}
const AtomicString& AccessibleNode::GetProperty( const AtomicString& AccessibleNode::GetProperty(
AOMStringProperty property) const { AOMStringProperty property) const {
for (const auto& item : string_properties_) { for (const auto& item : string_properties_) {
...@@ -981,7 +991,7 @@ void AccessibleNode::appendChild(AccessibleNode* child, ...@@ -981,7 +991,7 @@ void AccessibleNode::appendChild(AccessibleNode* child,
return; return;
} }
if (!document_->GetSecurityOrigin()->CanAccess( if (!GetDocument()->GetSecurityOrigin()->CanAccess(
child->GetDocument()->GetSecurityOrigin())) { child->GetDocument()->GetSecurityOrigin())) {
exception_state.ThrowDOMException( exception_state.ThrowDOMException(
kInvalidAccessError, kInvalidAccessError,
...@@ -1125,7 +1135,7 @@ void AccessibleNode::NotifyAttributeChanged( ...@@ -1125,7 +1135,7 @@ void AccessibleNode::NotifyAttributeChanged(
} }
AXObjectCache* AccessibleNode::GetAXObjectCache() { AXObjectCache* AccessibleNode::GetAXObjectCache() {
return document_->ExistingAXObjectCache(); return GetDocument()->ExistingAXObjectCache();
} }
void AccessibleNode::Trace(blink::Visitor* visitor) { void AccessibleNode::Trace(blink::Visitor* visitor) {
......
...@@ -120,7 +120,7 @@ class CORE_EXPORT AccessibleNode : public EventTargetWithInlineData { ...@@ -120,7 +120,7 @@ class CORE_EXPORT AccessibleNode : public EventTargetWithInlineData {
Element* element() const { return element_; } Element* element() const { return element_; }
// Gets the associated document. // Gets the associated document.
Document* GetDocument() const { return document_.Get(); } Document* GetDocument() const;
// Children. These are only virtual AccessibleNodes that were added // Children. These are only virtual AccessibleNodes that were added
// explicitly, never AccessibleNodes from DOM Elements. // explicitly, never AccessibleNodes from DOM Elements.
...@@ -382,7 +382,7 @@ class CORE_EXPORT AccessibleNode : public EventTargetWithInlineData { ...@@ -382,7 +382,7 @@ class CORE_EXPORT AccessibleNode : public EventTargetWithInlineData {
// This object's owner Element, if it corresponds to an Element. // This object's owner Element, if it corresponds to an Element.
Member<Element> element_; Member<Element> element_;
// The object's owner Document. // The object's owner Document. Only set if |element_| is nullptr.
Member<Document> document_; Member<Document> document_;
// This object's AccessibleNode children, which must be only virtual // This object's AccessibleNode children, which must be only virtual
......
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