Bug 1250823 part 2 - IMEContentObserver should cache adding ranges as a range during document change r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 08 Jun 2017 11:24:58 +0900
changeset 590763 48c467a9155b76fc7ae67205278c1dadd0e6a688
parent 590762 68a1cc6ddba1ccad46b61e8077fde414076dace8
child 632302 8fa26634e84e6f957468ad475ce72e745403e290
push id62823
push usermasayuki@d-toybox.com
push dateThu, 08 Jun 2017 02:46:52 +0000
reviewerssmaug
bugs1250823
milestone55.0a1
Bug 1250823 part 2 - IMEContentObserver should cache adding ranges as a range during document change r?smaug Between nsIDocumentObserver::BeginUpdate() and nsIDocumentObserver::EndUpdate(), IMEContentObserver can cache added nodes as a range if they are consecutive nodes. Even if a node is removed, a data node is changed or attribute is changed unexpectedly, IMEContentObserver can post text change of the added node range and handle it normally. MozReview-Commit-ID: IttDHBkr92Y
dom/events/IMEContentObserver.cpp
dom/events/IMEContentObserver.h
widget/tests/window_composition_text_querycontent.xul
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -116,16 +116,20 @@ public:
 };
 
 /******************************************************************************
  * mozilla::IMEContentObserver
  ******************************************************************************/
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(IMEContentObserver)
 
+// Note that we don't need to add mFirstAddedNodeContainer nor
+// mLastAddedNodeContainer to cycle collection because they are non-null only
+// during short time and shouldn't be touched while they are non-null.
+
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(IMEContentObserver)
   nsAutoScriptBlocker scriptBlocker;
 
   tmp->NotifyIMEOfBlur();
   tmp->UnregisterObservers();
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSelection)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mRootContent)
@@ -163,17 +167,19 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
  NS_INTERFACE_MAP_ENTRY(nsIEditorObserver)
  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsISelectionListener)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(IMEContentObserver)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(IMEContentObserver)
 
 IMEContentObserver::IMEContentObserver()
-  : mESM(nullptr)
+  : mFirstAddedNodeOffset(0)
+  , mLastAddedNodeOffset(0)
+  , mESM(nullptr)
   , mIMENotificationRequests(nullptr)
   , mSuppressNotifications(0)
   , mPreCharacterDataChangeLength(-1)
   , mSendingNotification(NOTIFY_IME_OF_NOTHING)
   , mIsObserving(false)
   , mIMEHasFocus(false)
   , mNeedsToNotifyIMEOfFocusSet(false)
   , mNeedsToNotifyIMEOfTextChange(false)
@@ -928,16 +934,23 @@ IMEContentObserver::CharacterDataWillCha
              "mPreCharacterDataChangeLength");
 
   if (!NeedsTextChangeNotification()) {
     return;
   }
 
   mEndOfAddedTextCache.Clear();
   mStartOfRemovingTextRangeCache.Clear();
+
+  // Although we don't assume this change occurs while this is storing
+  // the range of added consecutive nodes, if it actually happens, we need to
+  // flush them since this change may occur before or in the range.  So, it's
+  // safe to flush pending computation of mTextChangeData before handling this.
+  MaybeNotifyIMEOfAddedTextDuringDocumentChange();
+
   mPreCharacterDataChangeLength =
     ContentEventHandler::GetNativeTextLength(aContent, aInfo->mChangeStart,
                                              aInfo->mChangeEnd);
   MOZ_ASSERT(mPreCharacterDataChangeLength >=
                aInfo->mChangeEnd - aInfo->mChangeStart,
              "The computed length must be same as or larger than XP length");
 }
 
@@ -950,16 +963,18 @@ IMEContentObserver::CharacterDataChanged
                "character data changed for non-text node");
 
   if (!NeedsTextChangeNotification()) {
     return;
   }
 
   mEndOfAddedTextCache.Clear();
   mStartOfRemovingTextRangeCache.Clear();
+  MOZ_ASSERT(!HasAddedNodesDuringDocumentChange(),
+    "The stored range should be flushed before actually the data is changed");
 
   int64_t removedLength = mPreCharacterDataChangeLength;
   mPreCharacterDataChangeLength = -1;
 
   MOZ_ASSERT(removedLength >= 0,
              "mPreCharacterDataChangeLength should've been set by "
              "CharacterDataWillChange()");
 
