Bug 1572685 - part 8: Move HTMLEditRules::mCachedStyles to TopLevelEditSubAction r=m_kato

This patch makes `StyleCache` not inherit `ItemProp` because `MOZ_COUNT_CTOR`
and `MOZ_COUNT_DTOR` do not work well with `AutoTArray` and there is no reason
to do that since nobody treat `StyleCache` instance with `ItemProp` pointer.

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

--HG--
extra : moz-landing-system : lando

editor/libeditor/EditorBase.h

@@ -12,6 +12,7 @@
 #include "mozilla/Maybe.h"               // for Maybe
 #include "mozilla/OwningNonNull.h"       // for OwningNonNull
 #include "mozilla/PresShell.h"           // for PresShell
+#include "mozilla/TypeInState.h"         // for PropItem, StyleCache
 #include "mozilla/RangeBoundary.h"       // for RawRangeBoundary, RangeBoundary
 #include "mozilla/SelectionState.h"      // for RangeUpdater, etc.
 #include "mozilla/StyleSheet.h"          // for StyleSheet
@@ -619,6 +620,12 @@ class EditorBase : public nsIEditor,
     // the range while we're changing the DOM tree.
     RefPtr<RangeItem> mSelectedRange;
 
+    // XXX In strict speaking, mCachedInlineStyles isn't enough to cache inline
+    //     styles because inline style can be specified with "style" attribute
+    //     and/or CSS in <style> elements or CSS files.  So, we need to look
+    //     for better implementation about this.
+    AutoStyleCacheArray mCachedInlineStyles;
+
     // If we tried to delete selection, set to true.
     bool mDidDeleteSelection;
 
@@ -655,6 +662,7 @@ class EditorBase : public nsIEditor,
       }
       mNewBlockElement = nullptr;
       mSelectedRange->Clear();
+      mCachedInlineStyles.Clear();
       mDidDeleteSelection = false;
       mDidDeleteNonCollapsedRange = false;
       mDidDeleteEmptyParentBlocks = false;

editor/libeditor/HTMLEditRules.cpp

@@ -203,31 +203,6 @@ void HTMLEditRules::InitFields() {
   mReturnInEmptyLIKillsList = true;
   mUtilRange = nullptr;
   mJoinOffset = 0;
-
-  InitStyleCacheArray(mCachedStyles);
-}
-
-void HTMLEditRules::InitStyleCacheArray(
-    StyleCache aStyleCache[SIZE_STYLE_TABLE]) {
-  aStyleCache[0] = StyleCache(nsGkAtoms::b, nullptr);
-  aStyleCache[1] = StyleCache(nsGkAtoms::i, nullptr);
-  aStyleCache[2] = StyleCache(nsGkAtoms::u, nullptr);
-  aStyleCache[3] = StyleCache(nsGkAtoms::font, nsGkAtoms::face);
-  aStyleCache[4] = StyleCache(nsGkAtoms::font, nsGkAtoms::size);
-  aStyleCache[5] = StyleCache(nsGkAtoms::font, nsGkAtoms::color);
-  aStyleCache[6] = StyleCache(nsGkAtoms::tt, nullptr);
-  aStyleCache[7] = StyleCache(nsGkAtoms::em, nullptr);
-  aStyleCache[8] = StyleCache(nsGkAtoms::strong, nullptr);
-  aStyleCache[9] = StyleCache(nsGkAtoms::dfn, nullptr);
-  aStyleCache[10] = StyleCache(nsGkAtoms::code, nullptr);
-  aStyleCache[11] = StyleCache(nsGkAtoms::samp, nullptr);
-  aStyleCache[12] = StyleCache(nsGkAtoms::var, nullptr);
-  aStyleCache[13] = StyleCache(nsGkAtoms::cite, nullptr);
-  aStyleCache[14] = StyleCache(nsGkAtoms::abbr, nullptr);
-  aStyleCache[15] = StyleCache(nsGkAtoms::acronym, nullptr);
-  aStyleCache[16] = StyleCache(nsGkAtoms::backgroundColor, nullptr);
-  aStyleCache[17] = StyleCache(nsGkAtoms::sub, nullptr);
-  aStyleCache[18] = StyleCache(nsGkAtoms::sup, nullptr);
 }
 
 nsresult HTMLEditRules::Init(TextEditor* aTextEditor) {
@@ -659,7 +634,9 @@ nsresult HTMLEditRules::AfterEditInner() {
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
-      ClearCachedStyles();
+      HTMLEditorRef()
+          .TopLevelEditSubActionDataRef()
+          .mCachedInlineStyles.Clear();
     }
   }
 
