Bug 1484110 - part 1: Create HTMLEditor::RefereshEditingUI() for internal use of nsIHTMLEditor::CheckSelectionStateForAnonymousButtons() r=m_kato

HTMLEditor::CheckSelectionStateForAnonymousButtons() is called a lot internally.
Especially, its virtual call cost may make damage to our performance since
it's called from a selection listener.

So, we should create non-virtual method, RefereshEditingUI() for internal use.

editor/libeditor/EditorBase.cpp

@@ -4200,10 +4200,8 @@ EditorBase::EndUpdateViewBatch()
     return;
   }
 
-  DebugOnly<nsresult> rv =
-    htmlEditor->CheckSelectionStateForAnonymousButtons(selection);
-  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
-    "CheckSelectionStateForAnonymousButtons() failed");
+  DebugOnly<nsresult> rv = htmlEditor->RefereshEditingUI(*selection);
+  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "RefereshEditingUI() failed");
 }
 
 TextComposition*

editor/libeditor/HTMLAbsPositionEditor.cpp

@@ -418,7 +418,11 @@ HTMLEditor::EndMoving()
   if (!selection) {
     return NS_ERROR_NOT_INITIALIZED;
   }
-  return CheckSelectionStateForAnonymousButtons(selection);
+  nsresult rv = RefereshEditingUI(*selection);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return NS_OK;
 }
 nsresult
 HTMLEditor::SetFinalPosition(int32_t aX,

editor/libeditor/HTMLAnonymousNodeEditor.cpp

@@ -282,21 +282,26 @@ HTMLEditor::DeleteRefToAnonymousNode(ManualNACPtr aContent,
   // The ManualNACPtr destructor will invoke UnbindFromTree.
 }
 
-// The following method is mostly called by a selection listener. When a
-// selection change is notified, the method is called to check if resizing
-// handles, a grabber and/or inline table editing UI need to be displayed
-// or refreshed
 NS_IMETHODIMP
 HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection)
 {
   if (NS_WARN_IF(!aSelection)) {
     return NS_ERROR_INVALID_ARG;
   }
+  nsresult rv = RefereshEditingUI(*aSelection);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return NS_OK;
+}
 
+nsresult
+HTMLEditor::RefereshEditingUI(Selection& aSelection)
+{
   // early way out if all contextual UI extensions are disabled
-  if (NS_WARN_IF(!IsObjectResizerEnabled() &&
-                 !IsAbsolutePositionEditorEnabled() &&
-                 !IsInlineTableEditorEnabled())) {
+  if (!IsObjectResizerEnabled() &&
+      !IsAbsolutePositionEditorEnabled() &&
+      !IsInlineTableEditorEnabled()) {
     return NS_OK;
   }
 
@@ -306,7 +311,7 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection)
   }
 
   // let's get the containing element of the selection
-  RefPtr<Element> focusElement = GetSelectionContainerElement(*aSelection);
+  RefPtr<Element> focusElement = GetSelectionContainerElement(aSelection);
   if (NS_WARN_IF(!focusElement)) {
     return NS_OK;
   }
@@ -331,7 +336,7 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection)
     // Resizing or Inline Table Editing is enabled, we need to check if the
     // selection is contained in a table cell
     cellElement =
-      GetElementOrParentByTagNameAtSelection(*aSelection, *nsGkAtoms::td);
+      GetElementOrParentByTagNameAtSelection(aSelection, *nsGkAtoms::td);
   }
 
   if (IsObjectResizerEnabled() && cellElement) {
@@ -342,6 +347,9 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection)
     if (nsGkAtoms::img != focusTagAtom) {
       // the element container of the selection is not an image, so we'll show
       // the resizers around the table
+      // XXX There may be a bug.  cellElement may be not in <table> in invalid
+      //     tree.  So, perhaps, GetEnclosingTable() returns nullptr, we should
+      //     not set focusTagAtom to nsGkAtoms::table.
       focusElement = GetEnclosingTable(cellElement);
       focusTagAtom = nsGkAtoms::table;
     }
@@ -369,15 +377,24 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection)
 
   if (IsObjectResizerEnabled() && mResizedObject &&
       mResizedObject != focusElement) {
+    // Perhaps, even if HideResizers() failed, we should try to hide inline
+    // table editing UI.  However, it returns error only when we cannot do
+    // anything.  So, it's okay for now.
     nsresult rv = HideResizers();
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     NS_ASSERTION(!mResizedObject, "HideResizers failed");
   }
 
   if (mIsInlineTableEditingEnabled && mInlineEditedCell &&
       mInlineEditedCell != cellElement) {
+    // XXX HideInlineTableEditingUI() won't return error.  Should be change it
+    //     void later.
     nsresult rv = HideInlineTableEditingUI();
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed");
   }
 

editor/libeditor/HTMLEditor.cpp

@@ -434,9 +434,9 @@ HTMLEditor::NotifySelectionChanged(nsIDocument* aDocument,
     typeInState->OnSelectionChange(*aSelection);
 
     // We used a class which derived from nsISelectionListener to call
-    // HTMLEditor::CheckSelectionStateForAnonymousButtons().  The lifetime of
-    // the class was exactly same as mTypeInState.  So, call it only when
-    // mTypeInState is not nullptr.
+    // HTMLEditor::RefereshEditingUI().  The lifetime of the class was
+    // exactly same as mTypeInState.  So, call it only when mTypeInState
+    // is not nullptr.
     if ((aReason & (nsISelectionListener::MOUSEDOWN_REASON |
                     nsISelectionListener::KEYPRESS_REASON |
                     nsISelectionListener::SELECTALL_REASON)) && aSelection) {
@@ -445,7 +445,8 @@ HTMLEditor::NotifySelectionChanged(nsIDocument* aDocument,
       // FYI: This is an XPCOM method.  So, the caller, Selection, guarantees
       //      the lifetime of this instance.  So, don't need to grab this with
       //      local variable.
-      CheckSelectionStateForAnonymousButtons(aSelection);
+      DebugOnly<nsresult> rv = RefereshEditingUI(*aSelection);
+      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "RefereshEditingUI() failed");
     }
   }
 

editor/libeditor/HTMLEditor.h

@@ -1510,6 +1510,13 @@ protected: // Shouldn't be used by friend classes
   void DeleteRefToAnonymousNode(ManualNACPtr aContent,
                                 nsIPresShell* aShell);
 
+  /**
+   * RefereshEditingUI() may refresh editing UIs for current Selection, focus,
+   * etc.  If this shows or hides some UIs, it causes reflow.  So, this is
+   * not safe method.
+   */
+  nsresult RefereshEditingUI(Selection& aSelection);
+
   nsresult ShowResizersInner(Element& aResizedElement);
 
   /**

editor/nsIHTMLEditor.idl

@@ -397,9 +397,14 @@ interface nsIHTMLEditor : nsISupports
   attribute boolean isCSSEnabled;
 
   /**
-   * Checks if the anonymous nodes created by the HTML editor have to be
-   * refreshed or hidden depending on a possible new state of the selection
-   * @param aSelection [IN] a selection
+   * checkSelectionStateForAnonymousButtons() may refresh editing UI such as
+   * resizers, inline-table-editing UI, absolute positioning UI for current
+   * Selection and focus state.  When this method shows or hides UI, the
+   * editor (and/or its document/window) could be broken by mutation observers.
+   * FYI: Current user in script is only BlueGriffon.
+   *
+   * @param aSelection  Selection instance for the normal selection of the
+   *                    document.
    */
   void checkSelectionStateForAnonymousButtons(in Selection aSelection);