@@ -994,16 +1009,52 @@ IMEContentObserver::NotifyContentAdded(n
                                        int32_t aEndIndex)
 {
   if (!NeedsTextChangeNotification()) {
     return;
   }
 
   mStartOfRemovingTextRangeCache.Clear();
 
+  // If it's in a document change, nodes are added consecutively.  Therefore,
+  // if we cache the first node and the last node, we need to compute the
+  // range once.
+  // FYI: This is not true if the change caused by an operation in the editor.
+  if (IsInDocumentChange()) {
+    // Now, mEndOfAddedTextCache may be invalid if node is added before
+    // the last node in mEndOfAddedTextCache.  Clear it.
+    mEndOfAddedTextCache.Clear();
+    if (!HasAddedNodesDuringDocumentChange()) {
+      mFirstAddedNodeContainer = mLastAddedNodeContainer = aContainer;
+      mFirstAddedNodeOffset = aStartIndex;
+      mLastAddedNodeOffset = aEndIndex;
+      MOZ_LOG(sIMECOLog, LogLevel::Debug,
+        ("0x%p IMEContentObserver::NotifyContentAdded(), starts to store "
+         "consecutive added nodes", this));
+      return;
+    }
+    // If first node being added is not next node of the last node,
+    // notify IME of the previous range first, then, restart to cache the
+    // range.
+    if (NS_WARN_IF(!IsNextNodeOfLastAddedNode(aContainer, aStartIndex))) {
+      // Flush the old range first.
+      MaybeNotifyIMEOfAddedTextDuringDocumentChange();
+      mFirstAddedNodeContainer = aContainer;
+      mFirstAddedNodeOffset = aStartIndex;
+      MOZ_LOG(sIMECOLog, LogLevel::Debug,
+        ("0x%p IMEContentObserver::NotifyContentAdded(), starts to store "
+         "consecutive added nodes", this));
+    }
+    mLastAddedNodeContainer = aContainer;
+    mLastAddedNodeOffset = aEndIndex;
+    return;
+  }
+  MOZ_ASSERT(!HasAddedNodesDuringDocumentChange(),
+    "The cache should be cleared when document change finished");
+
   uint32_t offset = 0;
   nsresult rv = NS_OK;
   if (!mEndOfAddedTextCache.Match(aContainer, aStartIndex)) {
     mEndOfAddedTextCache.Clear();
     rv = ContentEventHandler::GetFlatTextLengthInRange(
                                 NodePosition(mRootContent, 0),
                                 NodePositionBefore(aContainer, aStartIndex),
                                 mRootContent, &offset, LINE_BREAK_TYPE_NATIVE);
@@ -1069,16 +1120,17 @@ IMEContentObserver::ContentRemoved(nsIDo
                                    int32_t aIndexInContainer,
                                    nsIContent* aPreviousSibling)
 {
   if (!NeedsTextChangeNotification()) {
     return;
   }
 
   mEndOfAddedTextCache.Clear();
+  MaybeNotifyIMEOfAddedTextDuringDocumentChange();
 
   nsINode* containerNode = NODE_FROM(aContainer, aDocument);
 
   uint32_t offset = 0;
   nsresult rv = NS_OK;
   if (!mStartOfRemovingTextRangeCache.Match(containerNode, aIndexInContainer)) {
     // At removing a child node of aContainer, we need the line break caused
     // by open tag of aContainer.  Be careful when aIndexInContainer is 0.
@@ -1154,16 +1206,19 @@ IMEContentObserver::AttributeChanged(nsI
   mEndOfAddedTextCache.Clear();
   mStartOfRemovingTextRangeCache.Clear();
 
   uint32_t postAttrChangeLength =
     ContentEventHandler::GetNativeTextLengthBefore(aElement, mRootContent);
   if (postAttrChangeLength == mPreAttrChangeLength) {
     return;
   }
+  // First, compute text range which were added during a document change.
+  MaybeNotifyIMEOfAddedTextDuringDocumentChange();
+  // Then, compute the new text changed caused by this attribute change.
   uint32_t start;
   nsresult rv =
     ContentEventHandler::GetFlatTextLengthInRange(
                            NodePosition(mRootContent, 0),
                            NodePositionBefore(aElement, 0),
                            mRootContent, &start, LINE_BREAK_TYPE_NATIVE);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
@@ -1172,25 +1227,176 @@ IMEContentObserver::AttributeChanged(nsI
   TextChangeData data(start, start + mPreAttrChangeLength,
                       start + postAttrChangeLength,
                       IsEditorHandlingEventForComposition(),
                       IsEditorComposing());
   MaybeNotifyIMEOfTextChange(data);
 }
 
 void
+IMEContentObserver::ClearAddedNodesDuringDocumentChange()
+{
+  mFirstAddedNodeContainer = mLastAddedNodeContainer = nullptr;
+  mFirstAddedNodeOffset = mLastAddedNodeOffset = 0;
+  MOZ_LOG(sIMECOLog, LogLevel::Debug,
+    ("0x%p IMEContentObserver::ClearAddedNodesDuringDocumentChange()"
+     ", finished storing consecutive nodes", this));
+}
+
+// static
+nsIContent*
+IMEContentObserver::GetChildNode(nsINode* aParent, int32_t aOffset)
+{
+  if (!aParent->HasChildren() || aOffset < 0 ||
+      aOffset >= static_cast<int32_t>(aParent->Length())) {
+    return nullptr;
+  }
+  if (!aOffset) {
+    return aParent->GetFirstChild();
+  }
+  if (aOffset == static_cast<int32_t>(aParent->Length() - 1)) {
+    return aParent->GetLastChild();
+  }
+  return aParent->GetChildAt(aOffset);
+}
+
+bool
+IMEContentObserver::IsNextNodeOfLastAddedNode(nsINode* aParent,
+                                              int32_t aOffset) const
+{
+  MOZ_ASSERT(aParent);
+  MOZ_ASSERT(aOffset >= 0 &&
+             aOffset <= static_cast<int32_t>(aParent->Length()));
+  MOZ_ASSERT(mRootContent);
+  MOZ_ASSERT(HasAddedNodesDuringDocumentChange());
+
+  // If the parent node isn't changed, we can check it only with offset.
+  if (aParent == mLastAddedNodeContainer) {
+    if (NS_WARN_IF(mLastAddedNodeOffset != aOffset)) {
+      return false;
+    }
+    return true;
+  }
+
+  // If the parent node is changed, that means that given offset should be the
+  // last added node not having next sibling.
+  if (NS_WARN_IF(mLastAddedNodeOffset !=
+                   static_cast<int32_t>(mLastAddedNodeContainer->Length()))) {
+    return false;
+  }
+
+  // If the node is aParent is a descendant of mLastAddedNodeContainer,
+  // aOffset should be 0.
+  if (mLastAddedNodeContainer == aParent->GetParent()) {
+    if (NS_WARN_IF(aOffset)) {
+      return false;
+    }
+    return true;
+  }
+
+  // Otherwise, we need to check it even with slow path.
+  nsIContent* lastAddedContent =
+    GetChildNode(mLastAddedNodeContainer, mLastAddedNodeOffset - 1);
+  if (NS_WARN_IF(!lastAddedContent)) {
+    return false;
+  }
+
+  nsIContent* nextContentOfLastAddedContent =
+    lastAddedContent->GetNextNode(mRootContent->GetParentNode());
+  if (NS_WARN_IF(!nextContentOfLastAddedContent)) {
+    return false;
+  }
+
+  nsIContent* startContent = GetChildNode(aParent, aOffset);
+  if (NS_WARN_IF(!startContent) ||
+      NS_WARN_IF(nextContentOfLastAddedContent != startContent)) {
+    return false;
+  }
+#ifdef DEBUG
+  NS_WARNING_ASSERTION(
+    !aOffset || aOffset == static_cast<int32_t>(aParent->Length() - 1),
+    "Used slow path for aParent");
+  NS_WARNING_ASSERTION(
+    !(mLastAddedNodeOffset - 1) ||
+    mLastAddedNodeOffset ==
+      static_cast<int32_t>(mLastAddedNodeContainer->Length()),
+    "Used slow path for mLastAddedNodeContainer");
+#endif // #ifdef DEBUG
+  return true;
+}
+
+void
+IMEContentObserver::MaybeNotifyIMEOfAddedTextDuringDocumentChange()
+{
+  if (!HasAddedNodesDuringDocumentChange()) {
+    return;
+  }
+
+  MOZ_LOG(sIMECOLog, LogLevel::Debug,
+    ("0x%p IMEContentObserver::MaybeNotifyIMEOfAddedTextDuringDocumentChange()"
+     ", flushing stored consecutive nodes", this));
+
+  // Notify IME of text change which is caused by added nodes now.
+
+  // First, compute offset of start of first added node from start of the
+  // editor.
+  uint32_t offset;
+  nsresult rv =
+    ContentEventHandler::GetFlatTextLengthInRange(
+                            NodePosition(mRootContent, 0),
+                            NodePosition(mFirstAddedNodeContainer,
+                                         mFirstAddedNodeOffset),
+                            mRootContent, &offset, LINE_BREAK_TYPE_NATIVE);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    ClearAddedNodesDuringDocumentChange();
+    return;
+  }
+
+  // Next, compute the text length of added nodes.
+  uint32_t length;
+  rv =
+    ContentEventHandler::GetFlatTextLengthInRange(
+                           NodePosition(mFirstAddedNodeContainer,
+                                        mFirstAddedNodeOffset),
+                           NodePosition(mLastAddedNodeContainer,
+                                        mLastAddedNodeOffset),
+                           mRootContent, &length, LINE_BREAK_TYPE_NATIVE);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    ClearAddedNodesDuringDocumentChange();
+    return;
+  }
+
+  // Finally, try to notify IME of the range.
+  TextChangeData data(offset, offset, offset + length,
+                      IsEditorHandlingEventForComposition(),
+                      IsEditorComposing());
+  MaybeNotifyIMEOfTextChange(data);
+  ClearAddedNodesDuringDocumentChange();
+}
+
+void
 IMEContentObserver::BeginDocumentUpdate()
 {
-  // TODO: Implement this later.
+  MOZ_LOG(sIMECOLog, LogLevel::Debug,
+    ("0x%p IMEContentObserver::BeginDocumentUpdate(), "
+     "HasAddedNodesDuringDocumentChange()=%s",
+     this, ToChar(HasAddedNodesDuringDocumentChange())));
+
+  MOZ_ASSERT(!HasAddedNodesDuringDocumentChange());
 }
 
 void
 IMEContentObserver::EndDocumentUpdate()
 {
-  // TODO: Implement this later.
+  MOZ_LOG(sIMECOLog, LogLevel::Debug,
+    ("0x%p IMEContentObserver::EndDocumentUpdate(), "
+     "HasAddedNodesDuringDocumentChange()=%s",
+     this, ToChar(HasAddedNodesDuringDocumentChange())));
+
+  MaybeNotifyIMEOfAddedTextDuringDocumentChange();
 }
 
 void
 IMEContentObserver::SuppressNotifyingIME()
 {
   mSuppressNotifications++;
 
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
--- a/dom/events/IMEContentObserver.h
+++ b/dom/events/IMEContentObserver.h
@@ -177,21 +177,71 @@ private:
   void OnIMEReceivedFocus();
   void Clear();
   bool IsObservingContent(nsPresContext* aPresContext,
                           nsIContent* aContent) const;
   bool IsReflowLocked() const;
   bool IsSafeToNotifyIME() const;
   bool IsEditorComposing() const;
 
+  /**
+   * nsINode::GetChildAt() is slow.  So, this avoids to use it if it's
+   * first child or last child of aParent.
+   */
+  static nsIContent* GetChildNode(nsINode* aParent, int32_t aOffset);
+
   // Following methods are called by DocumentObserver when
   // beginning to update the contents and ending updating the contents.
   void BeginDocumentUpdate();
   void EndDocumentUpdate();
 
+  // Following methods manages added nodes during a document change.
+
+  /**
+   * MaybeNotifyIMEOfAddedTextDuringDocumentChange() may send text change
+   * notification caused by the nodes added between mFirstAddedNodeOffset in
+   * mFirstAddedNodeContainer and mLastAddedNodeOffset in
+   * mLastAddedNodeContainer and forgets the range.
+   */
+  void MaybeNotifyIMEOfAddedTextDuringDocumentChange();
+
+  /**
+   * IsInDocumentChange() returns true while the DOM tree is being modified
+   * with mozAutoDocUpdate.  E.g., it's being modified by setting innerHTML or
+   * insertAdjacentHTML().  This returns false when user types something in
+   * the focused editor editor.
+   */
+  bool IsInDocumentChange() const
+  {
+    return mDocumentObserver && mDocumentObserver->IsUpdating();
+  }
+
+  /**
+   * Forget the range of added nodes during a document change.
+   */
+  void ClearAddedNodesDuringDocumentChange();
+
+  /**
+   * HasAddedNodesDuringDocumentChange() returns true when this stores range
+   * of nodes which were added into the DOM tree during a document change but
+   * have not been sent to IME.  Note that this should always return false when
+   * IsInDocumentChange() returns false.
+   */
+  bool HasAddedNodesDuringDocumentChange() const
+  {
+    return mFirstAddedNodeContainer && mLastAddedNodeContainer;
+  }
+
+  /**
+   * Returns true if the node at aOffset in aParent is next node of the node at
+   * mLastAddedNodeOffset in mLastAddedNodeContainer in pre-order tree
+   * traversal of the DOM.
+   */
+  bool IsNextNodeOfLastAddedNode(nsINode* aParent, int32_t aOffset) const;
+
   void PostFocusSetNotification();
   void MaybeNotifyIMEOfFocusSet();
   void PostTextChangeNotification();
   void MaybeNotifyIMEOfTextChange(const TextChangeDataBase& aTextChangeData);
   void CancelNotifyingIMEOfTextChange();
   void PostSelectionChangeNotification();
   void MaybeNotifyIMEOfSelectionChange(bool aCausedByComposition,
                                        bool aCausedBySelectionEvent,
@@ -409,16 +459,37 @@ private:
   // handled by the editor and no other mutation (e.g., removing node)
   // occur.
   FlatTextCache mEndOfAddedTextCache;
   // mStartOfRemovingTextRangeCache caches text length from the start of content
   // to the start of the last removed content only while an edit action is being
   // handled by the editor and no other mutation (e.g., adding node) occur.
   FlatTextCache mStartOfRemovingTextRangeCache;
 
+  // mFirstAddedNodeContainer is parent node of first added node in current
+  // document change.  So, this is not nullptr only when a node was added
+  // during a document change and the change has not been included into
+  // mTextChangeData yet.
+  // Note that this shouldn't be in cycle collection since this is not nullptr
+  // only during a document change.
+  nsCOMPtr<nsINode> mFirstAddedNodeContainer;
+  // mLastAddedNodeContainer is parent node of last added node in current
+  // document change.  So, this is not nullptr only when a node was added
+  // during a document change and the change has not been included into
+  // mTextChangeData yet.
+  // Note that this shouldn't be in cycle collection since this is not nullptr
+  // only during a document change.
+  nsCOMPtr<nsINode> mLastAddedNodeContainer;
+  // mFirstAddedNodeOffset is offset of first added node in
+  // mFirstAddedNodeContainer.
+  int32_t mFirstAddedNodeOffset;
+  // mLastAddedNodeOffset is offset of *after* last added node in
+  // mLastAddedNodeContainer.  I.e., the index of last added node + 1.
+  int32_t mLastAddedNodeOffset;
+
   TextChangeData mTextChangeData;
 
   // mSelectionData is the last selection data which was notified.  The
   // selection information is modified by UpdateSelectionCache().  The reason
   // of the selection change is modified by MaybeNotifyIMEOfSelectionChange().
   SelectionChangeData mSelectionData;
 
   EventStateManager* mESM;
--- a/widget/tests/window_composition_text_querycontent.xul
+++ b/widget/tests/window_composition_text_querycontent.xul
@@ -7505,23 +7505,17 @@ function* runIMEContentObserverTest()
     yield flushNotifications();
 
     // "a[]"
     var description = aDescription + "typing 'a'";
     notifications = [];
     synthesizeKey("a", { code: "KeyA" }, win, callback);
     yield waitUntilNotificationsReceived();
     ensureToRemovePrecedingPositionChangeNotification();
-    if (isDefaultParagraphSeparatorBlock) {
-      // XXX This must detect a bug.  The offset of inserting "a" into the first block should be
-      //     after the line breaker caused by the first block.
-      checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer - kLFLen, removedLength: 0, addedLength: 1 });
-    } else {
-      checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 0, addedLength: 1 });
-    }
+    checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 0, addedLength: 1 });
     checkSelectionChangeNotification(notifications[1], description, { offset: 1 + offsetAtContainer, text: "" });
     checkPositionChangeNotification(notifications[2], description);
     dumpUnexpectedNotifications(description, 3);
 
     // "ab[]"
     description = aDescription + "typing 'b'";
     notifications = [];
     synthesizeKey("b", { code: "KeyB" }, win, callback);
