Bug 1637487 - Store mixed content security flags as a flags word on Document instead of bools. r=ckerschb
☠☠ backed out by efbb5dd6bf7b ☠ ☠
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 13 May 2020 17:50:34 +0000
changeset 529727 5633cc2aaa9b69e27f8c89d742c03ccc453b2366
parent 529726 c4f0c757cae0c3c307f4e6709064ae2b0a29264c
child 529728 3d8f0e488dbfdc39d0481939dabc80e300d93619
push id37414
push usernbeleuzu@mozilla.com
push dateThu, 14 May 2020 02:40:10 +0000
treeherdermozilla-central@045d696faa87 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckerschb
bugs1637487
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1637487 - Store mixed content security flags as a flags word on Document instead of bools. r=ckerschb Depends on D75025 Differential Revision: https://phabricator.services.mozilla.com/D75026
dom/base/Document.cpp
dom/base/Document.h
dom/security/nsMixedContentBlocker.cpp
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -1246,20 +1246,16 @@ Document::Document(const char* aContentT
       mDocURISchemeIsChrome(false),
       mInChromeDocShell(false),
       mIsDevToolsDocument(false),
       mIsSyntheticDocument(false),
       mHasLinksToUpdateRunnable(false),
       mFlushingPendingLinkUpdates(false),
       mMayHaveDOMMutationObservers(false),
       mMayHaveAnimationObservers(false),
-      mHasMixedActiveContentLoaded(false),
-      mHasMixedActiveContentBlocked(false),
-      mHasMixedDisplayContentLoaded(false),
-      mHasMixedDisplayContentBlocked(false),
       mHasCSP(false),
       mHasUnsafeEvalCSP(false),
       mHasUnsafeInlineCSP(false),
       mHasCSPDeliveredThroughHeader(false),
       mBFCacheDisallowed(false),
       mHasHadDefaultView(false),
       mStyleSheetChangeEventsEnabled(false),
       mIsSrcdocDocument(false),
--- a/dom/base/Document.h
+++ b/dom/base/Document.h
@@ -1043,65 +1043,101 @@ class Document : public nsINode,
    * change to actually change anything immediately.
    * @see nsBidiUtils.h
    */
   void SetBidiOptions(uint32_t aBidiOptions) { mBidiOptions = aBidiOptions; }
 
   /**
    * Get the has mixed active content loaded flag for this document.
    */
-  bool GetHasMixedActiveContentLoaded() { return mHasMixedActiveContentLoaded; }
+  bool GetHasMixedActiveContentLoaded() {
+    return mMixedContentFlags &
+           nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT;
+  }
 
   /**
    * Set the has mixed active content loaded flag for this document.
    */
   void SetHasMixedActiveContentLoaded(bool aHasMixedActiveContentLoaded) {
-    mHasMixedActiveContentLoaded = aHasMixedActiveContentLoaded;
+    if (aHasMixedActiveContentLoaded) {
+      mMixedContentFlags |=
+          nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT;
+    } else {
+      mMixedContentFlags &=
+          ~nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT;
+    }
   }
 
   /**
    * Get mixed active content blocked flag for this document.
    */
   bool GetHasMixedActiveContentBlocked() {
-    return mHasMixedActiveContentBlocked;
+    return mMixedContentFlags &
+           nsIWebProgressListener::STATE_BLOCKED_MIXED_ACTIVE_CONTENT;
   }
 
   /**
    * Set the mixed active content blocked flag for this document.
    */
   void SetHasMixedActiveContentBlocked(bool aHasMixedActiveContentBlocked) {
-    mHasMixedActiveContentBlocked = aHasMixedActiveContentBlocked;
+    if (aHasMixedActiveContentBlocked) {
+      mMixedContentFlags |=
+          nsIWebProgressListener::STATE_BLOCKED_MIXED_ACTIVE_CONTENT;
+    } else {
+      mMixedContentFlags &=
+          ~nsIWebProgressListener::STATE_BLOCKED_MIXED_ACTIVE_CONTENT;
+    }
   }
 
   /**
    * Get the has mixed display content loaded flag for this document.
    */
   bool GetHasMixedDisplayContentLoaded() {
-    return mHasMixedDisplayContentLoaded;
+    return mMixedContentFlags &
+           nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT;
   }
 
   /**
    * Set the has mixed display content loaded flag for this document.
    */
   void SetHasMixedDisplayContentLoaded(bool aHasMixedDisplayContentLoaded) {
-    mHasMixedDisplayContentLoaded = aHasMixedDisplayContentLoaded;
+    if (aHasMixedDisplayContentLoaded) {
+      mMixedContentFlags |=
+          nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT;
+    } else {
+      mMixedContentFlags &=
+          ~nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT;
+    }
   }
 
   /**
    * Get mixed display content blocked flag for this document.
    */
   bool GetHasMixedDisplayContentBlocked() {
-    return mHasMixedDisplayContentBlocked;
+    return mMixedContentFlags &
+           nsIWebProgressListener::STATE_BLOCKED_MIXED_DISPLAY_CONTENT;
   }
 
   /**
    * Set the mixed display content blocked flag for this document.
    */
   void SetHasMixedDisplayContentBlocked(bool aHasMixedDisplayContentBlocked) {
-    mHasMixedDisplayContentBlocked = aHasMixedDisplayContentBlocked;
+    if (aHasMixedDisplayContentBlocked) {
+      mMixedContentFlags |=
+          nsIWebProgressListener::STATE_BLOCKED_MIXED_DISPLAY_CONTENT;
+    } else {
+      mMixedContentFlags &=
+          ~nsIWebProgressListener::STATE_BLOCKED_MIXED_DISPLAY_CONTENT;
+    }
+  }
+
+  uint32_t GetMixedContentFlags() const { return mMixedContentFlags; }
+
+  void AddMixedContentFlags(uint32_t aMixedContentFlags) {
+    mMixedContentFlags |= aMixedContentFlags;
   }
 
   /**
    * Set CSP flag for this document.
    */
   void SetHasCSP(bool aHasCSP) { mHasCSP = aHasCSP; }
 
   /**
@@ -4339,16 +4375,18 @@ class Document : public nsINode,
   RefPtr<mozilla::dom::FeaturePolicy> mFeaturePolicy;
 
   UniquePtr<ResizeObserverController> mResizeObserverController;
 
   // Permission Delegate Handler, lazily-initialized in
   // GetPermissionDelegateHandler
   RefPtr<PermissionDelegateHandler> mPermissionDelegateHandler;
 
+  uint32_t mMixedContentFlags = 0;
+
   // True if BIDI is enabled.
   bool mBidiEnabled : 1;
   // True if we may need to recompute the language prefs for this document.
   bool mMayNeedFontPrefsUpdate : 1;
   // True if a MathML element has ever been owned by this document.
   bool mMathMLEnabled : 1;
 
   // True if this document is the initial document for a window.  This should
@@ -4432,32 +4470,16 @@ class Document : public nsINode,
   // True if a DOMMutationObserver is perhaps attached to a node in the
   // document.
   bool mMayHaveDOMMutationObservers : 1;
 
   // True if an nsIAnimationObserver is perhaps attached to a node in the
   // document.
   bool mMayHaveAnimationObservers : 1;
 
-  // True if a document has loaded Mixed Active Script (see
-  // nsMixedContentBlocker.cpp)
-  bool mHasMixedActiveContentLoaded : 1;
-
-  // True if a document has blocked Mixed Active Script (see
-  // nsMixedContentBlocker.cpp)
-  bool mHasMixedActiveContentBlocked : 1;
-
-  // True if a document has loaded Mixed Display/Passive Content (see
-  // nsMixedContentBlocker.cpp)
-  bool mHasMixedDisplayContentLoaded : 1;
-
-  // True if a document has blocked Mixed Display/Passive Content (see
-  // nsMixedContentBlocker.cpp)
-  bool mHasMixedDisplayContentBlocked : 1;
-
   // True if a document load has a CSP attached.
   bool mHasCSP : 1;
 
   // True if a document load has a CSP with unsafe-eval attached.
   bool mHasUnsafeEvalCSP : 1;
 
   // True if a document load has a CSP with unsafe-inline attached.
   bool mHasUnsafeInlineCSP : 1;
--- a/dom/security/nsMixedContentBlocker.cpp
+++ b/dom/security/nsMixedContentBlocker.cpp
@@ -839,49 +839,22 @@ nsresult nsMixedContentBlocker::ShouldLo
   }
 
   LogMixedContentMessage(classification, aContentLocation, topWC->Id(),
                          (*aDecision == nsIContentPolicy::REJECT_REQUEST)
                              ? eBlocked
                              : eUserOverride,
                          requestingLocation);
 
-  // Check if the new flags we computed match the current state on the doc.
-  // This is really painful, and life would be eaiser if the doc had the same
-  // flags instead of bools.
-  bool stateChanged =
-      ((newState & nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT) ==
-       rootDoc->GetHasMixedActiveContentLoaded()) ||
-      ((newState &
-        nsIWebProgressListener::STATE_BLOCKED_MIXED_ACTIVE_CONTENT) ==
-       rootDoc->GetHasMixedActiveContentBlocked()) ||
-      ((newState &
-        nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT) ==
-       rootDoc->GetHasMixedDisplayContentLoaded()) ||
-      ((newState &
-        nsIWebProgressListener::STATE_BLOCKED_MIXED_DISPLAY_CONTENT) ==
-       rootDoc->GetHasMixedDisplayContentBlocked());
-
-  if (!stateChanged) {
+  if (rootDoc->GetMixedContentFlags() == newState) {
     return NS_OK;
   }
 
   // Copy the new state onto the Document flags.
-  if (newState & nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT) {
-    rootDoc->SetHasMixedActiveContentLoaded(true);
-  }
-  if (newState & nsIWebProgressListener::STATE_BLOCKED_MIXED_ACTIVE_CONTENT) {
-    rootDoc->SetHasMixedActiveContentBlocked(true);
-  }
-  if (newState & nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT) {
-    rootDoc->SetHasMixedDisplayContentLoaded(true);
-  }
-  if (newState & nsIWebProgressListener::STATE_BLOCKED_MIXED_DISPLAY_CONTENT) {
-    rootDoc->SetHasMixedDisplayContentBlocked(true);
-  }
+  rootDoc->AddMixedContentFlags(newState);
 
   uint32_t state = nsIWebProgressListener::STATE_IS_BROKEN;
   MOZ_ALWAYS_SUCCEEDS(securityUI->GetState(&state));
 
   if (*aDecision == nsIContentPolicy::ACCEPT && rootHasSecureConnection) {
     // reset state security flag
     state = state >> 4 << 4;
     // set state security flag to broken, since there is mixed content