Backed out changeset a81a40b250b4 (bug 1766794) for causing marionette failures. CLOSED TREE

accessible/base/TextUpdater.cpp

@@ -44,7 +44,8 @@ void TextUpdater::DoUpdate(const nsAString& aNewText, const nsAString& aOldText,
     return;
   }
 
-  mTextOffset = mHyperText->GetChildOffset(mTextLeaf);
+  // Get the text leaf accessible offset and invalidate cached offsets after it.
+  mTextOffset = mHyperText->GetChildOffset(mTextLeaf, true);
   NS_ASSERTION(mTextOffset != -1, "Text leaf hasn't offset within hyper text!");
 
   uint32_t oldLen = aOldText.Length(), newLen = aNewText.Length();
@@ -89,7 +90,6 @@ void TextUpdater::DoUpdate(const nsAString& aNewText, const nsAString& aOldText,
 
     // Update the text.
     mTextLeaf->SetText(aNewText);
-    mHyperText->InvalidateCachedHyperTextOffsets();
     return;
   }
 
@@ -135,7 +135,6 @@ void TextUpdater::DoUpdate(const nsAString& aNewText, const nsAString& aOldText,
 
   // Update the text.
   mTextLeaf->SetText(aNewText);
-  mHyperText->InvalidateCachedHyperTextOffsets();
 }
 
 void TextUpdater::ComputeTextChangeEvents(

accessible/basetypes/HyperTextAccessibleBase.cpp

@@ -14,33 +14,21 @@
 
 namespace mozilla::a11y {
 
-void HyperTextAccessibleBase::BuildCachedHyperTextOffsets(
-    nsTArray<int32_t>& aOffsets) const {
+int32_t HyperTextAccessibleBase::GetChildIndexAtOffset(uint32_t aOffset) const {
   const Accessible* thisAcc = Acc();
   uint32_t childCount = thisAcc->ChildCount();
-  int32_t lastTextOffset = 0;
-  while (aOffsets.Length() < childCount) {
-    Accessible* child = thisAcc->ChildAt(aOffsets.Length());
-    lastTextOffset += static_cast<int32_t>(nsAccUtils::TextLength(child));
-    aOffsets.AppendElement(lastTextOffset);
+  uint32_t lastTextOffset = 0;
+  for (uint32_t childIndex = 0; childIndex < childCount; ++childIndex) {
+    Accessible* child = thisAcc->ChildAt(childIndex);
+    lastTextOffset += nsAccUtils::TextLength(child);
+    if (aOffset < lastTextOffset) {
+      return childIndex;
+    }
   }
-}
-
-int32_t HyperTextAccessibleBase::GetChildIndexAtOffset(uint32_t aOffset) const {
-  const auto& offsets = GetCachedHyperTextOffsets();
-  auto childCount = offsets.Length();
-  size_t index;
-  if (BinarySearch(offsets, 0, childCount, static_cast<int32_t>(aOffset),
-                   &index)) {
-    // aOffset is the exclusive end of a child, so return the child before it.
-    return static_cast<int32_t>(index < childCount - 1 ? index + 1 : index);
+  if (aOffset == lastTextOffset) {
+    return childCount - 1;
   }
-  if (index == childCount) {
-    // aOffset is past the end of the text.
-    return -1;
-  }
-  // index points at the exclusive end after aOffset.
-  return static_cast<int32_t>(index);
+  return -1;
 }
 
 Accessible* HyperTextAccessibleBase::GetChildAtOffset(uint32_t aOffset) const {
@@ -48,8 +36,8 @@ Accessible* HyperTextAccessibleBase::GetChildAtOffset(uint32_t aOffset) const {
   return thisAcc->ChildAt(GetChildIndexAtOffset(aOffset));
 }
 
-int32_t HyperTextAccessibleBase::GetChildOffset(
-    const Accessible* aChild) const {
+int32_t HyperTextAccessibleBase::GetChildOffset(const Accessible* aChild,
+                                                bool aInvalidateAfter) const {
   const Accessible* thisAcc = Acc();
   if (aChild->Parent() != thisAcc) {
     return -1;
@@ -58,16 +46,26 @@ int32_t HyperTextAccessibleBase::GetChildOffset(
   if (index == -1) {
     return -1;
   }
-  return GetChildOffset(index);
+  return GetChildOffset(index, aInvalidateAfter);
 }
 
-int32_t HyperTextAccessibleBase::GetChildOffset(uint32_t aChildIndex) const {
+int32_t HyperTextAccessibleBase::GetChildOffset(uint32_t aChildIndex,
+                                                bool aInvalidateAfter) const {
   if (aChildIndex == 0) {
     return 0;
   }
-  MOZ_ASSERT(aChildIndex <= Acc()->ChildCount());
-  const auto& offsets = GetCachedHyperTextOffsets();
-  return offsets[aChildIndex - 1];
+  const Accessible* thisAcc = Acc();
+  MOZ_ASSERT(aChildIndex <= thisAcc->ChildCount());
+  uint32_t lastTextOffset = 0;
+  for (uint32_t childIndex = 0; childIndex <= aChildIndex; ++childIndex) {
+    if (childIndex == aChildIndex) {
+      return lastTextOffset;
+    }
+    Accessible* child = thisAcc->ChildAt(childIndex);
+    lastTextOffset += nsAccUtils::TextLength(child);
+  }
+  MOZ_ASSERT_UNREACHABLE();
+  return lastTextOffset;
 }
 
 uint32_t HyperTextAccessibleBase::CharacterCount() const {

accessible/basetypes/HyperTextAccessibleBase.h

@@ -40,12 +40,6 @@ class index_t {
 
 class HyperTextAccessibleBase {
  public:
-  /**
-   * Invalidate cached HyperText offsets. This should be called whenever a
-   * child is added or removed or the text of a text leaf child is changed.
-   */
-  virtual void InvalidateCachedHyperTextOffsets() = 0;
-
   /**
    * Return child accessible at the given text offset.
    *
@@ -65,13 +59,17 @@ class HyperTextAccessibleBase {
    * accessible.
    *
    * @param  aChild           [in] accessible child to get text offset for
+   * @param  aInvalidateAfter [in, optional] indicates whether invalidate
+   *                           cached offsets for next siblings of the child
    */
-  int32_t GetChildOffset(const Accessible* aChild) const;
+  int32_t GetChildOffset(const Accessible* aChild,
+                         bool aInvalidateAfter = false) const;
 
   /**
    * Return text offset for the child accessible index.
    */
-  virtual int32_t GetChildOffset(uint32_t aChildIndex) const;
+  virtual int32_t GetChildOffset(uint32_t aChildIndex,
+                                 bool aInvalidateAfter = false) const;
 
   /**
    * Return character count within the hypertext accessible.
@@ -192,20 +190,6 @@ class HyperTextAccessibleBase {
     return const_cast<Accessible*>(acc);
   }
 
-  /**
-   * Get the cached map of child indexes to HyperText offsets. If the cache
-   * hasn't been built yet, build it.
-   * This is an array which contains the exclusive end offset for each child.
-   * That is, the start offset for child c is array index c - 1.
-   */
-  virtual const nsTArray<int32_t>& GetCachedHyperTextOffsets() const = 0;
-
-  /**
-   * Build the HyperText offsets cache. This should only be called by
-   * GetCachedHyperTextOffsets.
-   */
-  void BuildCachedHyperTextOffsets(nsTArray<int32_t>& aOffsets) const;
-
  private:
   /**
    * Transform the given a11y point into an offset relative to this hypertext.

accessible/generic/HyperTextAccessible.cpp

@@ -2092,13 +2092,21 @@ void HyperTextAccessible::Shutdown() {
 }
 
 bool HyperTextAccessible::RemoveChild(LocalAccessible* aAccessible) {
-  InvalidateCachedHyperTextOffsets();
+  const int32_t childIndex = aAccessible->IndexInParent();
+  if (childIndex < static_cast<int64_t>(mOffsets.Length())) {
+    mOffsets.RemoveLastElements(mOffsets.Length() -
+                                aAccessible->IndexInParent());
+  }
+
   return AccessibleWrap::RemoveChild(aAccessible);
 }
 
 bool HyperTextAccessible::InsertChildAt(uint32_t aIndex,
                                         LocalAccessible* aChild) {
-  InvalidateCachedHyperTextOffsets();
+  if (aIndex < mOffsets.Length()) {
+    mOffsets.RemoveLastElements(mOffsets.Length() - aIndex);
+  }
+
   return AccessibleWrap::InsertChildAt(aIndex, aChild);
 }
 
@@ -2190,6 +2198,65 @@ nsresult HyperTextAccessible::RenderedToContentOffset(
   return NS_OK;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// HyperTextAccessible public
+
+int32_t HyperTextAccessible::GetChildOffset(uint32_t aChildIndex,
+                                            bool aInvalidateAfter) const {
+  if (aChildIndex == 0) {
+    if (aInvalidateAfter) mOffsets.Clear();
+
+    return aChildIndex;
+  }
+
+  int32_t count = mOffsets.Length() - aChildIndex;
+  if (count > 0) {
+    if (aInvalidateAfter) mOffsets.RemoveElementsAt(aChildIndex, count);
+
+    return mOffsets[aChildIndex - 1];
+  }
+
+  uint32_t lastOffset =
+      mOffsets.IsEmpty() ? 0 : mOffsets[mOffsets.Length() - 1];
+
+  while (mOffsets.Length() < aChildIndex) {
+    LocalAccessible* child = mChildren[mOffsets.Length()];
+    lastOffset += nsAccUtils::TextLength(child);
+    mOffsets.AppendElement(lastOffset);
+  }
+
+  return mOffsets[aChildIndex - 1];
+}
+
+int32_t HyperTextAccessible::GetChildIndexAtOffset(uint32_t aOffset) const {
+  uint32_t lastOffset = 0;
+  const uint32_t offsetCount = mOffsets.Length();
+
+  if (offsetCount > 0) {
+    lastOffset = mOffsets[offsetCount - 1];
+    if (aOffset < lastOffset) {
+      size_t index;
+      if (BinarySearch(mOffsets, 0, offsetCount, aOffset, &index)) {
+        return (index < (offsetCount - 1)) ? index + 1 : index;
+      }
+
+      return (index == offsetCount) ? -1 : index;
+    }
+  }
+
+  uint32_t childCount = ChildCount();
+  while (mOffsets.Length() < childCount) {
+    LocalAccessible* child = LocalChildAt(mOffsets.Length());
+    lastOffset += nsAccUtils::TextLength(child);
+    mOffsets.AppendElement(lastOffset);
+    if (aOffset < lastOffset) return mOffsets.Length() - 1;
+  }
+
+  if (aOffset == lastOffset) return mOffsets.Length() - 1;
+
+  return -1;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // HyperTextAccessible protected
 

accessible/generic/HyperTextAccessible.h

@@ -159,10 +159,12 @@ class HyperTextAccessible : public AccessibleWrap,
 
   virtual already_AddRefed<AccAttributes> DefaultTextAttributes() override;
 
-  virtual void InvalidateCachedHyperTextOffsets() override { mOffsets.Clear(); }
-
   // HyperTextAccessibleBase provides an overload which takes an Accessible.
   using HyperTextAccessibleBase::GetChildOffset;
+  virtual int32_t GetChildOffset(uint32_t aChildIndex,
+                                 bool aInvalidateAfter = false) const override;
+
+  virtual int32_t GetChildIndexAtOffset(uint32_t aOffset) const override;
 
   virtual LocalAccessible* GetChildAtOffset(uint32_t aOffset) const override {
     return LocalChildAt(GetChildIndexAtOffset(aOffset));
@@ -431,18 +433,11 @@ class HyperTextAccessible : public AccessibleWrap,
   // HyperTextAccessibleBase
   virtual const Accessible* Acc() const override { return this; }
 
-  virtual const nsTArray<int32_t>& GetCachedHyperTextOffsets() const override {
-    if (mOffsets.IsEmpty()) {
-      BuildCachedHyperTextOffsets(mOffsets);
-    }
-    return mOffsets;
-  }
-
  private:
   /**
    * End text offsets array.
    */
-  mutable nsTArray<int32_t> mOffsets;
+  mutable nsTArray<uint32_t> mOffsets;
 };
 
 ////////////////////////////////////////////////////////////////////////////////

accessible/ipc/DocAccessibleParent.cpp

@@ -296,10 +296,6 @@ mozilla::ipc::IPCResult DocAccessibleParent::RecvEvent(
   if (StaticPrefs::accessibility_cache_enabled_AtStartup()) {
     if (aEventType == nsIAccessibleEvent::EVENT_REORDER ||
         aEventType == nsIAccessibleEvent::EVENT_INNER_REORDER) {
-      if (proxy->IsHyperText()) {
-        // Invalidate the HyperText offset cache.
-        proxy->InvalidateCachedHyperTextOffsets();
-      }
       for (RemoteAccessible* child = proxy->RemoteFirstChild(); child;
            child = child->RemoteNextSibling()) {
         child->InvalidateGroupInfo();
@@ -578,13 +574,6 @@ mozilla::ipc::IPCResult DocAccessibleParent::RecvCache(
     }
 
     remote->ApplyCache(aUpdateType, entry.Fields());
-    if (aUpdateType == CacheUpdateType::Update && remote->IsTextLeaf()) {
-      // Invalidate the HyperText offset cache.
-      RemoteAccessible* parent = remote->RemoteParent();
-      if (parent && parent->IsHyperText()) {
-        parent->InvalidateCachedHyperTextOffsets();
-      }
-    }
   }
 
   if (nsCOMPtr<nsIObserverService> obsService =

accessible/ipc/RemoteAccessibleBase.cpp

@@ -13,7 +13,6 @@
 #include "mozilla/a11y/RemoteAccessibleBase.h"
 #include "mozilla/a11y/RemoteAccessible.h"
 #include "mozilla/a11y/Role.h"
-#include "mozilla/BinarySearch.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/BrowserParent.h"
 #include "mozilla/dom/CanonicalBrowsingContext.h"
@@ -1014,25 +1013,6 @@ bool RemoteAccessibleBase<Derived>::TableIsProbablyForLayout() {
   return false;
 }
 
-template <class Derived>
-const nsTArray<int32_t>&
-RemoteAccessibleBase<Derived>::GetCachedHyperTextOffsets() const {
-  if (mCachedFields) {
-    if (auto offsets =
-            mCachedFields->GetAttribute<nsTArray<int32_t>>(nsGkAtoms::offset)) {
-      return *offsets;
-    }
-  }
-  nsTArray<int32_t> newOffsets;
-  BuildCachedHyperTextOffsets(newOffsets);
-  if (!mCachedFields) {
-    const_cast<RemoteAccessibleBase<Derived>*>(this)->mCachedFields =
-        new AccAttributes();
-  }
-  mCachedFields->SetAttribute(nsGkAtoms::offset, std::move(newOffsets));
-  return *mCachedFields->GetAttribute<nsTArray<int32_t>>(nsGkAtoms::offset);
-}
-
 template class RemoteAccessibleBase<RemoteAccessible>;
 
 }  // namespace a11y

accessible/ipc/RemoteAccessibleBase.h

@@ -291,12 +291,6 @@ class RemoteAccessibleBase : public Accessible, public HyperTextAccessibleBase {
   // HyperTextAccessibleBase
   virtual already_AddRefed<AccAttributes> DefaultTextAttributes() override;
 
-  virtual void InvalidateCachedHyperTextOffsets() override {
-    if (mCachedFields) {
-      mCachedFields->Remove(nsGkAtoms::offset);
-    }
-  }
-
  protected:
   RemoteAccessibleBase(uint64_t aID, Derived* aParent,
                        DocAccessibleParent* aDoc, role aRole, AccType aType,
@@ -337,8 +331,6 @@ class RemoteAccessibleBase : public Accessible, public HyperTextAccessibleBase {
 
   nsAtom* GetPrimaryAction() const;
 
-  virtual const nsTArray<int32_t>& GetCachedHyperTextOffsets() const override;
-
  private:
   uintptr_t mParent;
   static const uintptr_t kNoParent = UINTPTR_MAX;