Implementation of AXCoreObject::innerHTML and outerHTML.

• Component: New Bugs
• Status: RESOLVED FIXED

[Andres Gonzalez]

Implementation of AXCoreObject::innerHTML and outerHTML.

[Andres Gonzalez]

Created attachment 414347 [details]
Patch

[chris fleizach]

Comment on attachment 414347 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414347&action=review

> Source/WebCore/accessibility/AccessibilityObject.cpp:3584
> +String AccessibilityObject::innerHTML() const

can we do this without the debug flags? there's been lots of times now that I am trying to debug something in WK without debug symbols and just wishing I could convert from AX tree > HTML

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:2118
> +        const_cast<AXIsolatedObject*>(this)->setProperty(AXPropertyName::OuterHTML, value);

worried that we're setting this property on the main thread. I guess we'll be ok because the AX thread is also blocked. can you think of any other issues with this?

[Darin Adler]

Comment on attachment 414347 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414347&action=review

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:2103
> +        String value = axObject ? axObject->innerHTML().isolatedCopy()
> +            : String().isolatedCopy();

String().isolatedCopy() is the same as Striing().

[Andres Gonzalez]

Created attachment 414350 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #2)
> Comment on attachment 414347 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414347&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:3584
> > +String AccessibilityObject::innerHTML() const
> 
> can we do this without the debug flags? there's been lots of times now that
> I am trying to debug something in WK without debug symbols and just wishing
> I could convert from AX tree > HTML

Done.
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:2118
> > +        const_cast<AXIsolatedObject*>(this)->setProperty(AXPropertyName::OuterHTML, value);
> 
> worried that we're setting this property on the main thread. I guess we'll
> be ok because the AX thread is also blocked. can you think of any other
> issues with this?

It could be a problem if this method is called on the main thread and there are other isolated object methods being called on the AX thread. This shouldn't happen though. Let me see if I can add an assert to ensure this in debug.

[Andres Gonzalez]

(In reply to Darin Adler from comment #3)
> Comment on attachment 414347 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414347&action=review
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:2103
> > +        String value = axObject ? axObject->innerHTML().isolatedCopy()
> > +            : String().isolatedCopy();
> 
> String().isolatedCopy() is the same as Striing().

Fixed.

[Andres Gonzalez]

(In reply to chris fleizach from comment #2)
> Comment on attachment 414347 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414347&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:3584
> > +String AccessibilityObject::innerHTML() const
> 
> can we do this without the debug flags? there's been lots of times now that
> I am trying to debug something in WK without debug symbols and just wishing
> I could convert from AX tree > HTML
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:2118
> > +        const_cast<AXIsolatedObject*>(this)->setProperty(AXPropertyName::OuterHTML, value);
> 
> worried that we're setting this property on the main thread. I guess we'll
> be ok because the AX thread is also blocked. can you think of any other
> issues with this?

The problem with the assert is that if you break on the main thread during debugging with isolated mode on, then you cannot call these methods. You can only call it on the secondary thread. Would that be ok?

[chris fleizach]

(In reply to Andres Gonzalez from comment #7)
> (In reply to chris fleizach from comment #2)
> > Comment on attachment 414347 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=414347&action=review
> > 
> > > Source/WebCore/accessibility/AccessibilityObject.cpp:3584
> > > +String AccessibilityObject::innerHTML() const
> > 
> > can we do this without the debug flags? there's been lots of times now that
> > I am trying to debug something in WK without debug symbols and just wishing
> > I could convert from AX tree > HTML
> > 
> > > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:2118
> > > +        const_cast<AXIsolatedObject*>(this)->setProperty(AXPropertyName::OuterHTML, value);
> > 
> > worried that we're setting this property on the main thread. I guess we'll
> > be ok because the AX thread is also blocked. can you think of any other
> > issues with this?
> 
> The problem with the assert is that if you break on the main thread during
> debugging with isolated mode on, then you cannot call these methods. You can
> only call it on the secondary thread. Would that be ok?

I think we're probably OK because the secondary thread will be occupied while the main thread modifies the property map, but its not really air-tight.

[EWS]

Committed r269923: <https://trac.webkit.org/changeset/269923>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414350 [details].

[Radar WebKit Bug Importer]

<rdar://problem/71504019>