@@ -1375,7 +1352,7 @@ nsresult HTMLEditRules::WillInsert(bool* aCancel) {
   // exceptions
   if (!IsStyleCachePreservingSubAction(
           HTMLEditorRef().GetTopLevelEditSubAction())) {
-    ClearCachedStyles();
+    HTMLEditorRef().TopLevelEditSubActionDataRef().mCachedInlineStyles.Clear();
   }
   return NS_OK;
 }
@@ -7937,7 +7914,9 @@ nsresult HTMLEditRules::ReturnInHeader(Element& aHeader, nsINode& aNode,
       sibling = HTMLEditorRef().GetNextHTMLSibling(aHeader.GetNextSibling());
     }
     if (!sibling || !sibling->IsHTMLElement(nsGkAtoms::br)) {
-      ClearCachedStyles();
+      HTMLEditorRef()
+          .TopLevelEditSubActionDataRef()
+          .mCachedInlineStyles.Clear();
       HTMLEditorRef().mTypeInState->ClearAllProps();
 
       // Create a paragraph
@@ -9190,25 +9169,27 @@ nsresult HTMLEditRules::CacheInlineStyles(nsINode* aNode) {
     return NS_ERROR_INVALID_ARG;
   }
 
-  nsresult rv = GetInlineStyles(aNode, mCachedStyles);
+  nsresult rv = GetInlineStyles(
+      aNode,
+      HTMLEditorRef().TopLevelEditSubActionDataRef().mCachedInlineStyles);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return NS_OK;
 }
 
