[AT-SPI] Rework children handling in ActorAccessible 71/268671/8
authorArtur Świgoń <a.swigon@samsung.com>
Wed, 29 Dec 2021 07:57:50 +0000 (08:57 +0100)
committerArtur Świgoń <a.swigon@samsung.com>
Thu, 13 Jan 2022 08:58:04 +0000 (09:58 +0100)
This patch reorganizes the children-related methods in ActorAccessible.
A few methods are also marked 'final' to prevent overriding in derived
classes. A new overridable method, DoGetChildren() is provided instead,
to offer a simple and bulletproof mechanism for derived classes to
customize the children to be reported in the AT-SPI tree (possibly
including non-ActorAccessible objects).

Change-Id: I0c34c3493091c108e94aa140f883f012aea347f6

dali/devel-api/adaptor-framework/actor-accessible.cpp
dali/devel-api/adaptor-framework/actor-accessible.h

index 82e4b18..81a0877 100644 (file)
 // CLASS HEADER
 #include <dali/devel-api/adaptor-framework/actor-accessible.h>
 
+// EXTERNAL INCLUDES
+#include <dali/devel-api/actors/actor-devel.h>
+
 // INTERNAL INCLUDES
 #include <dali/devel-api/adaptor-framework/window-devel.h>
 
 namespace Dali::Accessibility
 {
 ActorAccessible::ActorAccessible(Actor actor)
-: mSelf(actor)
+: mSelf(actor),
+  mChildrenDirty{true} // to trigger the initial UpdateChildren()
 {
+  // Select the right overload manually because Connect(this, &OnChildrenChanged) is ambiguous.
+  void (ActorAccessible::*handler)(Dali::Actor) = &ActorAccessible::OnChildrenChanged;
+
+  Dali::DevelActor::ChildAddedSignal(actor).Connect(this, handler);
+  Dali::DevelActor::ChildRemovedSignal(actor).Connect(this, handler);
+  Dali::DevelActor::ChildOrderChangedSignal(actor).Connect(this, handler);
 }
 
 std::string ActorAccessible::GetName() const
@@ -51,45 +61,45 @@ Accessible* ActorAccessible::GetParent()
 
 std::size_t ActorAccessible::GetChildCount() const
 {
-  return static_cast<std::size_t>(Self().GetChildCount());
+  // ActorAccessible is never stored in const memory and it is an implementation detail that
+  // children are recalculated lazily.
+  const_cast<ActorAccessible*>(this)->UpdateChildren();
+
+  return mChildren.size();
 }
 
 std::vector<Accessible*> ActorAccessible::GetChildren()
 {
-  std::vector<Accessible*> tmp(GetChildCount());
-  for(auto i = 0u; i < tmp.size(); ++i)
-  {
-    tmp[i] = GetChildAtIndex(i);
-  }
+  UpdateChildren();
 
-  return tmp;
+  return mChildren;
 }
 
 Accessible* ActorAccessible::GetChildAtIndex(std::size_t index)
 {
-  auto numberOfChildren = GetChildCount();
-  if(DALI_UNLIKELY(index >= numberOfChildren))
+  auto childCount = GetChildCount(); // calls UpdateChildren()
+  if(DALI_UNLIKELY(index >= childCount))
   {
-    throw std::domain_error{"invalid index " + std::to_string(index) + " for object with " + std::to_string(numberOfChildren) + " children"};
+    throw std::domain_error{"invalid index " + std::to_string(index) + " for object with " + std::to_string(childCount) + " children"};
   }
 
-  return Get(Self().GetChildAt(static_cast<std::uint32_t>(index)));
+  return mChildren[index];
 }
 
 std::size_t ActorAccessible::GetIndexInParent()
 {
-  auto self   = Self();
-  auto parent = self.GetParent();
+  auto* parent = GetParent();
 
   if(DALI_UNLIKELY(!parent))
   {
     throw std::domain_error{"can't call GetIndexInParent on object without parent"};
   }
 
-  auto size = static_cast<std::size_t>(parent.GetChildCount());
-  for(auto i = 0u; i < size; ++i)
+  // Avoid calling parent->GetChildren() in order not to make an unnecessary copy
+  auto childCount = parent->GetChildCount();
+  for(auto i = 0u; i < childCount; ++i)
   {
-    if(parent.GetChildAt(i) == self)
+    if(parent->GetChildAtIndex(i) == this)
     {
       return i;
     }
@@ -144,4 +154,50 @@ Dali::Rect<> ActorAccessible::GetExtents(CoordinateType type) const
   }
 }
 
+void ActorAccessible::OnChildrenChanged()
+{
+  mChildrenDirty = true;
+}
+
+void ActorAccessible::OnChildrenChanged(Dali::Actor)
+{
+  OnChildrenChanged();
+}
+
+void ActorAccessible::DoGetChildren(std::vector<Accessible*>& children)
+{
+  auto self       = Self();
+  auto childCount = self.GetChildCount();
+
+  children.reserve(childCount);
+
+  for(auto i = 0u; i < childCount; ++i)
+  {
+    children.push_back(Accessible::Get(self.GetChildAt(i)));
+  }
+}
+
+void ActorAccessible::UpdateChildren()
+{
+  if(!mChildrenDirty)
+  {
+    return;
+  }
+
+  // Set to false before DoGetChildren() to prevent recursion
+  // in case DoGetChildren() does something strange.
+  mChildrenDirty = false;
+
+  mChildren.clear();
+  DoGetChildren(mChildren);
+
+  // Erase-remove idiom
+  // TODO (C++20): Replace with std::erase_if
+  auto it = std::remove_if(mChildren.begin(), mChildren.end(), [](const Accessible* child) {
+    return !child;
+  });
+  mChildren.erase(it, mChildren.end());
+  mChildren.shrink_to_fit();
+}
+
 } // namespace Dali::Accessibility
