Bug 1892514 - part 5-2: Add optional assertions into IMEContentObserver::FlatTextCache to detect creating invalid cache r=smaug

It's hard to write text change notification tests to verify the cache in
`IMEContentObserver`.  Therefore, assertions helps to detect the bug of the
cache with existing automated tests.  On the other hand, the verification
requires a lot of CPU time if editor inserts a lot of text, etc.  Therefore,
I'd like to add assertions behind a pref and enable it in some specific tests
and entire the tests of editor/editing API.

Differential Revision: https://phabricator.services.mozilla.com/D211834

dom/events/IMEContentObserver.cpp

@@ -14,6 +14,7 @@
 #include "mozilla/IMEStateManager.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/PresShell.h"
+#include "mozilla/StaticPrefs_test.h"
 #include "mozilla/TextComposition.h"
 #include "mozilla/TextControlElement.h"
 #include "mozilla/TextEvents.h"
@@ -1016,7 +1017,7 @@ void IMEContentObserver::NotifyContentAdded(nsINode* aContainer,
   // length can skip to compute the text length before the adding node and
   // before of it.
   mEndOfAddedTextCache.CacheFlatTextLengthBeforeEndOfContent(
-      *aLastContent, offset + addingLengthOrError.inspect());
+      *aLastContent, offset + addingLengthOrError.inspect(), mRootElement);
 
   if (!addingLengthOrError.inspect()) {
     return;
@@ -2218,7 +2219,7 @@ nsresult IMEContentObserver::FlatTextCache::
     return rv;
   }
 
-  CacheFlatTextLengthBeforeEndOfContent(aContent, length);
+  CacheFlatTextLengthBeforeEndOfContent(aContent, length, aRootElement);
   return NS_OK;
 }
 
@@ -2229,20 +2230,18 @@ nsresult IMEContentObserver::FlatTextCache::
 
   Clear();
 
-  uint32_t lengthIncludingLineBreakCausedByOpenTagOfContainer = 0;
-  nsresult rv = ContentEventHandler::GetFlatTextLengthInRange(
-      RawNodePosition::BeforeFirstContentOf(*aRootElement),
-      // Include the line break caused by open tag of aContainer if it's an
-      // element when we cache text length before first content of aContainer.
-      RawNodePosition(const_cast<nsINode*>(&aContainer), nullptr), aRootElement,
-      &lengthIncludingLineBreakCausedByOpenTagOfContainer,
-      LineBreakType::LINE_BREAK_TYPE_NATIVE);
-  if (NS_FAILED(rv)) {
-    return rv;
+  const Result<uint32_t, nsresult>
+      lengthIncludingLineBreakCausedByOpenTagOfContainer =
+          FlatTextCache::ComputeTextLengthBeforeFirstContentOf(aContainer,
+                                                               aRootElement);
+  if (MOZ_UNLIKELY(
+          lengthIncludingLineBreakCausedByOpenTagOfContainer.isErr())) {
+    return lengthIncludingLineBreakCausedByOpenTagOfContainer.inspectErr();
   }
 
   CacheFlatTextLengthBeforeFirstContent(
-      aContainer, lengthIncludingLineBreakCausedByOpenTagOfContainer);
+      aContainer, lengthIncludingLineBreakCausedByOpenTagOfContainer.inspect(),
+      aRootElement);
   return NS_OK;
 }
 
@@ -2308,4 +2307,82 @@ Result<uint32_t, nsresult> IMEContentObserver::FlatTextCache::
   return textLength;
 }
 