-nsresult HTMLEditRules::GetInlineStyles(
+nsresult HTMLEditRules::GetInlineStyles(nsINode* aNode,
-    nsINode* aNode, StyleCache aStyleCache[SIZE_STYLE_TABLE]) {
+                                        AutoStyleCacheArray& aStyleCacheArray) {
   MOZ_ASSERT(IsEditorDataAvailable());
   MOZ_ASSERT(aNode);
 
   bool useCSS = HTMLEditorRef().IsCSSEnabled();
 
-  for (size_t j = 0; j < SIZE_STYLE_TABLE; ++j) {
+  for (StyleCache& styleCache : aStyleCacheArray) {
     // If type-in state is set, don't intervene
     bool typeInSet, unused;
     HTMLEditorRef().mTypeInState->GetTypingState(
-        typeInSet, unused, aStyleCache[j].tag, aStyleCache[j].attr, nullptr);
+        typeInSet, unused, styleCache.mTag, styleCache.mAttr, nullptr);
     if (typeInSet) {
       continue;
     }
@@ -9216,21 +9197,21 @@ nsresult HTMLEditRules::GetInlineStyles(
     bool isSet = false;
     nsAutoString outValue;
     // Don't use CSS for <font size>, we don't support it usefully (bug 780035)
-    if (!useCSS || (aStyleCache[j].tag == nsGkAtoms::font &&
+    if (!useCSS || (styleCache.mTag == nsGkAtoms::font &&
-                    aStyleCache[j].attr == nsGkAtoms::size)) {
+                    styleCache.mAttr == nsGkAtoms::size)) {
       isSet = HTMLEditorRef().IsTextPropertySetByContent(
-          aNode, aStyleCache[j].tag, aStyleCache[j].attr, nullptr, &outValue);
+          aNode, styleCache.mTag, styleCache.mAttr, nullptr, &outValue);
     } else {
       isSet = CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(
-          aNode, aStyleCache[j].tag, aStyleCache[j].attr, outValue,
+          aNode, styleCache.mTag, styleCache.mAttr, outValue,
           CSSEditUtils::eComputed);
       if (NS_WARN_IF(!CanHandleEditAction())) {
         return NS_ERROR_EDITOR_DESTROYED;
       }
     }
     if (isSet) {
-      aStyleCache[j].mPresent = true;
+      styleCache.mPresent = true;
-      aStyleCache[j].value.Assign(outValue);
+      styleCache.mValue.Assign(outValue);
     }
   }
   return NS_OK;
@@ -9262,15 +9243,17 @@ nsresult HTMLEditRules::ReapplyCachedStyles() {
     return NS_OK;
   }
 
-  StyleCache styleAtInsertionPoint[SIZE_STYLE_TABLE];
+  AutoStyleCacheArray styleCacheArrayAtInsertionPoint;
-  InitStyleCacheArray(styleAtInsertionPoint);
+  nsresult rv = GetInlineStyles(selNode, styleCacheArrayAtInsertionPoint);
-  nsresult rv = GetInlineStyles(selNode, styleAtInsertionPoint);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv == NS_ERROR_EDITOR_DESTROYED ? NS_ERROR_EDITOR_DESTROYED : NS_OK;
   }
 
-  for (size_t i = 0; i < SIZE_STYLE_TABLE; ++i) {
+  for (size_t i = 0; i < styleCacheArrayAtInsertionPoint.Length(); ++i) {
-    if (mCachedStyles[i].mPresent) {
+    StyleCache& styleCacheAtInsertionPoint = styleCacheArrayAtInsertionPoint[i];
+    StyleCache& styleCacheBeforeEdit =
+        HTMLEditorRef().TopLevelEditSubActionDataRef().mCachedInlineStyles[i];
+    if (styleCacheBeforeEdit.mPresent) {
       bool bFirst, bAny, bAll;
       bFirst = bAny = bAll = false;
 
@@ -9278,8 +9261,8 @@ nsresult HTMLEditRules::ReapplyCachedStyles() {
       if (useCSS) {
         // check computed style first in css case
         bAny = CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(
-            selNode, mCachedStyles[i].tag, mCachedStyles[i].attr, curValue,
+            selNode, styleCacheBeforeEdit.mTag, styleCacheBeforeEdit.mAttr,
-            CSSEditUtils::eComputed);
+            curValue, CSSEditUtils::eComputed);
         if (NS_WARN_IF(!CanHandleEditAction())) {
           return NS_ERROR_EDITOR_DESTROYED;
         }
@@ -9287,8 +9270,8 @@ nsresult HTMLEditRules::ReapplyCachedStyles() {
       if (!bAny) {
         // then check typeinstate and html style
         nsresult rv = HTMLEditorRef().GetInlinePropertyBase(
-            *mCachedStyles[i].tag, mCachedStyles[i].attr,
+            *styleCacheBeforeEdit.mTag, styleCacheBeforeEdit.mAttr,
-            &(mCachedStyles[i].value), &bFirst, &bAny, &bAll, &curValue);
+            &styleCacheBeforeEdit.mValue, &bFirst, &bAny, &bAll, &curValue);
         if (NS_WARN_IF(!CanHandleEditAction())) {
           return NS_ERROR_EDITOR_DESTROYED;
         }
@@ -9300,11 +9283,11 @@ nsresult HTMLEditRules::ReapplyCachedStyles() {
       // mTypeInState when same style isn't applied to the node already.
       if ((!bAny || IsStyleCachePreservingSubAction(
                         HTMLEditorRef().GetTopLevelEditSubAction())) &&
-          (!styleAtInsertionPoint[i].mPresent ||
+          (!styleCacheAtInsertionPoint.mPresent ||
-           styleAtInsertionPoint[i].value != mCachedStyles[i].value)) {
+           styleCacheAtInsertionPoint.mValue != styleCacheBeforeEdit.mValue)) {
-        HTMLEditorRef().mTypeInState->SetProp(mCachedStyles[i].tag,
+        HTMLEditorRef().mTypeInState->SetProp(styleCacheBeforeEdit.mTag,
-                                              mCachedStyles[i].attr,
+                                              styleCacheBeforeEdit.mAttr,
-                                              mCachedStyles[i].value);
+                                              styleCacheBeforeEdit.mValue);
       }
     }
   }
@@ -9312,14 +9295,6 @@ nsresult HTMLEditRules::ReapplyCachedStyles() {
   return NS_OK;
 }
 
-void HTMLEditRules::ClearCachedStyles() {
-  // clear the mPresent bits in mCachedStyles array
-  for (size_t j = 0; j < SIZE_STYLE_TABLE; j++) {
-    mCachedStyles[j].mPresent = false;
-    mCachedStyles[j].value.Truncate();
-  }
-}
-
 nsresult
 HTMLEditRules::InsertBRElementToEmptyListItemsAndTableCellsInChangedRange() {
   MOZ_ASSERT(IsEditorDataAvailable());

editor/libeditor/HTMLEditRules.h

@@ -10,6 +10,7 @@
 #include "mozilla/EditorDOMPoint.h"  // for EditorDOMPoint
 #include "mozilla/SelectionState.h"
 #include "mozilla/TextEditRules.h"
+#include "mozilla/TypeInState.h"  // for AutoStyleCacheArray
 #include "nsCOMPtr.h"
 #include "nsIEditor.h"
 #include "nsIHTMLEditor.h"
@@ -18,7 +19,6 @@
 #include "nscore.h"
 
 class nsAtom;
-class nsIEditor;
 class nsINode;
 class nsRange;
 
@@ -36,24 +36,6 @@ class Element;
 class Selection;
 }  // namespace dom
 
-struct StyleCache final : public PropItem {
-  bool mPresent;
-
-  StyleCache() : PropItem(), mPresent(false) { MOZ_COUNT_CTOR(StyleCache); }
-
-  StyleCache(nsAtom* aTag, nsAtom* aAttr, const nsAString& aValue)
-      : PropItem(aTag, aAttr, aValue), mPresent(false) {
-    MOZ_COUNT_CTOR(StyleCache);
-  }
-
-  StyleCache(nsAtom* aTag, nsAtom* aAttr)
-      : PropItem(aTag, aAttr, EmptyString()), mPresent(false) {
-    MOZ_COUNT_CTOR(StyleCache);
-  }
-
-  ~StyleCache() { MOZ_COUNT_DTOR(StyleCache); }
-};
-
 /**
  * Same as TextEditRules, any methods which may modify the DOM tree or
  * Selection should be marked as MOZ_MUST_USE and return nsresult directly
@@ -68,8 +50,6 @@ struct StyleCache final : public PropItem {
  * NS_ERROR_EDITOR_DESTROYED.
  */
 
-#define SIZE_STYLE_TABLE 19
-
 class HTMLEditRules : public TextEditRules {
  public:
   HTMLEditRules();
@@ -1140,9 +1120,9 @@ class HTMLEditRules : public TextEditRules {
   MOZ_MUST_USE nsresult RemoveListStructure(Element& aListElement);
 
   /**
-   * CacheInlineStyles() caches style of aNode into mCachedStyles.
+   * CacheInlineStyles() caches style of aNode into mCachedInlineStyles of
-   * This may cause flushing layout at retrieving computed value of CSS
+   * TopLevelEditSubAction.  This may cause flushing layout at retrieving
-   * properties.
+   * computed value of CSS properties.
    */
   MOZ_MUST_USE nsresult CacheInlineStyles(nsINode* aNode);
 
@@ -1332,19 +1312,13 @@ class HTMLEditRules : public TextEditRules {
    */
   MOZ_CAN_RUN_SCRIPT void DocumentModifiedWorker();
 
-  /**
-   * InitStyleCacheArray() initializes aStyleCache for usable with
-   * GetInlineStyles().
-   */
-  void InitStyleCacheArray(StyleCache aStyleCache[SIZE_STYLE_TABLE]);
-
   /**
    * GetInlineStyles() retrieves the style of aNode and modifies each item of
-   * aStyleCache.  This might cause flushing layout at retrieving computed
+   * aStyleCacheArray.  This might cause flushing layout at retrieving computed
    * values of CSS properties.
    */
-  MOZ_MUST_USE nsresult
+  MOZ_MUST_USE nsresult GetInlineStyles(nsINode* aNode,
-  GetInlineStyles(nsINode* aNode, StyleCache aStyleCache[SIZE_STYLE_TABLE]);
+                                        AutoStyleCacheArray& aStyleCacheArray);
 
  protected:
   HTMLEditor* mHTMLEditor;
@@ -1356,12 +1330,6 @@ class HTMLEditRules : public TextEditRules {
   // Need to remember an int across willJoin/didJoin...
   uint32_t mJoinOffset;
 
-  // XXX In strict speaking, mCachedStyles isn't enough to cache inline styles
-  //     because inline style can be specified with "style" attribute and/or
-  //     CSS in <style> elements or CSS files.  So, we need to look for better
-  //     implementation about this.
-  StyleCache mCachedStyles[SIZE_STYLE_TABLE];
-
   friend class NS_CYCLE_COLLECTION_CLASSNAME(TextEditRules);
 };
 

editor/libeditor/TypeInState.cpp

@@ -310,19 +310,4 @@ bool TypeInState::FindPropInList(nsAtom* aProp, nsAtom* aAttr,
   return false;
 }
 
-/********************************************************************
- * mozilla::PropItem: helper struct for mozilla::TypeInState
- *******************************************************************/
-
-PropItem::PropItem() : tag(nullptr), attr(nullptr) { MOZ_COUNT_CTOR(PropItem); }
-
-PropItem::PropItem(nsAtom* aTag, nsAtom* aAttr, const nsAString& aValue)
-    : tag(aTag),
-      attr(aAttr != nsGkAtoms::_empty ? aAttr : nullptr),
-      value(aValue) {
-  MOZ_COUNT_CTOR(PropItem);
-}
-
-PropItem::~PropItem() { MOZ_COUNT_DTOR(PropItem); }
-
 }  // namespace mozilla

editor/libeditor/TypeInState.h

@@ -3,13 +3,14 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#ifndef TypeInState_h
+#ifndef mozilla_TypeInState_h
-#define TypeInState_h
+#define mozilla_TypeInState_h
 
 #include "mozilla/EditorDOMPoint.h"
 #include "mozilla/UniquePtr.h"
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
+#include "nsGkAtoms.h"
 #include "nsISupportsImpl.h"
 #include "nsString.h"
 #include "nsTArray.h"
@@ -35,9 +36,62 @@ struct PropItem {
   nsAtom* attr;
   nsString value;
 
-  PropItem();
+  PropItem() : tag(nullptr), attr(nullptr) { MOZ_COUNT_CTOR(PropItem); }
-  PropItem(nsAtom* aTag, nsAtom* aAttr, const nsAString& aValue);
+  PropItem(nsAtom* aTag, nsAtom* aAttr, const nsAString& aValue)
-  ~PropItem();
+      : tag(aTag),
+        attr(aAttr != nsGkAtoms::_empty ? aAttr : nullptr),
+        value(aValue) {
+    MOZ_COUNT_CTOR(PropItem);
+  }
+  ~PropItem() { MOZ_COUNT_DTOR(PropItem); }
+};
+
+struct MOZ_STACK_CLASS StyleCache final {
+  nsAtom* mTag;
+  nsAtom* mAttr;
+  nsString mValue;
+  bool mPresent;
+
+  StyleCache() : mTag(nullptr), mAttr(nullptr), mPresent(false) {}
+  StyleCache(nsAtom* aTag, nsAtom* aAttr)
+      : mTag(aTag), mAttr(aAttr), mPresent(false) {}
+
+  inline void Clear() {
+    mPresent = false;
+    mValue.Truncate();
+  }
+};
+
+class MOZ_STACK_CLASS AutoStyleCacheArray final
+    : public AutoTArray<StyleCache, 19> {
+ public:
+  AutoStyleCacheArray() {
+    AppendElement(StyleCache(nsGkAtoms::b, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::i, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::u, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::font, nsGkAtoms::face));
+    AppendElement(StyleCache(nsGkAtoms::font, nsGkAtoms::size));
+    AppendElement(StyleCache(nsGkAtoms::font, nsGkAtoms::color));
+    AppendElement(StyleCache(nsGkAtoms::tt, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::em, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::strong, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::dfn, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::code, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::samp, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::var, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::cite, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::abbr, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::acronym, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::backgroundColor, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::sub, nullptr));
+    AppendElement(StyleCache(nsGkAtoms::sup, nullptr));
+  }
+
+  void Clear() {
+    for (auto& styleCache : *this) {
+      styleCache.Clear();
+    }
+  }
 };
 
 class TypeInState final {
@@ -102,4 +156,4 @@ class TypeInState final {
 
 }  // namespace mozilla
 
-#endif  // #ifndef TypeInState_h
+#endif  // #ifndef mozilla_TypeInState_h

editor/libeditor/moz.build

@@ -29,6 +29,7 @@ EXPORTS.mozilla += [
     'SelectionState.h',
     'TextEditor.h',
     'TextEditRules.h',
+    'TypeInState.h',
 ]
 
 UNIFIED_SOURCES += [