Bug 1372790 - Change uint8_t to back some enum and reorder some fields in StyleSheet to make it more compact. r?heycam draft
authorXidorn Quan <me@upsuper.org>
Wed, 14 Jun 2017 14:26:48 +1000
changeset 604502 fd321ea243c0ac13775e58ff420ec2faa10c5ed3
parent 604051 2e4d181768957e1b7b687481bf038228785b1814
child 636213 f2946440c1c4fc3d95f8618db1e31744261926c2
push id67097
push userxquan@mozilla.com
push dateThu, 06 Jul 2017 01:38:23 +0000
reviewersheycam
bugs1372790
milestone56.0a1
Bug 1372790 - Change uint8_t to back some enum and reorder some fields in StyleSheet to make it more compact. r?heycam MozReview-Commit-ID: DddIzhJqmcZ
layout/style/SheetParsingMode.h
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
layout/style/nsCSSParser.cpp
--- a/layout/style/SheetParsingMode.h
+++ b/layout/style/SheetParsingMode.h
@@ -2,16 +2,18 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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 mozilla_css_SheetParsingMode_h
 #define mozilla_css_SheetParsingMode_h
 
+#include <stdint.h>
+
 namespace mozilla {
 namespace css {
 
 /**
  * Enum defining the mode in which a sheet is to be parsed.  This is
  * usually, but not always, the same as the cascade level at which the
  * sheet will apply (see nsStyleSet.h).  Most of the Loader APIs only
  * support loading of author sheets.
@@ -35,17 +37,17 @@ namespace css {
  * styling of anonymous box pseudo-elements can violate layout
  * invariants.
  *
  * Agent sheets that do not use any unsafe rules could use
  * eSafeAgentSheetFeatures when creating the sheet. This enum value allows
  * Servo backend to recognize the sheets as the agent level, but Gecko
  * backend will parse it under _author_ level.
  */
-enum SheetParsingMode {
+enum SheetParsingMode : uint8_t {
   eAuthorSheetFeatures = 0,
   eUserSheetFeatures,
   eAgentSheetFeatures,
   eSafeAgentSheetFeatures,
 };
 
 } // namespace css
 } // namespace mozilla
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -23,40 +23,40 @@ namespace mozilla {
 StyleSheet::StyleSheet(StyleBackendType aType, css::SheetParsingMode aParsingMode)
   : mParent(nullptr)
   , mDocument(nullptr)
   , mOwningNode(nullptr)
   , mOwnerRule(nullptr)
   , mParsingMode(aParsingMode)
   , mType(aType)
   , mDisabled(false)
+  , mDirty(false)
   , mDocumentAssociationMode(NotOwnedByDocument)
   , mInner(nullptr)
-  , mDirty(false)
 {
 }
 
 StyleSheet::StyleSheet(const StyleSheet& aCopy,
                        StyleSheet* aParentToUse,
                        dom::CSSImportRule* aOwnerRuleToUse,
                        nsIDocument* aDocumentToUse,
                        nsINode* aOwningNodeToUse)
   : mParent(aParentToUse)
   , mTitle(aCopy.mTitle)
   , mDocument(aDocumentToUse)
   , mOwningNode(aOwningNodeToUse)
   , mOwnerRule(aOwnerRuleToUse)
   , mParsingMode(aCopy.mParsingMode)
   , mType(aCopy.mType)
   , mDisabled(aCopy.mDisabled)
+  , mDirty(aCopy.mDirty)
   // We only use this constructor during cloning.  It's the cloner's
   // responsibility to notify us if we end up being owned by a document.
   , mDocumentAssociationMode(NotOwnedByDocument)
   , mInner(aCopy.mInner) // Shallow copy, but concrete subclasses will fix up.
-  , mDirty(aCopy.mDirty)
 {
   MOZ_ASSERT(mInner, "Should only copy StyleSheets with an mInner.");
   mInner->AddSheet(this);
 
   if (aCopy.mMedia) {
     // XXX This is wrong; we should be keeping @import rules and
     // sheets in sync!
     mMedia = aCopy.mMedia->Clone();
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -139,17 +139,17 @@ public:
   bool IsModified() const { return mDirty; }
 
   void EnsureUniqueInner();
 
   // Append all of this sheet's child sheets to aArray.
   void AppendAllChildSheets(nsTArray<StyleSheet*>& aArray);
 
   // style sheet owner info
-  enum DocumentAssociationMode {
+  enum DocumentAssociationMode : uint8_t {
     // OwnedByDocument means mDocument owns us (possibly via a chain of other
     // stylesheets).
     OwnedByDocument,
     // NotOwnedByDocument means we're owned by something that might have a
     // different lifetime than mDocument.
     NotOwnedByDocument
   };
   nsIDocument* GetAssociatedDocument() const { return mDocument; }
@@ -332,27 +332,27 @@ protected:
   // mParsingMode controls access to nonstandard style constructs that
   // are not safe for use on the public Web but necessary in UA sheets
   // and/or useful in user sheets.
   css::SheetParsingMode mParsingMode;
 
   const StyleBackendType mType;
   bool                  mDisabled;
 
+  bool mDirty; // has been modified
+
   // mDocumentAssociationMode determines whether mDocument directly owns us (in
   // the sense that if it's known-live then we're known-live).  Always
   // NotOwnedByDocument when mDocument is null.
   DocumentAssociationMode mDocumentAssociationMode;
 
   // Core information we get from parsed sheets, which are shared amongst
   // StyleSheet clones.
   StyleSheetInfo* mInner;
 
-  bool mDirty; // has been modified
-
   nsTArray<StyleSetHandle> mStyleSets;
 
   friend class ::nsCSSRuleProcessor;
 
   // Make CSSStyleSheet and ServoStyleSheet friends so they can access
   // protected members of other StyleSheet objects (useful for iterating
   // through children).
   friend class mozilla::CSSStyleSheet;
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -1341,23 +1341,16 @@ protected:
   bool          mNavQuirkMode : 1;
 
   // True when the hashless color quirk applies.
   bool mHashlessColorQuirk : 1;
 
   // True when the unitless length quirk applies.
   bool mUnitlessLengthQuirk : 1;
 
-  // Controls access to nonstandard style constructs that are not safe
-  // for use on the public Web but necessary in UA sheets and/or
-  // useful in user sheets.  The only values stored in this field are
-  // 0, 1, and 2; three bits are allocated to avoid issues should the
-  // enum type be signed.
-  css::SheetParsingMode mParsingMode : 3;
-
   // True if we are in parsing rules for the chrome.
   bool mIsChrome : 1;
 
   // True if we're parsing SVG presentation attributes
   // These attributes allow non-calc lengths to be unitless (mapping to px)
   bool mIsSVGMode : 1;
 
   // True if viewport units should be allowed.
@@ -1385,16 +1378,21 @@ protected:
   // been passed in the nsCSSScanner constructor.  This is usually the case.
   // It can be set to false, for example, when we create an nsCSSParser solely
   // to parse a property value to test it for syntactic correctness.  When
   // false, an assertion that mSheetPrincipal is non-null is skipped.  Should
   // not be set to false if any nsCSSValues created during parsing can escape
   // out of the parser.
   bool mSheetPrincipalRequired;
 
+  // Controls access to nonstandard style constructs that are not safe
+  // for use on the public Web but necessary in UA sheets and/or
+  // useful in user sheets.
+  css::SheetParsingMode mParsingMode;
+
   // This enum helps us track whether we've unprefixed "display: -webkit-box"
   // (treating it as "display: flex") in an earlier declaration within a series
   // of declarations.  (This only impacts behavior if
   // sWebkitPrefixedAliasesEnabled is true.)
   enum WebkitBoxUnprefixState : uint8_t {
     eNotParsingDecls, // We are *not* currently parsing a sequence of
                       // CSS declarations. (default state)
 
@@ -1475,25 +1473,25 @@ CSSParserImpl::CSSParserImpl()
     mChildLoader(nullptr),
     mReusableSheets(nullptr),
     mSection(eCSSSection_Charset),
     mNameSpaceMap(nullptr),
     mHavePushBack(false),
     mNavQuirkMode(false),
     mHashlessColorQuirk(false),
     mUnitlessLengthQuirk(false),
-    mParsingMode(css::eAuthorSheetFeatures),
     mIsChrome(false),
     mIsSVGMode(false),
     mViewportUnitsEnabled(true),
     mParsingCompoundProperty(false),
     mInSupportsCondition(false),
     mInFailingSupportsRule(false),
     mSuppressErrors(false),
     mSheetPrincipalRequired(true),
+    mParsingMode(css::eAuthorSheetFeatures),
     mWebkitBoxUnprefixState(eNotParsingDecls),
     mNextFree(nullptr)
 {
 }
 
 CSSParserImpl::~CSSParserImpl()
 {
   mData.AssertInitialState();