+/* static */
+Result<uint32_t, nsresult>
+IMEContentObserver::FlatTextCache::ComputeTextLengthBeforeFirstContentOf(
+    const nsINode& aContainer, const dom::Element* aRootElement) {
+  uint32_t lengthIncludingLineBreakCausedByOpenTagOfContent = 0;
+  nsresult rv = ContentEventHandler::GetFlatTextLengthInRange(
+      RawNodePosition::BeforeFirstContentOf(*aRootElement),
+      // Include the line break caused by open tag of aContainer if it's an
+      // element when we cache text length before first content of aContainer.
+      RawNodePosition(const_cast<nsINode*>(&aContainer), nullptr), aRootElement,
+      &lengthIncludingLineBreakCausedByOpenTagOfContent,
+      LineBreakType::LINE_BREAK_TYPE_NATIVE);
+  if (NS_FAILED(rv)) {
+    return Err(rv);
+  }
+  return lengthIncludingLineBreakCausedByOpenTagOfContent;
+}
+
+void IMEContentObserver::FlatTextCache::AssertValidCache(
+    const Element* aRootElement) const {
+#ifdef DEBUG
+  if (MOZ_LIKELY(
+          !StaticPrefs::test_ime_content_observer_assert_valid_cache())) {
+    return;
+  }
+  MOZ_ASSERT(aRootElement);
+  if (!mContainerNode) {
+    return;
+  }
+  MOZ_ASSERT(mContainerNode->IsInclusiveDescendantOf(aRootElement));
+  MOZ_ASSERT_IF(mContent, mContent->IsInclusiveDescendantOf(aRootElement));
+
+  if (IsCachingToEndOfContent()) {
+    MOZ_ASSERT(mContent);
+    Result<uint32_t, nsresult> offset =
+        FlatTextCache::ComputeTextLengthBeforeContent(*mContent, aRootElement);
+    MOZ_ASSERT(offset.isOk());
+    Result<uint32_t, nsresult> length =
+        FlatTextCache::ComputeTextLengthStartOfContentToEndOfContent(
+            *mContent, *mContent, aRootElement);
+    MOZ_ASSERT(length.isOk());
+    if (mFlatTextLength != offset.inspect() + length.inspect()) {
+      nsAutoString innerHTMLOfEditable;
+      const_cast<Element*>(aRootElement)
+          ->GetInnerHTML(innerHTMLOfEditable, IgnoreErrors());
+      NS_WARNING(
+          nsPrintfCString(
+              "mFlatTextLength=%u, offset: %u, length: %u, mContainerNode:%s, "
+              "mContent=%s (%s)",
+              mFlatTextLength, offset.inspect(), length.inspect(),
+              ToString(mContainerNode).c_str(), ToString(*mContent).c_str(),
+              NS_ConvertUTF16toUTF8(innerHTMLOfEditable).get())
+              .get());
+    }
+    MOZ_ASSERT(mFlatTextLength == offset.inspect() + length.inspect());
+    return;
+  }
+
+  MOZ_ASSERT(!mContent);
+  MOZ_ASSERT(mContainerNode->IsContent());
+  Result<uint32_t, nsresult> offset =
+      ComputeTextLengthBeforeFirstContentOf(*mContainerNode, aRootElement);
+  MOZ_ASSERT(offset.isOk());
+  if (mFlatTextLength != offset.inspect()) {
+    nsAutoString innerHTMLOfEditable;
+    const_cast<Element*>(aRootElement)
+        ->GetInnerHTML(innerHTMLOfEditable, IgnoreErrors());
+    NS_WARNING(nsPrintfCString(
+                   "mFlatTextLength=%u, offset: %u, mContainerNode:%s (%s)",
+                   mFlatTextLength, offset.inspect(),
+                   ToString(mContainerNode).c_str(),
+                   NS_ConvertUTF16toUTF8(innerHTMLOfEditable).get())
+                   .get());
+  }
+  MOZ_ASSERT(mFlatTextLength == offset.inspect());
+#endif  // #ifdef DEBUG
+}
+
 }  // namespace mozilla

dom/events/IMEContentObserver.h

@@ -500,12 +500,14 @@ class IMEContentObserver final : public nsStubMutationObserver,
     [[nodiscard]] nsresult ComputeAndCacheFlatTextLengthBeforeEndOfContent(
         const nsIContent& aContent, const dom::Element* aRootElement);
 