@@ -7609,23 +7603,17 @@ function* runIMEContentObserverTest()
     dumpUnexpectedNotifications(description, 3);
 
     // "[]"
     description = aDescription + "deleting following 'c' with pressing Delete";
     notifications = [];
     synthesizeKey("KEY_Delete", { code: "Delete" }, win, callback);
     yield waitUntilNotificationsReceived();
     ensureToRemovePrecedingPositionChangeNotification();
-    if (isDefaultParagraphSeparatorBlock) {
-      // XXX Making a block empty causes removing the block once.
-      //     However, after that, new block is inserted with <br>.
-      checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer - kLFLen, removedLength: getNativeText("\nc").length, addedLength: kLFLen * 2 });
-    } else {
-      checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 1, addedLength: kLFLen });
-    }
+    checkTextChangeNotification(notifications[0], description, { offset: 0 + offsetAtContainer, removedLength: 1, addedLength: kLFLen });
     checkPositionChangeNotification(notifications[1], description);
     dumpUnexpectedNotifications(description, 2);
 
     // "abc[]"
     synthesizeKey("a", { code: "KeyA" }, win, callback);
     synthesizeKey("b", { code: "KeyB" }, win, callback);
     synthesizeKey("c", { code: "KeyC" }, win, callback);
     yield flushNotifications();