Bug 268804 – AX: Support site isolation for VoiceOver

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

Initial support site isolation for VoiceOver

<rdar://problem/99665561>

[chris fleizach]

Created attachment 469736 [details]
Patch

[Andres Gonzalez]

(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];
         }

[Tyler Wilcock]

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;

[chris fleizach]

Created attachment 469874 [details]
Patch

[chris fleizach]

Created attachment 469875 [details]
Patch

[chris fleizach]

Created attachment 469881 [details]
Patch

[chris fleizach]

Created attachment 469884 [details]
Patch

[chris fleizach]

Created attachment 469890 [details]
Patch

[chris fleizach]

Created attachment 469894 [details]
Patch

[chris fleizach]

Created attachment 469895 [details]
Patch

[Andres Gonzalez]

(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 };

[Andres Gonzalez]

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())

[Andres Gonzalez]

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);
 }

[Andres Gonzalez]

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;
 }

[chris fleizach]

Created attachment 469897 [details]
Patch

[Andres Gonzalez]

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;
 }

[Andres Gonzalez]

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;

[chris fleizach]

> +    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

[Andres Gonzalez]

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;

[Andres Gonzalez]

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

[chris fleizach]

+        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

[Andres Gonzalez]

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)

[chris fleizach]

+    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

[Andres Gonzalez]

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;

[Andres Gonzalez]

(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.

[chris fleizach]

(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<>

[chris fleizach]

Created attachment 469902 [details]
Patch

[chris fleizach]

Created attachment 469907 [details]
Patch

[chris fleizach]

Created attachment 469912 [details]
Patch

[chris fleizach]

Created attachment 469914 [details]
Patch

[chris fleizach]

Created attachment 469915 [details]
Patch

[chris fleizach]

Created attachment 469922 [details]
Patch

[chris fleizach]

Created attachment 469926 [details]
Patch

[Andres Gonzalez]

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*)
 {

[Andres Gonzalez]

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;
     }

[chris fleizach]

> 
> +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

[chris fleizach]

Created attachment 469936 [details]
Patch

[chris fleizach]

Created attachment 469946 [details]
Patch

[Andres Gonzalez]

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)
 {

[Andres Gonzalez]

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)

[Andres Gonzalez]

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();

[Andres Gonzalez]

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();

[chris fleizach]

> 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

[chris fleizach]

Created attachment 469948 [details]
Patch

[Andres Gonzalez]

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)

[Andres Gonzalez]

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();

[Andres Gonzalez]

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());

[Andres Gonzalez]

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());
 }

[chris fleizach]

Created attachment 469953 [details]
Patch

[EWS]

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].