-    void CacheFlatTextLengthBeforeEndOfContent(const nsIContent& aContent,
-                                               uint32_t aFlatTextLength) {
+    void CacheFlatTextLengthBeforeEndOfContent(
+        const nsIContent& aContent, uint32_t aFlatTextLength,
+        const dom::Element* aRootElement) {
       mContainerNode = aContent.GetParentNode();
       mContent = const_cast<nsIContent*>(&aContent);
       mFlatTextLength = aFlatTextLength;
       MOZ_ASSERT(IsCachingToEndOfContent());
+      AssertValidCache(aRootElement);
     }
 
     /**
@@ -525,12 +527,14 @@ class IMEContentObserver final : public nsStubMutationObserver,
     [[nodiscard]] nsresult ComputeAndCacheFlatTextLengthBeforeFirstContent(
         const nsINode& aContainer, const dom::Element* aRootElement);
 
-    void CacheFlatTextLengthBeforeFirstContent(const nsINode& aContainer,
-                                               uint32_t aFlatTextLength) {
+    void CacheFlatTextLengthBeforeFirstContent(
+        const nsINode& aContainer, uint32_t aFlatTextLength,
+        const dom::Element* aRootElement) {
       mContainerNode = const_cast<nsINode*>(&aContainer);
       mContent = nullptr;
       mFlatTextLength = aFlatTextLength;
       MOZ_ASSERT(IsCachingToStartOfContainer());
+      AssertValidCache(aRootElement);
     }
 
     /**
@@ -590,6 +594,25 @@ class IMEContentObserver final : public nsStubMutationObserver,
         const nsIContent& aStartContent, const nsIContent& aEndContent,
         const dom::Element* aRootElement);
 
+    /**
+     * Return flattened text length starting from first content of aRootElement
+     * and ending at start of the first content of aContainer.  So, if
+     * ContentEventHandler generates a line break at the open tag of aContainer,
+     * the result includes the line break length.
+     *
+     * @param aContainer        The container node which you want to compute the
+     *                          flattened text length before the first content
+     *                          of.
+     * @param aRootElement      The root element of the editor, i.e., editing
+     *                          host or the anonymous <div> in a text control.
+     *                          For avoiding to generate a redundant line break
+     *                          at open tag of this element, this is required
+     *                          to call methods of ContentEventHandler.
+     */
+    [[nodiscard]] static Result<uint32_t, nsresult>
+    ComputeTextLengthBeforeFirstContentOf(const nsINode& aContainer,
+                                          const dom::Element* aRootElement);
+
     [[nodiscard]] bool CachesTextLengthBeforeContent(
         const nsIContent& aContent) const {
       MOZ_ASSERT(!aContent.IsBeingRemoved());
@@ -612,6 +635,14 @@ class IMEContentObserver final : public nsStubMutationObserver,
              mContent == aPreviousSibling;
     }
 
+    /**
+     * This works only in the debug build and
+     * test.ime_content_observer.assert_valid_cache pref is enabled.  This
+     * checks with expensive computation, therefore, the pref is enabled only
+     * when running automated tests for editors.
+     */
+    void AssertValidCache(const dom::Element* aRootElement) const;
+
     // mContainerNode is parent node of mContent when it's cached.
     nsCOMPtr<nsINode> mContainerNode;
     // mContent points to the last child which participates in the current

modules/libpref/init/StaticPrefList.yaml

@@ -15633,6 +15633,13 @@
   value: false
   mirror: always
 
+# Enable assertions in IMEContentObserver::FlatTextCache on debug builds.
+# If this pref is enabled, DOM mutation becomes much slower.
+- name: test.ime_content_observer.assert_valid_cache
+  type: bool
+  value: false
+  mirror: always
+
 - name: test.mousescroll
   type: RelaxedAtomicBool
   value: false