index 1306229..1cf3812 100644 (file)
@@ -20,6 +20,7 @@
 // EXTERNAL INCLUDES
 #include <dali/public-api/actors/actor.h>
 #include <dali/public-api/object/weak-handle.h>
+#include <dali/public-api/signals/connection-tracker.h>
 
 // INTERNAL INCLUDES
 #include <dali/devel-api/atspi-interfaces/accessible.h>
 
 namespace Dali::Accessibility
 {
-class DALI_ADAPTOR_API ActorAccessible : public virtual Accessible, public virtual Collection, public virtual Component
+class DALI_ADAPTOR_API ActorAccessible : public virtual Accessible,
+                                         public virtual Collection,
+                                         public virtual Component,
+                                         public Dali::ConnectionTracker
 {
 public:
   ActorAccessible() = delete;
@@ -48,32 +52,32 @@ public:
   /**
    * @copydoc Dali::Accessibility::Accessible::GetParent()
    */
-  Accessible* GetParent() override;
+  Accessible* GetParent() final;
 
   /**
    * @copydoc Dali::Accessibility::Accessible::GetChildCount()
    */
-  std::size_t GetChildCount() const override;
+  std::size_t GetChildCount() const final;
 
   /**
    * @copydoc Dali::Accessibility::Accessible::GetChildren()
    */
-  std::vector<Accessible*> GetChildren() override;
+  std::vector<Accessible*> GetChildren() final;
 
   /**
    * @copydoc Dali::Accessibility::Accessible::GetChildAtIndex()
    */
-  Accessible* GetChildAtIndex(std::size_t index) override;
+  Accessible* GetChildAtIndex(std::size_t index) final;
 
   /**
    * @copydoc Dali::Accessibility::Accessible::GetIndexInParent()
    */
-  std::size_t GetIndexInParent() override;
+  std::size_t GetIndexInParent() final;
 
   /**
    * @copydoc Dali::Accessibility::Accessible::GetInternalActor()
    */
-  Dali::Actor GetInternalActor() override;
+  Dali::Actor GetInternalActor() final;
 
   /**
    * @copydoc Dali::Accessibility::Component::GetLayer()
@@ -100,6 +104,16 @@ public:
    */
   Dali::Rect<> GetExtents(CoordinateType type) const override;
 
+  /**
+   * @brief Notifies this object that its children have changed.
+   *
+   * This is useful if you maintain a custom collection of children that are not derived from
+   * ActorAccessible and the contents or order of elements in that collection change.
+   *
+   * @see DoGetChildren()
+   */
+  void OnChildrenChanged();
+
 protected:
   Dali::Actor Self() const
   {
@@ -111,8 +125,37 @@ protected:
     return handle;
   }
 
+  /**
+   * @brief Populates the collection of children of this Accessible.
+   *
+   * The default implementation retrieves the children from the Actor hierarchy. Override this if
+   * you want to report other objects as children, either instead of, or together with the
+   * dependent Actor-derived Accessibles. Remember to call OnChildrenChanged() if you want your
+   * implementation of DoGetChildren() to be called again (in case your custom collection of
+   * children changes).
+   *
+   * @param[out] children The initially empty vector to insert children into
+   *
+   * @note GetChildCound(), GetChildren() and GetChildAtIndex() are not available for overriding,
+   * but they respect the children collection reported by this method.
+   *
+   * @see OnChildrenChanged()
+   * @see GetChildCount()
+   * @see GetChildren()
+   * @see GetChildAtIndex()
+   */
+  virtual void DoGetChildren(std::vector<Accessible*>& children);
+
 private:
+  // Extra overload for OnChildrenChanged() to connect to signals directly
+  void OnChildrenChanged(Dali::Actor);
+
+  // Ensures children are up to date (calls DoGetChildren() if necessary)
+  void UpdateChildren();
+
   Dali::WeakHandle<Dali::Actor> mSelf;
+  std::vector<Accessible*>      mChildren;
+  bool                          mChildrenDirty;
 };
 
 } // namespace Dali::Accessibility