Bug 268804

Created attachment 469736 [details] Patch

(In reply to chris fleizach from comment #1) > Created attachment 469736 [details] > Patch diff --git a/Source/WebCore/accessibility/AXCoreObject.h b/Source/WebCore/accessibility/AXCoreObject.h index 09fce90d3ecc..5d0f352eef6b 100644 --- a/Source/WebCore/accessibility/AXCoreObject.h +++ b/Source/WebCore/accessibility/AXCoreObject.h @@ -205,6 +205,7 @@ enum class AccessibilityRole { ProgressIndicator, RadioButton, RadioGroup, + RemoteFrame, RowHeader, Row, RowGroup, @@ -442,6 +443,8 @@ ALWAYS_INLINE String accessibilityRoleToString(AccessibilityRole role) return "RadioButton"_s; case AccessibilityRole::RadioGroup: return "RadioGroup"_s; + case AccessibilityRole::RemoteFrame: + return "RemoteFrame"_s; case AccessibilityRole::RowHeader: return "RowHeader"_s; case AccessibilityRole::Row: @@ -818,6 +821,7 @@ public: virtual bool isAccessibilityARIAGridRowInstance() const = 0; virtual bool isAccessibilityARIAGridCellInstance() const = 0; virtual bool isAXIsolatedObjectInstance() const = 0; + virtual bool isAccessibilityRemoteFrame() const = 0; AG: can we use the AX prefix instead of the whole accessibility word for file and class names already inside the accessibility subproject? E.g., isAXIsolatedObjectInstance(). bool isHeading() const { return roleValue() == AccessibilityRole::Heading; } virtual bool isLink() const = 0; @@ -922,6 +926,12 @@ public: bool isTreeGrid() const { return roleValue() == AccessibilityRole::TreeGrid; } bool isTreeItem() const { return roleValue() == AccessibilityRole::TreeItem; } bool isScrollbar() const { return roleValue() == AccessibilityRole::ScrollBar; } + bool isRemoteFrame() const { return roleValue() == AccessibilityRole::RemoteFrame; } +#if PLATFORM(COCOA) + virtual RetainPtr<id> remoteFramePlatformElement() const = 0; +#endif + virtual bool hasRemoteFrameChild() const = 0; + bool isButton() const; virtual bool isMeter() const = 0; @@ -1161,6 +1171,7 @@ public: // Viewport-relative means that when the page scrolls, the portion of the page in the viewport changes, and thus // any viewport-relative rects do too (since they are either closer to or farther from the viewport origin after the scroll). virtual FloatPoint screenRelativePosition() const = 0; + virtual IntPoint remoteFrameOffset() const = 0; virtual FloatRect convertFrameToSpace(const FloatRect&, AccessibilityConversionSpace) const = 0; #if PLATFORM(COCOA) diff --git a/Source/WebCore/accessibility/AXLogger.cpp b/Source/WebCore/accessibility/AXLogger.cpp index 659d885e3559..a1ac4e3f70e2 100644 --- a/Source/WebCore/accessibility/AXLogger.cpp +++ b/Source/WebCore/accessibility/AXLogger.cpp @@ -873,6 +873,9 @@ void streamAXCoreObject(TextStream& stream, const AXCoreObject& object, const Op if (options & AXStreamOptions::Role) stream.dumpProperty("role", object.roleValue()); + if (object.renderer()) + stream.dumpProperty("renderName", object.renderer()->renderName()); + if (options & AXStreamOptions::ParentID) { auto* parent = object.parentObjectUnignored(); stream.dumpProperty("parentID", parent ? parent->objectID() : AXID()); diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp index a8d8a18ccfdc..b42a225a07d9 100644 --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ b/Source/WebCore/accessibility/AXObjectCache.cpp @@ -50,6 +50,7 @@ #include "AccessibilityMenuListOption.h" #include "AccessibilityMenuListPopup.h" #include "AccessibilityProgressIndicator.h" +#include "AccessibilityRemoteFrame.h" #include "AccessibilityRenderObject.h" #include "AccessibilitySVGElement.h" #include "AccessibilitySVGRoot.h" @@ -1043,7 +1044,10 @@ AccessibilityObject* AXObjectCache::create(AccessibilityRole role) break; case AccessibilityRole::TableHeaderContainer: obj = AccessibilityTableHeaderContainer::create(); - break; + break; + case AccessibilityRole::RemoteFrame: + obj = AccessibilityRemoteFrame::create(); + break; case AccessibilityRole::SliderThumb: obj = AccessibilitySliderThumb::create(); break; @@ -1339,6 +1343,11 @@ void AXObjectCache::handleRecomputeCellSlots(AccessibilityTable& axTable) #endif } +void AXObjectCache::onRemoteFrameInitialized(AccessibilityRemoteFrame& remoteFrame) +{ + updateIsolatedTree(remoteFrame, AXPropertyName::RemoteFramePlatformElement); AG: missing #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) +} + #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) void AXObjectCache::handleRowspanChanged(AccessibilityTableCell& axCell) { diff --git a/Source/WebCore/accessibility/AXObjectCache.h b/Source/WebCore/accessibility/AXObjectCache.h index 6970eb7e3339..a635ec0682e3 100644 --- a/Source/WebCore/accessibility/AXObjectCache.h +++ b/Source/WebCore/accessibility/AXObjectCache.h @@ -47,6 +47,7 @@ class TextStream; namespace WebCore { +class AccessibilityRemoteFrame; class AccessibilityTable; class AccessibilityTableCell; class Document; @@ -246,6 +247,7 @@ public: void deferNodeAddedOrRemoved(Node*); void handleScrolledToAnchor(const Node* anchorNode); void onScrollbarUpdate(ScrollView*); + void onRemoteFrameInitialized(AccessibilityRemoteFrame&); bool isRetrievingCurrentModalNode() { return m_isRetrievingCurrentModalNode; } Node* modalNode(); diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp index de9ac2598217..ebc314210e7d 100644 --- a/Source/WebCore/accessibility/AccessibilityObject.cpp +++ b/Source/WebCore/accessibility/AccessibilityObject.cpp @@ -544,7 +544,10 @@ FloatRect AccessibilityObject::convertFrameToSpace(const FloatRect& frameRect, A FloatRect AccessibilityObject::relativeFrame() const { - return convertFrameToSpace(elementRect(), AccessibilityConversionSpace::Page); + auto rect = elementRect(); + auto remoteOffset = remoteFrameOffset(); + rect.move(remoteOffset.x(), remoteOffset.y()); + return convertFrameToSpace(rect, AccessibilityConversionSpace::Page); } AccessibilityObject* AccessibilityObject::nextSiblingUnignored(int limit) const @@ -2092,6 +2095,17 @@ RemoteAXObjectRef AccessibilityObject::remoteParentObject() const } return nullptr; } + +IntPoint AccessibilityObject::remoteFrameOffset() const +{ + if (auto* document = this->document()) { + if (auto* frame = document->frame()) + return frame->loader().client().accessibilityRemoteFrameOffset(); + } + + return { }; +} + #endif Document* AccessibilityObject::document() const diff --git a/Source/WebCore/accessibility/AccessibilityObject.h b/Source/WebCore/accessibility/AccessibilityObject.h index 0af77439c355..fba1e0515a19 100644 --- a/Source/WebCore/accessibility/AccessibilityObject.h +++ b/Source/WebCore/accessibility/AccessibilityObject.h @@ -102,6 +102,7 @@ public: virtual bool isAccessibilityNodeObject() const { return false; } bool isAccessibilityRenderObject() const override { return false; } virtual bool isAccessibilityScrollbar() const { return false; } + bool isAccessibilityRemoteFrame() const override { return false; } virtual bool isAccessibilityScrollViewInstance() const { return false; } virtual bool isAccessibilitySVGRoot() const { return false; } bool isAccessibilityTableInstance() const override { return false; } @@ -463,8 +464,12 @@ public: #if PLATFORM(COCOA) RemoteAXObjectRef remoteParentObject() const override; + IntPoint remoteFrameOffset() const override; FloatRect convertRectToPlatformSpace(const FloatRect&, AccessibilityConversionSpace) const override; + RetainPtr<id> remoteFramePlatformElement() const override { return nil; } #endif + bool hasRemoteFrameChild() const override { return false; } + Page* page() const override; Document* document() const override; LocalFrameView* documentFrameView() const override; diff --git a/Source/WebCore/accessibility/AccessibilityRemoteFrame.cpp b/Source/WebCore/accessibility/AccessibilityRemoteFrame.cpp new file mode 100644 index 000000000000..ce92ab2c66ec --- /dev/null +++ b/Source/WebCore/accessibility/AccessibilityRemoteFrame.cpp AG: AXRemoteFrame.cpp @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2024 Apple Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF + * THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "config.h" +#include "AccessibilityRemoteFrame.h" + +namespace WebCore { + +Ref<AccessibilityRemoteFrame> AccessibilityRemoteFrame::create() +{ + return adoptRef(*new AccessibilityRemoteFrame); +} + +AccessibilityRemoteFrame::AccessibilityRemoteFrame() +: AG: indentation. +#if PLATFORM(COCOA) + m_remoteFramePlatformElement(nullptr) + , +#endif + m_processIdentifier(0) +{ +} + +LayoutRect AccessibilityRemoteFrame::elementRect() const +{ + if (auto* parent = parentObject()) + return parent->elementRect(); + + return { }; +} AG: more succinct: auto* parent = parentObject(); return parent ? parent->elementRect() : { }; + +} // namespace WebCore diff --git a/Source/WebCore/accessibility/AccessibilityRemoteFrame.h b/Source/WebCore/accessibility/AccessibilityRemoteFrame.h new file mode 100644 index 000000000000..187b61f32201 --- /dev/null +++ b/Source/WebCore/accessibility/AccessibilityRemoteFrame.h AG: AXRemoteFrame.h @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2023 Apple Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF + * THE POSSIBILITY OF SUCH DAMAGE. + */ + +#pragma once + +#include "AccessibilityMockObject.h" + +namespace WebCore { + +class RemoteFrame; + +class AccessibilityRemoteFrame final : public AccessibilityMockObject { AG: AXRemoteFrame. +public: + static Ref<AccessibilityRemoteFrame> create(); + + void initializePlatformElementWithRemoteToken(const DataSegment&, int); + + RefPtr<DataSegment> generateRemoteToken() const; +#if PLATFORM(COCOA) + RetainPtr<id> remoteFramePlatformElement() const final { return m_remoteFramePlatformElement; } + int processIdentifier() const { return m_processIdentifier; } AG: shouldn't return pid_t ? +#endif + +private: + explicit AccessibilityRemoteFrame(); + virtual ~AccessibilityRemoteFrame() = default; + + AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::RemoteFrame; } + bool computeAccessibilityIsIgnored() const override { return false; } + bool isAccessibilityRemoteFrame() const override { return true; } AG: isAXRemoteFrameInstance ? + LayoutRect elementRect() const override; + +#if PLATFORM(COCOA) + RetainPtr<id> m_remoteFramePlatformElement; +#endif + pid_t m_processIdentifier; AG: doesn't it need to be COCOA only? +}; + +} // namespace WebCore + +SPECIALIZE_TYPE_TRAITS_ACCESSIBILITY(AccessibilityRemoteFrame, isAccessibilityRemoteFrame()) diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp index bb68607d9b7c..2deb5d057f98 100644 --- a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp +++ b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp @@ -2395,7 +2395,7 @@ void AccessibilityRenderObject::addAttachmentChildren() // LocalFrameView's need to be inserted into the AX hierarchy when encountered. Widget* widget = widgetForAttachmentView(); - if (!widget || !widget->isLocalFrameView()) + if (!widget || !(widget->isLocalFrameView() || widget->isRemoteFrameView())) return; addChild(axObjectCache()->getOrCreate(widget)); @@ -2550,7 +2550,7 @@ void AccessibilityRenderObject::addChildren() auto owners = object.owners(); if (owners.size() && !owners.contains(this)) return; - + addChild(&object); }; diff --git a/Source/WebCore/accessibility/AccessibilityScrollView.cpp b/Source/WebCore/accessibility/AccessibilityScrollView.cpp index 6bc841ebb067..24d20058777c 100644 --- a/Source/WebCore/accessibility/AccessibilityScrollView.cpp +++ b/Source/WebCore/accessibility/AccessibilityScrollView.cpp @@ -27,10 +27,12 @@ #include "AccessibilityScrollView.h" #include "AXObjectCache.h" +#include "AccessibilityRemoteFrame.h" #include "AccessibilityScrollbar.h" #include "HTMLFrameOwnerElement.h" #include "LocalFrame.h" #include "LocalFrameView.h" +#include "RemoteFrameView.h" #include "RenderElement.h" #include "Widget.h" @@ -52,6 +54,14 @@ AccessibilityScrollView::~AccessibilityScrollView() void AccessibilityScrollView::detachRemoteParts(AccessibilityDetachmentType detachmentType) { AccessibilityObject::detachRemoteParts(detachmentType); + + if (is<RemoteFrameView>(m_scrollView) && m_remoteFrame && (detachmentType == AccessibilityDetachmentType::ElementDestroyed || detachmentType == AccessibilityDetachmentType::CacheDestroyed)) { + AG: extra blank line. + auto& remoteFrame = downcast<RemoteFrameView>(*m_scrollView).frame(); AG: instead of is<RemoteFrameView> and downcast<RemoteFrameView>, you want auto* remoteFrameView = dynamicDowncast<RemoteFrameView>(m_scrollView); + remoteFrame.unbindRemoteAccessibilityFrames(m_remoteFrame->processIdentifier()); + m_remoteFrame = nullptr; + } + m_scrollView = nullptr; m_frameOwnerElement = nullptr; } @@ -177,8 +187,10 @@ AccessibilityScrollbar* AccessibilityScrollView::addChildScrollbar(Scrollbar* sc void AccessibilityScrollView::clearChildren() { AccessibilityObject::clearChildren(); + m_verticalScrollbar = nullptr; m_horizontalScrollbar = nullptr; + m_childrenDirty = false; } @@ -191,11 +203,42 @@ bool AccessibilityScrollView::computeAccessibilityIsIgnored() const return webArea->accessibilityIsIgnored(); } +void AccessibilityScrollView::addRemoteFrameChild() +{ + if (!is<RemoteFrameView>(m_scrollView)) + return; + auto* cache = axObjectCache(); AG: CheckedPtr or WeakPtr ? + if (!cache) + return; + + if (!m_remoteFrame) { + // Make the faux element that represents the remote transfer element for AX. + auto& remoteFrame = downcast<RemoteFrameView>(*m_scrollView).frame(); + m_remoteFrame = downcast<AccessibilityRemoteFrame>(cache->create(AccessibilityRole::RemoteFrame)); + m_remoteFrame->setParent(this); + + // Generate a new token and pass it back to the other remote frame so it can bind these objects together. + auto generatedToken = m_remoteFrame->generateRemoteToken(); + remoteFrame.bindRemoteAccessibilityFrames(getpid(), *generatedToken, [this, &remoteFrame, protectedAccessbilityRemoteFrame = RefPtr { m_remoteFrame }] (const DataSegment& token, int processIdentifier) mutable { +#if OS(DARWIN) + protectedAccessbilityRemoteFrame->initializePlatformElementWithRemoteToken(token, processIdentifier); +#endif + // Update the remote side with the offset of this object so it can calculate frames correctly. + auto location = elementRect().location(); + remoteFrame.updateRemoteFrameAccessibilityOffset(flooredIntPoint(location)); + }); + } else + m_remoteFrame->setParent(this); + + addChild(m_remoteFrame.get()); +} + void AccessibilityScrollView::addChildren() { ASSERT(!m_childrenInitialized); m_childrenInitialized = true; + addRemoteFrameChild(); addChild(webAreaObject()); updateScrollbars(); } @@ -203,7 +246,7 @@ void AccessibilityScrollView::addChildren() AccessibilityObject* AccessibilityScrollView::webAreaObject() const { auto* document = this->document(); - if (!document || !document->hasLivingRenderTree()) + if (!document || !document->hasLivingRenderTree() || m_remoteFrame) return nullptr; if (auto* cache = axObjectCache()) @@ -234,9 +277,15 @@ LayoutRect AccessibilityScrollView::elementRect() const Document* AccessibilityScrollView::document() const { - if (auto* frameView = dynamicDowncast<LocalFrameView>(m_scrollView.get())) { + if (auto* frameView = dynamicDowncast<LocalFrameView>(m_scrollView.get())) return frameView->frame().document(); + + // For the RemoteFrameView case, we need to return the document of our hosting parent so axObjectCache() resolves correctly. + if (auto* remoteFrameView = dynamicDowncast<RemoteFrameView>(m_scrollView.get())) { + if (auto* owner = remoteFrameView->frame().ownerElement()) + return &(owner->document()); } + return AccessibilityObject::document(); } @@ -253,16 +302,16 @@ LocalFrameView* AccessibilityScrollView::documentFrameView() const AccessibilityObject* AccessibilityScrollView::parentObject() const { - AXObjectCache* cache = axObjectCache(); - if (!cache) - return nullptr; - HTMLFrameOwnerElement* owner = m_frameOwnerElement.get(); if (is<LocalFrameView>(m_scrollView)) owner = downcast<LocalFrameView>(*m_scrollView).frame().ownerElement(); + else if (is<RemoteFrameView>(m_scrollView)) + owner = downcast<RemoteFrameView>(*m_scrollView).frame().ownerElement(); AG: better to avoid the is<> + downcast<> with a dynamicDowncast, the former is less efficient. - if (owner && owner->renderer()) - return cache->getOrCreate(owner); + if (AXObjectCache* cache = axObjectCache()) { AG: CheckedPtr or WeakPtr ? + if (owner && owner->renderer()) + return cache->getOrCreate(owner); + } return nullptr; } diff --git a/Source/WebCore/accessibility/AccessibilityScrollView.h b/Source/WebCore/accessibility/AccessibilityScrollView.h index 557044671c7d..85f91a91f5ca 100644 --- a/Source/WebCore/accessibility/AccessibilityScrollView.h +++ b/Source/WebCore/accessibility/AccessibilityScrollView.h @@ -30,6 +30,7 @@ namespace WebCore { +class AccessibilityRemoteFrame; class AccessibilityScrollbar; class Scrollbar; class ScrollView; @@ -54,7 +55,8 @@ private: bool computeAccessibilityIsIgnored() const override; bool isAccessibilityScrollViewInstance() const override { return true; } bool isEnabled() const override { return true; } - + bool hasRemoteFrameChild() const override { return m_remoteFrame != nullptr; } AG: shouldn't it be final? Remove != nullptr. + bool isAttachment() const override; PlatformWidget platformWidget() const override; Widget* widgetForAttachmentView() const override { return currentScrollView(); } @@ -69,6 +71,7 @@ private: void setFocused(bool) override; bool canSetFocusAttribute() const override; bool isFocused() const override; + void addRemoteFrameChild(); Document* document() const override; LocalFrameView* documentFrameView() const override; @@ -85,6 +88,7 @@ private: RefPtr<AccessibilityObject> m_horizontalScrollbar; RefPtr<AccessibilityObject> m_verticalScrollbar; bool m_childrenDirty; + RefPtr<AccessibilityRemoteFrame> m_remoteFrame; }; } // namespace WebCore diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp index 3f2031332ad1..4b9f3166e31e 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp @@ -375,6 +375,10 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb #endif setObjectProperty(AXPropertyName::InternalLinkElement, object.internalLinkElement()); + setProperty(AXPropertyName::HasRemoteFrameChild, object.hasRemoteFrameChild()); AG: Can this be true for other than a ScrollView? + // Don't duplicate the remoteFrameOffset for every object. Just store in the WebArea. + if (object.isWebArea()) + setProperty(AXPropertyName::RemoteFrameOffset, object.remoteFrameOffset()); initializePlatformProperties(axObject); } @@ -461,6 +465,7 @@ void AXIsolatedObject::setProperty(AXPropertyName propertyName, AXPropertyValueV [](OptionSet<AXAncestorFlag>& typedValue) { return typedValue.isEmpty(); }, #if PLATFORM(COCOA) [](RetainPtr<NSAttributedString>& typedValue) { return !typedValue; }, + [](RetainPtr<id>& typedValue) { return !typedValue; }, #endif [](InsideLink& typedValue) { return typedValue == InsideLink(); }, [](Vector<Vector<AXID>>& typedValue) { return typedValue.isEmpty(); }, @@ -1203,6 +1208,19 @@ LayoutRect AXIsolatedObject::elementRect() const }); } +IntPoint AXIsolatedObject::remoteFrameOffset() const +{ + IntPoint remoteFrameOffset = { }; AG: Don't need the = { } + for (auto *isolatedObject = this; isolatedObject != nullptr; isolatedObject = isolatedObject->parentObject()) { + if (auto point = optionalAttributeValue<IntPoint>(AXPropertyName::RemoteFrameOffset)) { + remoteFrameOffset = *point; + break; + } + } AG: do you mean isolatedObject->optionalAttributeValue ... ? This is only set for the WebArea, so why we need to get it for every ancestor? Wouldn't it be better to get the WebArea first?' + + return remoteFrameOffset; +} + FloatPoint AXIsolatedObject::screenRelativePosition() const { if (auto point = optionalAttributeValue<FloatPoint>(AXPropertyName::ScreenRelativePosition)) @@ -1226,28 +1244,32 @@ FloatPoint AXIsolatedObject::screenRelativePosition() const FloatRect AXIsolatedObject::relativeFrame() const { + auto remoteOffset = remoteFrameOffset(); + FloatRect relativeFrame = { }; AG: don't need the = { } + if (auto cachedRelativeFrame = optionalAttributeValue<IntRect>(AXPropertyName::RelativeFrame)) { // We should not have cached a relative frame for elements that get their geometry from their children. ASSERT(!m_getsGeometryFromChildren); - return *cachedRelativeFrame; - } - - if (m_getsGeometryFromChildren) { + relativeFrame = *cachedRelativeFrame; + } else if (m_getsGeometryFromChildren) { auto frame = enclosingIntRect(relativeFrameFromChildren()); if (!frame.isEmpty()) - return frame; + relativeFrame = frame; // Either we had no children, or our children had empty frames. The right thing to do would be to return // a rect at the position of the nearest render tree ancestor with some made-up size (AccessibilityNodeObject::boundingBoxRect does this). // However, we don't have access to the render tree in this context (only the AX isolated tree, which is too sparse for this purpose), so // until we cache the necessary information let's go to the main-thread. } else if (roleValue() == AccessibilityRole::Column || roleValue() == AccessibilityRole::TableHeaderContainer) - return exposedTableAncestor() ? relativeFrameFromChildren() : FloatRect(); + relativeFrame = exposedTableAncestor() ? relativeFrameFromChildren() : FloatRect(); - return Accessibility::retrieveValueFromMainThread<FloatRect>([this] () -> FloatRect { + relativeFrame = Accessibility::retrieveValueFromMainThread<FloatRect>([this] () -> FloatRect { if (auto* axObject = associatedAXObject()) return axObject->relativeFrame(); return { }; }); + + relativeFrame.moveBy(FloatPoint(remoteOffset)); + return relativeFrame; } FloatRect AXIsolatedObject::relativeFrameFromChildren() const diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h index dafff1f45d04..c689f563ec73 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h @@ -214,6 +214,7 @@ private: bool isFileUploadButton() const final { return boolAttributeValue(AXPropertyName::IsFileUploadButton); } bool isMeter() const final { return boolAttributeValue(AXPropertyName::IsMeter); }; FloatPoint screenRelativePosition() const final; + IntPoint remoteFrameOffset() const final; FloatRect relativeFrame() const final; #if PLATFORM(MAC) FloatRect primaryScreenRect() const final; @@ -454,7 +455,7 @@ private: bool isAccessibilityARIAGridInstance() const final { return false; } bool isAccessibilityARIAGridRowInstance() const final { return false; } bool isAccessibilityARIAGridCellInstance() const final { return false; } - + bool isAccessibilityRemoteFrame() const final { return false; } bool isNativeTextControl() const final; bool isListBoxOption() const final; bool isMockObject() const final; @@ -534,7 +535,9 @@ private: String nameAttribute() const final { return stringAttributeValue(AXPropertyName::NameAttribute); } #if PLATFORM(COCOA) bool hasApplePDFAnnotationAttribute() const final { return boolAttributeValue(AXPropertyName::HasApplePDFAnnotationAttribute); } + RetainPtr<id> remoteFramePlatformElement() const final; #endif + bool hasRemoteFrameChild() const override { return boolAttributeValue(AXPropertyName::HasRemoteFrameChild); } #if PLATFORM(COCOA) && ENABLE(MODEL_ELEMENT) Vector<RetainPtr<id>> modelElementChildren() final; diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp index 2ca457181608..a2821de8dcdc 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp @@ -660,6 +660,12 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const AXProper case AXPropertyName::PosInSet: propertyMap.set(AXPropertyName::PosInSet, axObject.posInSet()); break; + case AXPropertyName::RemoteFramePlatformElement: + propertyMap.set(AXPropertyName::RemoteFramePlatformElement, axObject.remoteFramePlatformElement()); + break; + case AXPropertyName::HasRemoteFrameChild: + propertyMap.set(AXPropertyName::HasRemoteFrameChild, axObject.hasRemoteFrameChild()); + break; case AXPropertyName::RoleDescription: propertyMap.set(AXPropertyName::RoleDescription, axObject.roleDescription().isolatedCopy()); break; diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h index 51a04c72e0cd..8fa0ba3e007d 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h @@ -104,6 +104,7 @@ enum class AXPropertyName : uint16_t { HasHighlighting, HasItalicFont, HasPlainText, + HasRemoteFrameChild, HasUnderline, HeaderContainer, HeadingLevel, @@ -199,6 +200,8 @@ enum class AXPropertyName : uint16_t { PreventKeyboardDOMEventDispatch, RadioButtonGroup, RelativeFrame, + RemoteFrameOffset, + RemoteFramePlatformElement, RoleValue, RolePlatformString, RoleDescription, @@ -254,6 +257,7 @@ using AXPropertyNameSet = HashSet<AXPropertyName, IntHash<AXPropertyName>, WTF:: using AXPropertyValueVariant = std::variant<std::nullptr_t, AXID, String, bool, int, unsigned, double, float, uint64_t, AccessibilityButtonState, Color, URL, LayoutRect, FloatPoint, FloatRect, IntPoint, IntRect, std::pair<unsigned, unsigned>, Vector<AccessibilityText>, Vector<AXID>, Vector<std::pair<AXID, AXID>>, Vector<String>, Path, OptionSet<AXAncestorFlag>, InsideLink, Vector<Vector<AXID>>, CharacterRange, std::pair<AXID, CharacterRange> #if PLATFORM(COCOA) , RetainPtr<NSAttributedString> + , RetainPtr<id> #endif #if ENABLE(AX_THREAD_TEXT_APIS) , AXTextRuns diff --git a/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm b/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm index 2bd094845e2e..386b2a2872aa 100644 --- a/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm +++ b/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm @@ -63,6 +63,9 @@ void AXIsolatedObject::initializePlatformProperties(const Ref<const Accessibilit } } + if (object->isRemoteFrame()) + setProperty(AXPropertyName::RemoteFramePlatformElement, object->remoteFramePlatformElement()); + // Cache the StringValue only if it differs from the AttributedText. auto value = object->stringValue(); if (!attributedText || value != String([attributedText string])) @@ -177,6 +180,11 @@ unsigned AXIsolatedObject::textLength() const return 0; } +RetainPtr<id> AXIsolatedObject::remoteFramePlatformElement() const +{ + return propertyValue<RetainPtr<id>>(AXPropertyName::RemoteFramePlatformElement); +} + RetainPtr<NSAttributedString> AXIsolatedObject::attributedStringForTextMarkerRange(AXTextMarkerRange&& markerRange, SpellCheck spellCheck) const { if (!markerRange) diff --git a/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm b/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm index ba2accd80d04..5d7fcf36429e 100644 --- a/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm +++ b/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm @@ -28,6 +28,7 @@ #import "AccessibilityLabel.h" #import "AccessibilityList.h" +#import "AccessibilityRemoteFrame.h" #import "ColorCocoa.h" #import "CompositionHighlight.h" #import "CompositionUnderline.h" @@ -51,6 +52,7 @@ #import "WebAccessibilityObjectWrapperMac.h" #import "Widget.h" +#import <pal/spi/cocoa/NSAccessibilitySPI.h> #import <pal/spi/mac/NSSpellCheckerSPI.h> namespace WebCore { @@ -749,6 +751,30 @@ RetainPtr<NSAttributedString> attributedStringCreate(Node* node, StringView text return result; } +RefPtr<DataSegment> AccessibilityRemoteFrame::generateRemoteToken() const +{ +#if PLATFORM(MAC) + // We use the parent's wrapper so that the remote frame acts as a pass through for the remote token bridge. + CFDataRef dataRef = (__bridge CFDataRef)[NSAccessibilityRemoteUIElement remoteTokenForLocalUIElement:parentObject()->wrapper()]; + return DataSegment::create(dataRef); +#else + // TDOO: Support iOS AG: TODO -> FIXME + return { } +#endif +} + +void AccessibilityRemoteFrame::initializePlatformElementWithRemoteToken(const DataSegment& token, int processIdentifier) +{ +#if PLATFORM(MAC) + m_processIdentifier = processIdentifier; + [wrapper() accessibilitySetPresenterProcessIdentifier:processIdentifier]; + NSData *tokenData = [NSData dataWithBytes:token.data() length:token.size()]; + m_remoteFramePlatformElement = adoptNS([[NSAccessibilityRemoteUIElement alloc] initWithRemoteToken:tokenData]); + + axObjectCache()->onRemoteFrameInitialized(*this); +#endif +} + namespace Accessibility { PlatformRoleMap createPlatformRoleMap() @@ -891,6 +917,7 @@ PlatformRoleMap createPlatformRoleMap() { AccessibilityRole::Superscript, NSAccessibilityGroupRole }, { AccessibilityRole::Model, NSAccessibilityGroupRole }, { AccessibilityRole::Suggestion, NSAccessibilityGroupRole }, + { AccessibilityRole::RemoteFrame, NSAccessibilityGroupRole }, }; PlatformRoleMap roleMap; for (auto& role : roles) diff --git a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm index 6357fa7c271b..e0755839244d 100644 --- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm +++ b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm @@ -35,6 +35,7 @@ #import "AccessibilityARIAGridRow.h" #import "AccessibilityList.h" #import "AccessibilityListBox.h" +#import "AccessibilityRemoteFrame.h" #import "AccessibilityRenderObject.h" #import "AccessibilityScrollView.h" #import "AccessibilitySpinButton.h" @@ -265,13 +266,13 @@ NSArray *makeNSArray(const WebCore::AXCoreObject::AccessibilityChildrenVector& c return nil; auto wrapper = child->wrapper(); - // We want to return the attachment view instead of the object representing the attachment, // otherwise, we get palindrome errors in the AX hierarchy. if (child->isAttachment()) { if (id attachmentView = wrapper.attachmentView) return attachmentView; - } + } else if (child->isRemoteFrame()) + return child->remoteFramePlatformElement().get(); return wrapper; }).autorelease(); diff --git a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm index 2f60b6204c49..bdab7c56ad98 100644 --- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm +++ b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm @@ -40,6 +40,7 @@ #import "AccessibilityList.h" #import "AccessibilityListBox.h" #import "AccessibilityProgressIndicator.h" +#import "AccessibilityRemoteFrame.h" #import "AccessibilityRenderObject.h" #import "AccessibilityScrollView.h" #import "AccessibilitySpinButton.h" @@ -479,6 +480,10 @@ using namespace WebCore; #define NSAccessibilityTextInputMarkedTextMarkerRangeAttribute @"AXTextInputMarkedTextMarkerRange" #endif +#ifndef kAXConvertRelativeFrameParameterizedAttribute +#define kAXConvertRelativeFrameParameterizedAttribute @"AXConvertRelativeFrame" +#endif + @implementation WebAccessibilityObjectWrapper - (void)detach @@ -1366,9 +1371,15 @@ static void WebTransformCGPathToNSBezierPath(void* info, const CGPathElement *el - (NSString*)role { + auto* backingObject = self.axBackingObject; + if (!backingObject) + return nil; + ALLOW_DEPRECATED_DECLARATIONS_BEGIN - if (self.axBackingObject->isAttachment()) + if (backingObject->isAttachment()) return [[self attachmentView] accessibilityAttributeValue:NSAccessibilityRoleAttribute]; + if (backingObject->isRemoteFrame()) + return [backingObject->remoteFramePlatformElement().get() accessibilityAttributeValue:NSAccessibilityRoleAttribute]; ALLOW_DEPRECATED_DECLARATIONS_END NSString *string = self.axBackingObject->rolePlatformString(); @@ -1388,6 +1399,9 @@ ALLOW_DEPRECATED_DECLARATIONS_END return false; #endif + if (backingObject->isRemoteFrame()) + return false; + return [[self role] isEqual:NSAccessibilityGroupRole] && backingObject->children().isEmpty() && ![[self renderWidgetChildren] count]; @@ -1421,6 +1435,8 @@ ALLOW_DEPRECATED_DECLARATIONS_BEGIN // attachments have the AXImage role, but a different subrole if (backingObject->isAttachment()) return [[self attachmentView] accessibilityAttributeValue:NSAccessibilityRoleDescriptionAttribute]; + if (backingObject->isRemoteFrame()) + return [backingObject->remoteFramePlatformElement().get() accessibilityAttributeValue:NSAccessibilityRoleDescriptionAttribute]; ALLOW_DEPRECATED_DECLARATIONS_END String roleDescription = backingObject->roleDescription(); @@ -1473,6 +1489,27 @@ ALLOW_DEPRECATED_DECLARATIONS_END return axScrollView ? [axScrollView->platformWidget() window] : nil; } +- (NSArray *)_transformSpecialChildrenCases:(RefPtr<AXCoreObject>)backingObject +{ +#if ENABLE(MODEL_ELEMENT) + if (backingObject->isModel()) { + auto modelChildren = backingObject->modelElementChildren(); + if (modelChildren.size()) { + return createNSArray(WTFMove(modelChildren), [] (auto&& child) -> id { + return child.get(); + }).autorelease(); + } + } +#endif + + if (!self.childrenVectorSize) { + if (NSArray *children = [self renderWidgetChildren]) + return children; + } + + return nil; +} + // FIXME: split up this function in a better way. // suggestions: Use a hash table that maps attribute names to function calls, // or maybe pointers to member functions @@ -1531,22 +1568,9 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END } if ([attributeName isEqualToString:NSAccessibilityChildrenAttribute] || [attributeName isEqualToString:NSAccessibilityChildrenInNavigationOrderAttribute]) { -#if ENABLE(MODEL_ELEMENT) - if (backingObject->isModel()) { - auto modelChildren = backingObject->modelElementChildren(); - if (modelChildren.size()) { - return createNSArray(WTFMove(modelChildren), [] (auto&& child) -> id { - return child.get(); - }).autorelease(); - } - } -#endif - - const auto& children = backingObject->children(); - if (children.isEmpty()) { - if (NSArray *widgetChildren = [self renderWidgetChildren]) - return widgetChildren; - } + NSArray *specialChildren = [self _transformSpecialChildrenCases:backingObject]; + if (specialChildren.count) + return specialChildren; // The tree's (AXOutline) children are supposed to be its rows and columns. // The ARIA spec doesn't have columns, so we just need rows. @@ -1713,16 +1737,14 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END if ([attributeName isEqualToString: NSAccessibilityEnabledAttribute]) return [NSNumber numberWithBool: backingObject->isEnabled()]; - if ([attributeName isEqualToString: NSAccessibilitySizeAttribute]) { - auto size = backingObject->size(); - return [NSValue valueWithSize:NSMakeSize(size.width(), size.height())]; - } + if ([attributeName isEqualToString: NSAccessibilitySizeAttribute]) AG: extra space after : + return [NSValue valueWithSize:(CGSize)backingObject->size()]; if ([attributeName isEqualToString:NSAccessibilityPrimaryScreenHeightAttribute]) return @(backingObject->primaryScreenRect().height()); if ([attributeName isEqualToString:NSAccessibilityPositionAttribute]) - return @(CGPoint(backingObject->screenRelativePosition())); + return [NSValue valueWithPoint:(CGPoint)backingObject->screenRelativePosition()]; if ([attributeName isEqualToString:NSAccessibilityPathAttribute]) return [self path]; @@ -2237,6 +2259,10 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END return range ? range.platformData().bridgingAutorelease() : nil; } + // VoiceOver property to ignore certain groups. + if ([attributeName isEqualToString:@"AXAutoInteractable"]) + return @(backingObject->isRemoteFrame()); + // Used by LayoutTests only, not by AT clients. if (UNLIKELY([attributeName isEqualToString:@"AXARIARole"])) return backingObject->computedRoleString(); @@ -2330,7 +2356,8 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END if (axObject->isAttachment()) { if (id attachmentView = [axObject->wrapper() attachmentView]) return attachmentView; - } + } else if (axObject->isRemoteFrame()) + return axObject->remoteFramePlatformElement().get(); // Only call out to the main-thread if this object has a backing widget to query. if (axObject->isWidget()) { @@ -2507,6 +2534,10 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END if (backingObject->isWebArea()) return webAreaParamAttrs; + // The object that serves up the remote frame also is the one that does the frame conversion. + if (backingObject->hasRemoteFrameChild()) + return [paramAttrs arrayByAddingObject:kAXConvertRelativeFrameParameterizedAttribute]; + return paramAttrs; } @@ -3177,6 +3208,18 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END return widgetChildren; criteria.resultsLimit -= [widgetChildren count]; } + } else if (backingObject->isRemoteFrame() + && criteria.searchKeys.contains(AccessibilitySearchKey::AnyType) + && (!criteria.visibleOnly || backingObject->isVisible())) { + NSArray *remoteFrameChildren = [self accessibilityAttributeValue:NSAccessibilityChildrenAttribute]; + ASSERT(remoteFrameChildren.count == 1); + if (remoteFrameChildren.count == 1) { + NSUInteger includedChildrenCount = std::min([remoteFrameChildren count], NSUInteger(criteria.resultsLimit)); + widgetChildren = [remoteFrameChildren subarrayWithRange:NSMakeRange(0, includedChildrenCount)]; + if ([widgetChildren count] >= criteria.resultsLimit) + return remoteFrameChildren; + criteria.resultsLimit -= [widgetChildren count]; + } } AccessibilityObject::AccessibilityChildrenVector results; @@ -3669,6 +3712,11 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END } } + if ([attribute isEqualToString:kAXConvertRelativeFrameParameterizedAttribute]) { + auto convertedRect = backingObject->parentObject()->convertFrameToSpace(FloatRect(rect), AccessibilityConversionSpace::Page); + return [NSValue valueWithRect:convertedRect]; + } + // There are some parameters that super handles that are not explicitly returned by the list of the element's attributes. // In that case it must be passed to super. return [super accessibilityAttributeValue:attribute forParameter:parameter]; @@ -3722,8 +3770,10 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END if (!child) continue; WebAccessibilityObjectWrapper *childWrapper = child->wrapper(); - if (childWrapper == targetChild || (child->isAttachment() && [childWrapper attachmentView] == targetChild)) + if (childWrapper == targetChild || (child->isAttachment() && [childWrapper attachmentView] == targetChild) + || (child->isRemoteFrame() && child->remoteFramePlatformElement() == targetChild)) { return i; + } } return NSNotFound; } @@ -3740,7 +3790,7 @@ ALLOW_DEPRECATED_DECLARATIONS_BEGIN if ([attribute isEqualToString:NSAccessibilityChildrenAttribute]) { // Tree items object returns a different set of children than those that are in children() // because an AXOutline (the mac role is becomes) has some odd stipulations. - if (backingObject->isTree() || backingObject->isTreeItem()) + if (backingObject->isTree() || backingObject->isTreeItem() || backingObject->isRemoteFrame()) return [[self accessibilityAttributeValue:NSAccessibilityChildrenAttribute] count]; auto childrenSize = backingObject->children().size(); @@ -3776,16 +3826,7 @@ ALLOW_DEPRECATED_DECLARATIONS_END if ([attribute isEqualToString:NSAccessibilityChildrenAttribute]) { if (backingObject->children().isEmpty()) { - NSArray *children = nil; -#if ENABLE(MODEL_ELEMENT) - if (backingObject->isModel()) { - children = createNSArray(backingObject->modelElementChildren(), [] (auto&& child) -> id { - return child.get(); - }).autorelease(); - } else -#endif - children = [self renderWidgetChildren]; - + NSArray *children = [self _transformSpecialChildrenCases:backingObject]; if (!children) return nil; @@ -3816,8 +3857,13 @@ ALLOW_DEPRECATED_DECLARATIONS_END // The attachment view should be returned, otherwise AX palindrome errors occur. id attachmentView = nil; - if ([wrapper isKindOfClass:[WebAccessibilityObjectWrapper class]] && wrapper.axBackingObject && wrapper.axBackingObject->isAttachment()) - attachmentView = [wrapper attachmentView]; + if ([wrapper isKindOfClass:[WebAccessibilityObjectWrapper class]] && wrapper.axBackingObject) { + if (wrapper.axBackingObject->isAttachment()) + attachmentView = [wrapper attachmentView]; + else if (wrapper.axBackingObject->isRemoteFrame()) + attachmentView = wrapper.axBackingObject->remoteFramePlatformElement().get(); + } + [subarray addObject:attachmentView ? attachmentView : wrapper]; }

Comment on attachment 469736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=469736&action=review > Source/WebCore/accessibility/AXCoreObject.h:1174 > + virtual IntPoint remoteFrameOffset() const = 0; Could we add a comment explaining this function? What is the offset relative to? > Source/WebCore/accessibility/AccessibilityObject.cpp:2106 > + if (auto* document = this->document()) { > + if (auto* frame = document->frame()) > + return frame->loader().client().accessibilityRemoteFrameOffset(); > + } > + > + return { }; This can be written a bit more simply: auto* document = this->document(); auto* frame = document ? document->frame() : nullptr; return frame ? frame->loader().client().accessibilityRemoteFrameOffset() : IntPoint(); > Source/WebCore/accessibility/AccessibilityRemoteFrame.cpp:51 > + if (auto* parent = parentObject()) > + return parent->elementRect(); > + > + return { }; auto* parent = parentObject(); return parent ? parent->elementRect() : LayoutRect(); > Source/WebCore/accessibility/AccessibilityRemoteFrame.h:53 > + bool computeAccessibilityIsIgnored() const override { return false; } > + bool isAccessibilityRemoteFrame() const override { return true; } > + LayoutRect elementRect() const override; Can these be final? > Source/WebCore/accessibility/AccessibilityScrollView.cpp:309 > + else if (is<RemoteFrameView>(m_scrollView)) > + owner = downcast<RemoteFrameView>(*m_scrollView).frame().ownerElement(); Can use dynamicDowncast here: else if (auto* remoteFrameView = dynamicDowncast<RemoteFrameView>(m_scrollView)) owner = remoteFrameView->frame().ownerElement(); > Source/WebCore/accessibility/AccessibilityScrollView.cpp:316 > + if (AXObjectCache* cache = axObjectCache()) { > + if (owner && owner->renderer()) > + return cache->getOrCreate(owner); > + } > > return nullptr; Can simplify this: auto* cache = axObjectCache(); return cache ? cache->getOrCreate(owner->renderer()) : nullptr; getOrCreate already bails early if the given renderer is nullptr, so this (existing before your patch) null check isn't necessary. > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1213 > + IntPoint remoteFrameOffset = { }; Can remove this line. > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1214 > + for (auto *isolatedObject = this; isolatedObject != nullptr; isolatedObject = isolatedObject->parentObject()) { * is in the wrong spot for C++, don't need the != nullptr bit: for (auto* isolatedObject = this; isolatedObject; isolatedObject = isolatedObject->parentObject()) > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1217 > + remoteFrameOffset = *point; > + break; return *point; > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1221 > + return remoteFrameOffset; return { }; > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:540 > + bool hasRemoteFrameChild() const override { return boolAttributeValue(AXPropertyName::HasRemoteFrameChild); } Can this be final? > Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:260 > + , RetainPtr<id> When you add a property here, you need to update the bool isDefaultValue = ... statement in AXIsolatedObject::setProperty to avoid wasting memory storing nil values. > Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:67 > + if (object->isRemoteFrame()) > + setProperty(AXPropertyName::RemoteFramePlatformElement, object->remoteFramePlatformElement()); Don't need to check object->isRemoteFrame(). setProperty is smart enough to not store nil values. > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:761 > + // TDOO: Support iOS Typo on TODO > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1374 > + auto* backingObject = self.axBackingObject; This should be a RefPtr instead of auto* to ensure the backingObject is not deleted from under us in the course of this method. > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1385 > NSString *string = self.axBackingObject->rolePlatformString(); Can we change this self.axBackingObject to use the backingObject you added in this patch, so that everything in this method operates off a consistent object? > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3716 > + auto convertedRect = backingObject->parentObject()->convertFrameToSpace(FloatRect(rect), AccessibilityConversionSpace::Page); Need to null-check the result of parentObject() here. > Source/WebCore/page/Page.cpp:2191 > + Document* mainDocument = nullptr; Don't need "= nullptr" here. > Source/WebCore/page/Page.cpp:2194 > + if (auto* localMainFrame = dynamicDowncast<LocalFrame>(mainFrame())) { > + mainDocument = localMainFrame ? localMainFrame->document() : nullptr; > + } else if (auto* remoteFrame = dynamicDowncast<RemoteFrame>(mainFrame())) { Don't need the curly-braces after the if and before the else-if. > Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp:1818 > + RefPtr webPage = m_frame->page(); > + if (!webPage) > + return { }; > + > + return webPage->accessibilityRemoteFrameOffset(); RefPtr webPage = m_frame->page(); return webPage ? webPage->accessibilityRemoteFrameOffset() : WebCore::IntPoint(); > Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp:106 > + RefPtr page = m_frame->page(); > + if (!page) > + return; > + > + // Make sure AppKit system knows about our remote UI process status now. > + page->accessibilityManageRemoteElementStatus(false, processIdentifier); if (RefPtr page = m_frame->page()) { // Make sure AppKit system knows about our remote UI process status now. page->accessibilityManageRemoteElementStatus(false, processIdentifier); } > Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp:115 > + RefPtr page = m_frame->page(); > + if (!page) > + return; > + > + page->send(Messages::WebPageProxy::UpdateRemoteFrameAccessibilityOffset(frameID, offset)); if (RefPtr page = m_frame->page()) page->send(Messages::WebPageProxy::UpdateRemoteFrameAccessibilityOffset(frameID, offset)); > Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm:68 > + if (auto* firstFrame = dynamicDowncast<WebCore::LocalFrame>(tree.firstChild())) > + return firstFrame->document()->axObjectCache(); Do we need to null check firstFrame->document()? auto* firstFrame = dynamicDowncast<WebCore::LocalFrame>(tree.firstChild()); auto* document = firstFrame ? firstFrame->document() : nullptr; return document ? document->axObjectCache() : nullptr;

Created attachment 469874 [details] Patch

Created attachment 469875 [details] Patch

Created attachment 469881 [details] Patch

Created attachment 469884 [details] Patch

Created attachment 469890 [details] Patch

Created attachment 469894 [details] Patch

Created attachment 469895 [details] Patch

(In reply to chris fleizach from comment #10) > Created attachment 469895 [details] > Patch diff --git a/Source/WebCore/accessibility/AXRemoteFrame.cpp b/Source/WebCore/accessibility/AXRemoteFrame.cpp new file mode 100644 index 000000000000..60434e344477 --- /dev/null +++ b/Source/WebCore/accessibility/AXRemoteFrame.cpp +AXRemoteFrame::AXRemoteFrame() +: +#if PLATFORM(COCOA) + m_remoteFramePlatformElement(nullptr) + , m_processIdentifier(0) +#endif +{ +} AG: I don't think you need this constructor at all since you have in the class declaration: RetainPtr<id> m_remoteFramePlatformElement; which is initialized to nullptr and for the pid, you can add an initializer in the declaration as well: pid_t m_processIdentifier { 0 };

diff --git a/Source/WebCore/accessibility/AXRemoteFrame.h b/Source/WebCore/accessibility/AXRemoteFrame.h new file mode 100644 index 000000000000..abc94d3aa6cf --- /dev/null +++ b/Source/WebCore/accessibility/AXRemoteFrame.h + +#pragma once + +#include "AccessibilityMockObject.h" + +namespace WebCore { + +class RemoteFrame; + +class AXRemoteFrame final : public AccessibilityMockObject { +public: + static Ref<AXRemoteFrame> create(); + +#if PLATFORM(COCOA) + void initializePlatformElementWithRemoteToken(std::span<const uint8_t>, int); + std::span<const uint8_t> generateRemoteToken() const; + RetainPtr<id> remoteFramePlatformElement() const final { return m_remoteFramePlatformElement; } AG: don't need final in individual method since the class is final. + pid_t processIdentifier() const { return m_processIdentifier; } +#endif + +private: + explicit AXRemoteFrame(); + virtual ~AXRemoteFrame() = default; + + AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::RemoteFrame; } + bool computeAccessibilityIsIgnored() const final { return false; } + bool isAXRemoteFrame() const final { return true; } + LayoutRect elementRect() const final; AG: same in all of these. + +#if PLATFORM(COCOA) + RetainPtr<id> m_remoteFramePlatformElement; + pid_t m_processIdentifier; AG: initializer { 0 } +#endif +}; + +} // namespace WebCore + +SPECIALIZE_TYPE_TRAITS_ACCESSIBILITY(AXRemoteFrame, isAXRemoteFrame())

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp index 3e32db093afb..09842664328e 100644 --- a/Source/WebCore/accessibility/AccessibilityObject.cpp +++ b/Source/WebCore/accessibility/AccessibilityObject.cpp @@ -544,7 +544,10 @@ FloatRect AccessibilityObject::convertFrameToSpace(const FloatRect& frameRect, A FloatRect AccessibilityObject::relativeFrame() const { - return convertFrameToSpace(elementRect(), AccessibilityConversionSpace::Page); + auto rect = elementRect(); + auto remoteOffset = remoteFrameOffset(); + rect.move(remoteOffset.x(), remoteOffset.y()); AG: I think the last two lines can be more concise as: rect.moveBy(remoteFrameOffset()); + return convertFrameToSpace(rect, AccessibilityConversionSpace::Page); }

diff --git a/Source/WebCore/accessibility/AccessibilityScrollView.cpp b/Source/WebCore/accessibility/AccessibilityScrollView.cpp index 2bbc3087b9f8..9ba269be3960 100644 --- a/Source/WebCore/accessibility/AccessibilityScrollView.cpp +++ b/Source/WebCore/accessibility/AccessibilityScrollView.cpp @@ -27,10 +27,12 @@ #include "AccessibilityScrollView.h" #include "AXObjectCache.h" +#include "AXRemoteFrame.h" #include "AccessibilityScrollbar.h" #include "HTMLFrameOwnerElement.h" #include "LocalFrame.h" #include "LocalFrameView.h" +#include "RemoteFrameView.h" #include "RenderElement.h" #include "Widget.h" @@ -52,6 +54,14 @@ AccessibilityScrollView::~AccessibilityScrollView() void AccessibilityScrollView::detachRemoteParts(AccessibilityDetachmentType detachmentType) { AccessibilityObject::detachRemoteParts(detachmentType); + + auto* remoteFrameView = dynamicDowncast<RemoteFrameView>(m_scrollView.get()); + if (remoteFrameView && m_remoteFrame && (detachmentType == AccessibilityDetachmentType::ElementDestroyed || detachmentType == AccessibilityDetachmentType::CacheDestroyed)) { + auto& remoteFrame = remoteFrameView->frame(); + remoteFrame.unbindRemoteAccessibilityFrames(m_remoteFrame->processIdentifier()); + m_remoteFrame = nullptr; + } + m_scrollView = nullptr; m_frameOwnerElement = nullptr; } @@ -177,8 +187,10 @@ AccessibilityScrollbar* AccessibilityScrollView::addChildScrollbar(Scrollbar* sc void AccessibilityScrollView::clearChildren() { AccessibilityObject::clearChildren(); + m_verticalScrollbar = nullptr; m_horizontalScrollbar = nullptr; + m_childrenDirty = false; } @@ -191,11 +203,43 @@ bool AccessibilityScrollView::computeAccessibilityIsIgnored() const return webArea->accessibilityIsIgnored(); } +void AccessibilityScrollView::addRemoteFrameChild() +{ + if (!is<RemoteFrameView>(m_scrollView)) AG: I think you have to do m_scrollView.get() because it is a SingleThreadWeakPtr<ScrollView>, a smart pointer, and not usre that is<T> looks inside the smart pointer. + return; + WeakPtr cache = axObjectCache(); + if (!cache) + return; + + if (!m_remoteFrame) { + // Make the faux element that represents the remote transfer element for AX. + auto& remoteFrame = downcast<RemoteFrameView>(*m_scrollView).frame(); + m_remoteFrame = downcast<AXRemoteFrame>(cache->create(AccessibilityRole::RemoteFrame)); AG: we should avoid downcast<T>, but instead dynamicDowncast or uncheckedDowncast when sure of the type. + m_remoteFrame->setParent(this); + +#if PLATFORM(MAC) + // Generate a new token and pass it back to the other remote frame so it can bind these objects together. + auto generatedToken = m_remoteFrame->generateRemoteToken(); + remoteFrame.bindRemoteAccessibilityFrames(getpid(), generatedToken, [this, &remoteFrame, protectedAccessbilityRemoteFrame = RefPtr { m_remoteFrame }] (std::span<const uint8_t> token, int processIdentifier) mutable { + protectedAccessbilityRemoteFrame->initializePlatformElementWithRemoteToken(token, processIdentifier); + + // Update the remote side with the offset of this object so it can calculate frames correctly. + auto location = elementRect().location(); + remoteFrame.updateRemoteFrameAccessibilityOffset(flooredIntPoint(location)); + }); +#endif + } else + m_remoteFrame->setParent(this); + + addChild(m_remoteFrame.get()); +} + void AccessibilityScrollView::addChildren() { ASSERT(!m_childrenInitialized); m_childrenInitialized = true; + addRemoteFrameChild(); addChild(webAreaObject()); updateScrollbars(); } @@ -203,7 +247,7 @@ void AccessibilityScrollView::addChildren() AccessibilityObject* AccessibilityScrollView::webAreaObject() const { auto* document = this->document(); - if (!document || !document->hasLivingRenderTree()) + if (!document || !document->hasLivingRenderTree() || m_remoteFrame) return nullptr; if (auto* cache = axObjectCache()) @@ -234,9 +278,15 @@ LayoutRect AccessibilityScrollView::elementRect() const Document* AccessibilityScrollView::document() const { - if (auto* frameView = dynamicDowncast<LocalFrameView>(m_scrollView.get())) { + if (auto* frameView = dynamicDowncast<LocalFrameView>(m_scrollView.get())) return frameView->frame().document(); + + // For the RemoteFrameView case, we need to return the document of our hosting parent so axObjectCache() resolves correctly. + if (auto* remoteFrameView = dynamicDowncast<RemoteFrameView>(m_scrollView.get())) { + if (auto* owner = remoteFrameView->frame().ownerElement()) + return &(owner->document()); } + return AccessibilityObject::document(); } @@ -259,8 +309,10 @@ AccessibilityObject* AccessibilityScrollView::parentObject() const WeakPtr owner = m_frameOwnerElement.get(); if (auto* localFrameView = dynamicDowncast<LocalFrameView>(m_scrollView.get())) owner = localFrameView->frame().ownerElement(); + else if (auto* remoteFrameView = dynamicDowncast<RemoteFrameView>(m_scrollView.get())) + owner = remoteFrameView->frame().ownerElement(); - if (cache && owner && owner->renderer()) + if (owner && owner->renderer()) return cache->getOrCreate(owner.get()); return nullptr; }

Created attachment 469897 [details] Patch

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp index 1014546278f3..177d9397a0a7 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp @@ -378,6 +378,10 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb #endif setObjectProperty(AXPropertyName::InternalLinkElement, object.internalLinkElement()); + setProperty(AXPropertyName::HasRemoteFrameChild, object.hasRemoteFrameChild()); AG: this seems to be possibly true only for AXScrollViews, if that is correct, should we set this property only for AXScrollViews? + // Don't duplicate the remoteFrameOffset for every object. Just store in the WebArea. + if (object.isWebArea()) + setProperty(AXPropertyName::RemoteFrameOffset, object.remoteFrameOffset()); initializePlatformProperties(axObject); } @@ -464,6 +468,7 @@ void AXIsolatedObject::setProperty(AXPropertyName propertyName, AXPropertyValueV [](OptionSet<AXAncestorFlag>& typedValue) { return typedValue.isEmpty(); }, #if PLATFORM(COCOA) [](RetainPtr<NSAttributedString>& typedValue) { return !typedValue; }, + [](RetainPtr<id>& typedValue) { return !typedValue; }, #endif [](InsideLink& typedValue) { return typedValue == InsideLink(); }, [](Vector<Vector<AXID>>& typedValue) { return typedValue.isEmpty(); }, @@ -1206,6 +1211,21 @@ LayoutRect AXIsolatedObject::elementRect() const }); } +IntPoint AXIsolatedObject::remoteFrameOffset() const +{ + auto* webArea = Accessibility::findAncestor<AXIsolatedObject>(*this, false, [] (const auto& object) { + return object.isWebArea(); + }); AG: shouldn't findAncestor take true here to include this? + + if (!webArea) + return { }; + + if (auto point = webArea->optionalAttributeValue<IntPoint>(AXPropertyName::RemoteFrameOffset)) + return *point; + + return { }; +} + FloatPoint AXIsolatedObject::screenRelativePosition() const { if (auto point = optionalAttributeValue<FloatPoint>(AXPropertyName::ScreenRelativePosition)) @@ -1229,22 +1249,23 @@ FloatPoint AXIsolatedObject::screenRelativePosition() const FloatRect AXIsolatedObject::relativeFrame() const { + FloatRect relativeFrame; + if (auto cachedRelativeFrame = optionalAttributeValue<IntRect>(AXPropertyName::RelativeFrame)) { // We should not have cached a relative frame for elements that get their geometry from their children. ASSERT(!m_getsGeometryFromChildren); - return *cachedRelativeFrame; - } - - if (m_getsGeometryFromChildren) { + relativeFrame = *cachedRelativeFrame; + } else if (m_getsGeometryFromChildren) { auto frame = enclosingIntRect(relativeFrameFromChildren()); if (!frame.isEmpty()) - return frame; + relativeFrame = frame; // Either we had no children, or our children had empty frames. The right thing to do would be to return // a rect at the position of the nearest render tree ancestor with some made-up size (AccessibilityNodeObject::boundingBoxRect does this). // However, we don't have access to the render tree in this context (only the AX isolated tree, which is too sparse for this purpose), so // until we cache the necessary information let's go to the main-thread. } else if (roleValue() == AccessibilityRole::Column || roleValue() == AccessibilityRole::TableHeaderContainer) - return exposedTableAncestor() ? relativeFrameFromChildren() : FloatRect(); + relativeFrame = exposedTableAncestor() ? relativeFrameFromChildren() : FloatRect(); + // Mock objects and SVG objects need use the main thread since they do not have render nodes and are not painted with layers, respectively. // FIXME: Remove isNonLayerSVGObject when LBSE is enabled & SVG frames are cached. @@ -1266,6 +1287,8 @@ FloatRect AXIsolatedObject::relativeFrame() const if (ancestor && frameRect.location() == FloatPoint()) frameRect.setLocation(ancestor->relativeFrame().location()); + auto remoteOffset = remoteFrameOffset(); + frameRect.moveBy(FloatPoint(remoteOffset)); AG: frameRect.moveBy(remoteFrameOffset()) or frameRect.moveBy({ remoteFrameOffset() }) return frameRect; }

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h index d2a66c2f53e4..de827d5fe016 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h @@ -28,6 +28,7 @@ #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) #include "AXCoreObject.h" +#include "AXIsolatedTree.h" #include "AXObjectCache.h" #include "IntPoint.h" #include "LayoutRect.h" @@ -215,6 +216,7 @@ private: bool isFileUploadButton() const final { return boolAttributeValue(AXPropertyName::IsFileUploadButton); } bool isMeter() const final { return boolAttributeValue(AXPropertyName::IsMeter); }; FloatPoint screenRelativePosition() const final; + IntPoint remoteFrameOffset() const final; FloatRect relativeFrame() const final; bool hasCachedRelativeFrame() const { return optionalAttributeValue<IntRect>(AXPropertyName::RelativeFrame).has_value(); } #if PLATFORM(MAC) @@ -263,7 +265,7 @@ private: AXIsolatedObject* accessibilityHitTest(const IntPoint&) const final; AXIsolatedObject* focusedUIElement() const final; AXCoreObject* internalLinkElement() const final { return objectAttributeValue(AXPropertyName::InternalLinkElement); } - AccessibilityChildrenVector radioButtonGroup() const { return tree()->objectsForIDs(vectorAttributeValue<AXID>(AXPropertyName::RadioButtonGroup)); } + AccessibilityChildrenVector radioButtonGroup() const final { return tree()->objectsForIDs(vectorAttributeValue<AXID>(AXPropertyName::RadioButtonGroup)); } AXIsolatedObject* scrollBar(AccessibilityOrientation) final; const String placeholderValue() const final { return stringAttributeValue(AXPropertyName::PlaceholderValue); } String expandedTextValue() const final { return stringAttributeValue(AXPropertyName::ExpandedTextValue); } @@ -456,7 +458,7 @@ private: bool isAccessibilityARIAGridInstance() const final { return false; } bool isAccessibilityARIAGridRowInstance() const final { return false; } bool isAccessibilityARIAGridCellInstance() const final { return false; } - + bool isAXRemoteFrame() const final { return false; } AG: We don't need this method in the AXCoreObject interface and thus don't need to be overridden here. bool isNativeTextControl() const final; bool isListBoxOption() const final; bool isMockObject() const final; @@ -537,7 +539,9 @@ private: String nameAttribute() const final { return stringAttributeValue(AXPropertyName::NameAttribute); } #if PLATFORM(COCOA) bool hasApplePDFAnnotationAttribute() const final { return boolAttributeValue(AXPropertyName::HasApplePDFAnnotationAttribute); } + RetainPtr<id> remoteFramePlatformElement() const final; #endif + bool hasRemoteFrameChild() const final { return boolAttributeValue(AXPropertyName::HasRemoteFrameChild); } #if PLATFORM(COCOA) && ENABLE(MODEL_ELEMENT) Vector<RetainPtr<id>> modelElementChildren() final;

> + bool isAXRemoteFrame() const final { return false; } > > AG: We don't need this method in the AXCoreObject interface and thus don't > need to be overridden here. > It's pure virtual on AXCoreObject, so it needs to be implemented here

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp index 696658448f65..1be3b7990539 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp @@ -30,6 +30,7 @@ #include "AXIsolatedObject.h" #include "AXLogger.h" +#include "AccessibilityTable.h" #include "AccessibilityTableRow.h" #include "FrameSelection.h" #include "LocalFrameView.h" @@ -225,46 +226,26 @@ RefPtr<AXIsolatedObject> AXIsolatedTree::objectForID(const AXID axID) const return axID.isValid() ? m_readerThreadNodeMap.get(axID) : nullptr; } -template<typename U> -Vector<RefPtr<AXCoreObject>> AXIsolatedTree::objectsForIDs(const U& axIDs) +RefPtr<AXIsolatedObject> AXIsolatedTree::fetchObjectForIDFromMainThread(const AXID axID) const { - ASSERT(!isMainThread()); - - Vector<RefPtr<AXCoreObject>> result; - result.reserveInitialCapacity(axIDs.size()); - for (const auto& axID : axIDs) { - RefPtr object = objectForID(axID); - if (object) { - result.append(WTFMove(object)); - continue; - } - - // There is no isolated object for this AXID. This can happen if the corresponding live object is ignored. - // If there is a live object for this ID and it is an ignored target of a relationship, create an isolated object for it. - object = Accessibility::retrieveValueFromMainThread<RefPtr<AXIsolatedObject>>([&axID, this] () -> RefPtr<AXIsolatedObject> { - auto* cache = axObjectCache(); - if (!cache || !cache->relationTargetIDs().contains(axID)) - return nullptr; - - RefPtr axObject = cache->objectForID(axID); - if (!axObject || !axObject->accessibilityIsIgnored()) - return nullptr; - - auto object = AXIsolatedObject::create(*axObject, const_cast<AXIsolatedTree*>(this)); - ASSERT(axObject->wrapper()); - object->attachPlatformWrapper(axObject->wrapper()); - return object; - }); - if (object) { - m_readerThreadNodeMap.add(axID, *object); - result.append(WTFMove(object)); - } - } - result.shrinkToFit(); - return result; + // There is no isolated object for this AXID. This can happen if the corresponding live object is ignored. + // If there is a live object for this ID and it is an ignored target of a relationship, create an isolated object for it. + return Accessibility::retrieveValueFromMainThread<RefPtr<AXIsolatedObject>>([&axID, this] () -> RefPtr<AXIsolatedObject> { + auto* cache = axObjectCache(); + if (!cache || !cache->relationTargetIDs().contains(axID)) + return nullptr; + + RefPtr axObject = cache->objectForID(axID); + if (!axObject || !axObject->accessibilityIsIgnored()) + return nullptr; + + auto object = AXIsolatedObject::create(*axObject, const_cast<AXIsolatedTree*>(this)); + ASSERT(axObject->wrapper()); + object->attachPlatformWrapper(axObject->wrapper()); + return object; AG: we should use addUnconnectedNode instead. + }); } - void AXIsolatedTree::generateSubtree(AccessibilityObject& axObject) { AXTRACE("AXIsolatedTree::generateSubtree"_s); @@ -660,6 +641,12 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const AXProper case AXPropertyName::PosInSet: propertyMap.set(AXPropertyName::PosInSet, axObject.posInSet()); break; + case AXPropertyName::RemoteFramePlatformElement: + propertyMap.set(AXPropertyName::RemoteFramePlatformElement, axObject.remoteFramePlatformElement()); + break; + case AXPropertyName::HasRemoteFrameChild: + propertyMap.set(AXPropertyName::HasRemoteFrameChild, axObject.hasRemoteFrameChild()); + break; case AXPropertyName::RoleDescription: propertyMap.set(AXPropertyName::RoleDescription, axObject.roleDescription().isolatedCopy()); break;

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h index a417927978d2..6b4c5e8331a3 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h @@ -104,6 +104,7 @@ enum class AXPropertyName : uint16_t { HasHighlighting, HasItalicFont, HasPlainText, + HasRemoteFrameChild, HasUnderline, HeaderContainer, HeadingLevel, @@ -201,6 +202,8 @@ enum class AXPropertyName : uint16_t { PreventKeyboardDOMEventDispatch, RadioButtonGroup, RelativeFrame, + RemoteFrameOffset, + RemoteFramePlatformElement, RoleValue, RolePlatformString, RoleDescription, @@ -257,6 +260,7 @@ using AXPropertyNameSet = HashSet<AXPropertyName, IntHash<AXPropertyName>, WTF:: using AXPropertyValueVariant = std::variant<std::nullptr_t, AXID, String, bool, int, unsigned, double, float, uint64_t, AccessibilityButtonState, Color, URL, LayoutRect, FloatPoint, FloatRect, IntPoint, IntRect, std::pair<unsigned, unsigned>, Vector<AccessibilityText>, Vector<AXID>, Vector<std::pair<AXID, AXID>>, Vector<String>, Path, OptionSet<AXAncestorFlag>, InsideLink, Vector<Vector<AXID>>, CharacterRange, std::pair<AXID, CharacterRange> #if PLATFORM(COCOA) , RetainPtr<NSAttributedString> + , RetainPtr<id> #endif #if ENABLE(AX_THREAD_TEXT_APIS) , AXTextRuns @@ -397,6 +401,7 @@ private: void createEmptyContent(AccessibilityObject&); constexpr bool isUpdatingSubtree() const { return m_rootOfSubtreeBeingUpdated; } constexpr void updatingSubtree(AccessibilityObject* axObject) { m_rootOfSubtreeBeingUpdated = axObject; } + RefPtr<AXIsolatedObject> fetchObjectForIDFromMainThread(const AXID) const; AG: we have used retrieve instead of fetch got get things for the main thread. I'm fine with either one but we should use one. enum class AttachWrapper : bool { OnMainThread, OnAXThread }; struct NodeChange { @@ -497,6 +502,30 @@ inline RefPtr<AXIsolatedTree> AXIsolatedTree::treeForPageID(std::optional<PageId return pageID ? treeForPageID(*pageID) : nullptr; } +template<typename U> +inline Vector<RefPtr<AXCoreObject>> AXIsolatedTree::objectsForIDs(const U& axIDs) +{ + ASSERT(!isMainThread()); + + Vector<RefPtr<AXCoreObject>> result; + result.reserveInitialCapacity(axIDs.size()); + for (const auto& axID : axIDs) { + RefPtr object = objectForID(axID); + if (object) { + result.append(WTFMove(object)); + continue; + } + + object = fetchObjectForIDFromMainThread(axID); + if (object) { + m_readerThreadNodeMap.add(axID, *object); + result.append(WTFMove(object)); + } + } + result.shrinkToFit(); + return result; +} + } // namespace WebCore #endif

+ auto object = AXIsolatedObject::create(*axObject, const_cast<AXIsolatedTree*>(this)); + ASSERT(axObject->wrapper()); + object->attachPlatformWrapper(axObject->wrapper()); + return object; AG: we should use addUnconnectedNode instead. This is from existing code, not changing it in this PR

diff --git a/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm b/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm index ba2accd80d04..7c7a4f04957c 100644 --- a/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm +++ b/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm @@ -26,6 +26,7 @@ #import "config.h" #import "AccessibilityObject.h" +#import "AXRemoteFrame.h" #import "AccessibilityLabel.h" #import "AccessibilityList.h" #import "ColorCocoa.h" @@ -51,6 +52,7 @@ #import "WebAccessibilityObjectWrapperMac.h" #import "Widget.h" +#import <pal/spi/cocoa/NSAccessibilitySPI.h> #import <pal/spi/mac/NSSpellCheckerSPI.h> namespace WebCore { @@ -749,6 +751,24 @@ RetainPtr<NSAttributedString> attributedStringCreate(Node* node, StringView text return result; } +std::span<const uint8_t> AXRemoteFrame::generateRemoteToken() const +{ + // We use the parent's wrapper so that the remote frame acts as a pass through for the remote token bridge. + NSData *data = [NSAccessibilityRemoteUIElement remoteTokenForLocalUIElement:parentObject()->wrapper()]; AG: parentObject() can be null. + return std::span(static_cast<const uint8_t*>([data bytes]), [data length]); +} + +void AXRemoteFrame::initializePlatformElementWithRemoteToken(std::span<const uint8_t> token, int processIdentifier) +{ + m_processIdentifier = processIdentifier; + if ([wrapper() respondsToSelector:@selector(accessibilitySetPresenterProcessIdentifier:)]) + [(id)wrapper() accessibilitySetPresenterProcessIdentifier:processIdentifier]; AG: do you need to cast the wrapper to (id) here? + NSData *tokenData = [NSData dataWithBytes:token.data() length:token.size()]; + m_remoteFramePlatformElement = adoptNS([[NSAccessibilityRemoteUIElement alloc] initWithRemoteToken:tokenData]); + + axObjectCache()->onRemoteFrameInitialized(*this); AG: axObjectCache() can be null. +} + namespace Accessibility { PlatformRoleMap createPlatformRoleMap() @@ -891,6 +911,7 @@ PlatformRoleMap createPlatformRoleMap() { AccessibilityRole::Superscript, NSAccessibilityGroupRole }, { AccessibilityRole::Model, NSAccessibilityGroupRole }, { AccessibilityRole::Suggestion, NSAccessibilityGroupRole }, + { AccessibilityRole::RemoteFrame, NSAccessibilityGroupRole }, }; PlatformRoleMap roleMap; for (auto& role : roles)

+ m_processIdentifier = processIdentifier; + if ([wrapper() respondsToSelector:@selector(accessibilitySetPresenterProcessIdentifier:)]) + [(id)wrapper() accessibilitySetPresenterProcessIdentifier:processIdentifier]; AG: do you need to cast the wrapper to (id) here? Yes - this is only defined on NSAccessibilityRemoteUIElement in AppKit for some reason

diff --git a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm index 6357fa7c271b..f14245fc3b67 100644 --- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm +++ b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm @@ -32,6 +32,7 @@ #import "AXCoreObject.h" #import "AXIsolatedObject.h" #import "AXObjectCache.h" +#import "AXRemoteFrame.h" #import "AccessibilityARIAGridRow.h" #import "AccessibilityList.h" #import "AccessibilityListBox.h" @@ -265,13 +266,13 @@ NSArray *makeNSArray(const WebCore::AXCoreObject::AccessibilityChildrenVector& c return nil; auto wrapper = child->wrapper(); - // We want to return the attachment view instead of the object representing the attachment, // otherwise, we get palindrome errors in the AX hierarchy. if (child->isAttachment()) { if (id attachmentView = wrapper.attachmentView) return attachmentView; - } + } else if (child->isRemoteFrame()) + return child->remoteFramePlatformElement().get(); return wrapper; }).autorelease(); @@ -389,7 +390,7 @@ NSArray *makeNSArray(const WebCore::AXCoreObject::AccessibilityChildrenVector& c - (BOOL)isIsolatedObject { #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) - auto* backingObject = self.axBackingObject; + RefPtr backingObject = dynamicDowncast<AccessibilityObject>(self.baseUpdateBackingStore); return backingObject && backingObject->isAXIsolatedObjectInstance(); AG: I think this can be just: return dynamicDowncast<AXIsolatedObject>(self.baseUpdateBackingStore); and I don't know if this is actuallly used any where any more. #else return NO; @@ -523,7 +524,7 @@ static void convertPathToScreenSpaceFunction(PathConversionInfo& conversion, con - (CGRect)convertRectToSpace:(const WebCore::FloatRect&)rect space:(AccessibilityConversionSpace)space { - auto* backingObject = self.axBackingObject; + RefPtr backingObject = dynamicDowncast<AccessibilityObject>(self.baseUpdateBackingStore); if (!backingObject) return CGRectZero;

(In reply to chris fleizach from comment #18) > > + bool isAXRemoteFrame() const final { return false; } > > > > AG: We don't need this method in the AXCoreObject interface and thus don't > > need to be overridden here. > > > > It's pure virtual on AXCoreObject, so it needs to be implemented here AG: you can make it a virtual in AccessibilityObject and remove it from the AXCoreObject class.

(In reply to Andres Gonzalez from comment #25) > (In reply to chris fleizach from comment #18) > > > + bool isAXRemoteFrame() const final { return false; } > > > > > > AG: We don't need this method in the AXCoreObject interface and thus don't > > > need to be overridden here. > > > > > > > It's pure virtual on AXCoreObject, so it needs to be implemented here > > AG: you can make it a virtual in AccessibilityObject and remove it from the > AXCoreObject class. Then we can't use SPECIALIZE_TYPE_TRAITS_ACCESSIBILITY or downcast<>

Created attachment 469902 [details] Patch

Created attachment 469907 [details] Patch

Created attachment 469912 [details] Patch

Created attachment 469914 [details] Patch

Created attachment 469915 [details] Patch

Created attachment 469922 [details] Patch

Created attachment 469926 [details] Patch

diff --git a/Source/WebCore/loader/EmptyClients.cpp b/Source/WebCore/loader/EmptyClients.cpp index 258c14b86a8b..5717ade2a452 100644 --- a/Source/WebCore/loader/EmptyClients.cpp +++ b/Source/WebCore/loader/EmptyClients.cpp @@ -1085,6 +1085,11 @@ RemoteAXObjectRef EmptyFrameLoaderClient::accessibilityRemoteObject() return nullptr; } +IntPoint EmptyFrameLoaderClient::accessibilityRemoteFrameOffset() +{ + return { }; +} AG: don't know about what is customery in this file but we tend to put this trivial definition of a method right on the header in one line. + #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) void EmptyFrameLoaderClient::setAXIsolatedTreeRoot(WebCore::AXCoreObject*) {

diff --git a/Source/WebCore/page/Page.cpp b/Source/WebCore/page/Page.cpp index 5d2feab65796..b4c252e02362 100644 --- a/Source/WebCore/page/Page.cpp +++ b/Source/WebCore/page/Page.cpp @@ -2208,11 +2208,18 @@ bool Page::shouldUpdateAccessibilityRegions() const ASSERT(m_lastRenderingUpdateTimestamp >= m_lastAccessibilityObjectRegionsUpdate); if ((m_lastRenderingUpdateTimestamp - m_lastAccessibilityObjectRegionsUpdate) < updateInterval) { // We've already updated accessibility object rects recently, so skip this update and schedule another for later. - auto* localMainFrame = dynamicDowncast<LocalFrame>(mainFrame()); - RefPtr mainDocument = localMainFrame ? localMainFrame->document() : nullptr; + + RefPtr<Document> protectedMainDocument = nullptr; AG: don't need = nullptr, RefPtr constructor does it for you. + if (auto* localMainFrame = dynamicDowncast<LocalFrame>(mainFrame())) + protectedMainDocument = localMainFrame ? localMainFrame->document() : nullptr; + else if (auto* remoteFrame = dynamicDowncast<RemoteFrame>(mainFrame())) { + if (auto* owner = remoteFrame->ownerElement()) + protectedMainDocument = &(owner->document()); + } + // If accessibility is enabled and we have a main document, that document should have an AX object cache. - ASSERT(!mainDocument || mainDocument->existingAXObjectCache()); - if (CheckedPtr topAxObjectCache = mainDocument ? mainDocument->existingAXObjectCache() : nullptr) + ASSERT(!protectedMainDocument || protectedMainDocument->existingAXObjectCache()); + if (CheckedPtr topAxObjectCache = protectedMainDocument ? protectedMainDocument->existingAXObjectCache() : nullptr) topAxObjectCache->scheduleObjectRegionsUpdate(); return false; }

> > +IntPoint EmptyFrameLoaderClient::accessibilityRemoteFrameOffset() > +{ > + return { }; > +} > > AG: don't know about what is customery in this file but we tend to put this > trivial definition of a method right on the header in one line. > It looks like everything else in EmptyFrameLoader puts their empty definitions in the cpp, so I should probably stick with that

Created attachment 469936 [details] Patch

Created attachment 469946 [details] Patch

diff --git a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm index 5b10f59c6661..deb4d58e5fd6 100644 --- a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm +++ b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm @@ -763,11 +763,10 @@ void WebPageProxy::setSmartInsertDeleteEnabled(bool) notImplemented(); } -void WebPageProxy::registerWebProcessAccessibilityToken(std::span<const uint8_t> data) +void WebPageProxy::registerWebProcessAccessibilityToken(std::span<const uint8_t> data, FrameIdentifier frameID) { - protectedPageClient()->accessibilityWebProcessTokenReceived(data); -} - + pageClient().accessibilityWebProcessTokenReceived(data, frameID, messageSenderConnection()->remoteProcessID()); AG: is messageSenderConnection() guaranteed to be not null? +} void WebPageProxy::relayAccessibilityNotification(const String& notificationName, std::span<const uint8_t> data) {

diff --git a/Source/WebKit/UIProcess/mac/WebViewImpl.mm b/Source/WebKit/UIProcess/mac/WebViewImpl.mm index cc8d3c7ff0c6..7f35772ab4e8 100644 --- a/Source/WebKit/UIProcess/mac/WebViewImpl.mm +++ b/Source/WebKit/UIProcess/mac/WebViewImpl.mm @@ -3479,10 +3479,12 @@ void WebViewImpl::setIgnoresMouseDraggedEvents(bool ignoresMouseDraggedEvents) m_ignoresMouseDraggedEvents = ignoresMouseDraggedEvents; } -void WebViewImpl::setAccessibilityWebProcessToken(NSData *data) +void WebViewImpl::setAccessibilityWebProcessToken(NSData *data, WebCore::FrameIdentifier frameID, pid_t pid) { - m_remoteAccessibilityChild = data.length ? adoptNS([[NSAccessibilityRemoteUIElement alloc] initWithRemoteToken:data]) : nil; - updateRemoteAccessibilityRegistration(true); + if (pid == m_page->process().processID()) { AG: m_page null check? + m_remoteAccessibilityChild = data.length ? adoptNS([[NSAccessibilityRemoteUIElement alloc] initWithRemoteToken:data]) : nil; + updateRemoteAccessibilityRegistration(true); + } } void WebViewImpl::updateRemoteAccessibilityRegistration(bool registerProcess)

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp index 35987a1f85e4..ecddea9f99d6 100644 --- a/Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp +++ b/Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp @@ -1806,6 +1806,12 @@ void WebLocalFrameLoaderClient::dispatchWillDestroyGlobalObjectForDOMWindowExten #if PLATFORM(COCOA) +WebCore::IntPoint WebLocalFrameLoaderClient::accessibilityRemoteFrameOffset() +{ + RefPtr webPage = m_frame->page(); AG: m_frame null check? + return webPage ? webPage->accessibilityRemoteFrameOffset() : IntPoint(); +} + RemoteAXObjectRef WebLocalFrameLoaderClient::accessibilityRemoteObject() { RefPtr webPage = m_frame->page();

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp index 38b0f325aa16..d4df7ef529a2 100644 --- a/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp +++ b/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp @@ -96,6 +96,44 @@ String WebRemoteFrameClient::renderTreeAsText(size_t baseIndent, OptionSet<Rende return result; } +void WebRemoteFrameClient::unbindRemoteAccessibilityFrames(int processIdentifier) +{ +#if PLATFORM(COCOA) + // Make sure AppKit system knows about our remote UI process status now. + if (RefPtr page = m_frame->page()) AG: m_frame null check? + page->accessibilityManageRemoteElementStatus(false, processIdentifier); +#else + UNUSED_PARAM(processIdentifier); +#endif +} + +void WebRemoteFrameClient::updateRemoteFrameAccessibilityOffset(WebCore::FrameIdentifier frameID, WebCore::IntPoint offset) +{ + if (RefPtr page = m_frame->page()) AG: dito? + page->send(Messages::WebPageProxy::UpdateRemoteFrameAccessibilityOffset(frameID, offset)); +} + +void WebRemoteFrameClient::bindRemoteAccessibilityFrames(int processIdentifier, WebCore::FrameIdentifier frameID, const std::span<const uint8_t> dataToken, CompletionHandler<void(std::span<const uint8_t>, int)>&& completionHandler) +{ + RefPtr page = m_frame->page(); AG: dito? + if (!page) { + completionHandler(std::span<const uint8_t> { }, 0); AG: do you need std::span<const uint8_t> cause the compiler cannot infer the type? + return; + } + + auto sendResult = page->sendSync(Messages::WebPageProxy::BindRemoteAccessibilityFrames(processIdentifier, frameID, dataToken)); + if (!sendResult.succeeded()) + return; AG: not calling the completion handler? + + auto [resultToken, processIdentifierResult] = sendResult.takeReply(); + + // Make sure AppKit system knows about our remote UI process status now. +#if PLATFORM(COCOA) + page->accessibilityManageRemoteElementStatus(true, processIdentifierResult); +#endif + completionHandler(resultToken, processIdentifierResult); +} + void WebRemoteFrameClient::broadcastFrameRemovalToOtherProcesses() { WebFrameLoaderClient::broadcastFrameRemovalToOtherProcesses();

> AG: m_frame null check? > > + return webPage ? webPage->accessibilityRemoteFrameOffset() : IntPoint(); > +} > + > RemoteAXObjectRef WebLocalFrameLoaderClient::accessibilityRemoteObject() > { > RefPtr webPage = m_frame->page(); No other functions are checking m_frame in this file, so I think it's ok

Created attachment 469948 [details] Patch

diff --git a/Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm b/Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm index cefe2e2345ab..d4e7c6b5dcaf 100644 --- a/Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm +++ b/Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm @@ -77,6 +77,7 @@ #import <WebCore/UTIRegistry.h> #import <WebCore/UTIUtilities.h> #import <pal/spi/cocoa/LaunchServicesSPI.h> +#import <pal/spi/cocoa/NSAccessibilitySPI.h> #import <pal/spi/cocoa/QuartzCoreSPI.h> #import <wtf/spi/darwin/SandboxSPI.h> @@ -404,10 +405,51 @@ void WebPage::clearDictationAlternatives(Vector<DictationContext>&& contexts) }, DocumentMarker::Type::DictationAlternatives); } -void WebPage::accessibilityTransferRemoteToken(RetainPtr<NSData> remoteToken) +void WebPage::accessibilityTransferRemoteToken(RetainPtr<NSData> remoteToken, FrameIdentifier frameID) { std::span dataToken(reinterpret_cast<const uint8_t*>([remoteToken bytes]), [remoteToken length]); - send(Messages::WebPageProxy::RegisterWebProcessAccessibilityToken(dataToken)); + send(Messages::WebPageProxy::RegisterWebProcessAccessibilityToken(dataToken, frameID)); +} + +void WebPage::accessibilityManageRemoteElementStatus(bool registerStatus, int processIdentifier) +{ +#if PLATFORM(MAC) + if (registerStatus) + [NSAccessibilityRemoteUIElement registerRemoteUIProcessIdentifier:processIdentifier]; + else + [NSAccessibilityRemoteUIElement unregisterRemoteUIProcessIdentifier:processIdentifier]; +#else + UNUSED_PARAM(registerStatus); + UNUSED_PARAM(processIdentifier); +#endif +} + +void WebPage::bindRemoteAccessibilityFrames(int processIdentifier, WebCore::FrameIdentifier frameID, std::span<const uint8_t> dataToken, CompletionHandler<void(std::span<const uint8_t>, int)>&& completionHandler) +{ + RefPtr webFrame = WebProcess::singleton().webFrame(frameID); + if (!webFrame) { + ASSERT_NOT_REACHED(); + return completionHandler({ }, 0); + } + + RefPtr coreLocalFrame = webFrame->coreLocalFrame(); + if (!coreLocalFrame) { + ASSERT_NOT_REACHED(); + return completionHandler({ }, 0); + } + + auto* renderer = coreLocalFrame->contentRenderer(); + if (!renderer) { + ASSERT_NOT_REACHED(); + return completionHandler({ }, 0); + } AG: what is the purpose of getting this renderer that is not used? + + registerRemoteFrameAccessibilityTokens(processIdentifier, dataToken); + + // Get our remote token data and send back to the RemoteFrame. + auto remoteToken = accessibilityRemoteTokenData(); + std::span remoteDataToken(reinterpret_cast<const uint8_t*>([remoteToken bytes]), [remoteToken length]); + completionHandler(remoteDataToken, getpid()); } #if ENABLE(APPLE_PAY)

diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm index 104410c82c8c..a7c860d01f5b 100644 --- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm +++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm @@ -243,12 +243,15 @@ void WebPage::platformInitializeAccessibility() m_mockAccessibilityElement = adoptNS([[WKAccessibilityWebPageObject alloc] init]); [m_mockAccessibilityElement setWebPage:this]; - accessibilityTransferRemoteToken(accessibilityRemoteTokenData()); + RefPtr localMainFrame = dynamicDowncast<LocalFrame>(m_page->mainFrame()); AG: m_page null check? + if (localMainFrame) + accessibilityTransferRemoteToken(accessibilityRemoteTokenData(), localMainFrame->frameID()); } void WebPage::platformReinitialize() { - accessibilityTransferRemoteToken(accessibilityRemoteTokenData()); + Ref frame = CheckedRef(m_page->focusController())->focusedOrMainFrame(); AG: m_page->focusController())-> null checks? + accessibilityTransferRemoteToken(accessibilityRemoteTokenData(), frame->frameID()); } RetainPtr<NSData> WebPage::accessibilityRemoteTokenData() const @@ -644,6 +647,16 @@ NSObject *WebPage::accessibilityObjectForMainFramePlugin() return nil; } +void WebPage::updateRemotePageAccessibilityOffset(WebCore::FrameIdentifier, WebCore::IntPoint) +{ + // FIXME: Implement +} + +void WebPage::registerRemoteFrameAccessibilityTokens(pid_t, std::span<const uint8_t>) +{ + // FIXME: Implement +} + void WebPage::registerUIProcessAccessibilityTokens(std::span<const uint8_t> elementToken, std::span<const uint8_t>) { NSData *elementTokenData = [NSData dataWithBytes:elementToken.data() length:elementToken.size()]; @@ -662,6 +675,12 @@ void WebPage::getDataSelectionForPasteboard(const String, CompletionHandler<void completionHandler({ }); } +WebCore::IntPoint WebPage::accessibilityRemoteFrameOffset() +{ + notImplemented(); + return { }; +} + WKAccessibilityWebPageObject* WebPage::accessibilityRemoteObject() { notImplemented();

diff --git a/Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm b/Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm index 6f99545aebf5..bec8d73faa2e 100644 --- a/Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm +++ b/Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm @@ -36,9 +36,11 @@ #import "WKStringCF.h" #import <WebCore/AXObjectCache.h> #import <WebCore/Document.h> +#import <WebCore/FrameTree.h> #import <WebCore/LocalFrame.h> #import <WebCore/LocalFrameView.h> #import <WebCore/Page.h> +#import <WebCore/RemoteFrame.h> #import <WebCore/ScrollView.h> #import <WebCore/Scrollbar.h> @@ -57,11 +59,17 @@ namespace ax = WebCore::Accessibility; if (!page) return nullptr; - auto* localMainFrame = dynamicDowncast<WebCore::LocalFrame>(page->mainFrame()); - if (!localMainFrame || !localMainFrame->document()) - return nullptr; + if (auto* localMainFrame = dynamicDowncast<WebCore::LocalFrame>(page->mainFrame())) { + if (auto* document = localMainFrame->document()) + return document->axObjectCache(); + } else if (auto* remoteMainFrame = dynamicDowncast<WebCore::RemoteFrame>(page->mainFrame())) { + auto& tree = remoteMainFrame->tree(); + auto* firstFrame = dynamicDowncast<WebCore::LocalFrame>(tree.firstChild()); + auto* document = firstFrame ? firstFrame->document() : nullptr; + return document ? document->axObjectCache() : nullptr; + } - return localMainFrame->document()->axObjectCache(); + return nullptr; } - (id)accessibilityPluginObject @@ -152,6 +160,17 @@ namespace ax = WebCore::Accessibility; m_hasMainFramePlugin = hasPlugin; } +- (void)setRemoteFrameOffset:(WebCore::IntPoint)offset +{ + ASSERT(isMainRunLoop()); + m_remoteFrameOffset = offset; +} + +- (WebCore::IntPoint)accessibilityRemoteFrameOffset +{ AG: should we ASSERT(isMainRunLoop()); here too? + return m_remoteFrameOffset; +} + - (void)setRemoteParent:(id)parent { ASSERT(isMainRunLoop());

diff --git a/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm b/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm index 96ed64adbcff..5f9b880c80bf 100644 --- a/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm +++ b/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm @@ -123,16 +123,12 @@ void WebPage::platformInitializeAccessibility() // Currently, it is also needed to allocate and initialize an NSApplication object. [NSApplication _accessibilityInitialize]; - auto mockAccessibilityElement = adoptNS([[WKAccessibilityWebPageObject alloc] init]); - // Get the pid for the starting process. pid_t pid = WebCore::presentingApplicationPID(); - if ([mockAccessibilityElement respondsToSelector:@selector(accessibilitySetPresenterProcessIdentifier:)]) - [(id)mockAccessibilityElement.get() accessibilitySetPresenterProcessIdentifier:pid]; - [mockAccessibilityElement setWebPage:this]; - m_mockAccessibilityElement = WTFMove(mockAccessibilityElement); - - accessibilityTransferRemoteToken(accessibilityRemoteTokenData()); + createMockAccessibilityElement(pid); + RefPtr localMainFrame = dynamicDowncast<LocalFrame>(m_page->mainFrame()); AG: m_page null check? + if (localMainFrame) + accessibilityTransferRemoteToken(accessibilityRemoteTokenData(), localMainFrame->frameID()); // Close Mach connection to Launch Services. #if HAVE(LS_SERVER_CONNECTION_STATUS_RELEASE_NOTIFICATIONS_MASK) @@ -144,9 +140,20 @@ void WebPage::platformInitializeAccessibility() WebProcess::singleton().revokeLaunchServicesSandboxExtension(); } +void WebPage::createMockAccessibilityElement(pid_t pid) +{ + auto mockAccessibilityElement = adoptNS([[WKAccessibilityWebPageObject alloc] init]); + + if ([mockAccessibilityElement respondsToSelector:@selector(accessibilitySetPresenterProcessIdentifier:)]) + [(id)mockAccessibilityElement.get() accessibilitySetPresenterProcessIdentifier:pid]; + [mockAccessibilityElement setWebPage:this]; + m_mockAccessibilityElement = WTFMove(mockAccessibilityElement); +} + void WebPage::platformReinitialize() { - accessibilityTransferRemoteToken(accessibilityRemoteTokenData()); + Ref frame = m_page->focusController().focusedOrMainFrame(); AG: m_page null check? + accessibilityTransferRemoteToken(accessibilityRemoteTokenData(), frame->frameID()); }

Created attachment 469953 [details] Patch

Committed 274955@main (9a0be8b71b1e): <https://commits.webkit.org/274955@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 469953 [details].