Bug 1525371 - Fix a very subtle invalidation bug exposed by test_initial_computation.html. r=jwatt
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 06 Feb 2019 18:12:17 +0100
changeset 457595 d9be4af74c4b90d08a03fbce96bdabf7dca9faf8
parent 457594 56eeff69792c1809cd2dd51a77ea276cde635cdc
child 457596 c90fe3ee1999efedec2bddca08db72b8975fc561
push id35516
push userrmaries@mozilla.com
push dateFri, 08 Feb 2019 04:23:26 +0000
treeherdermozilla-central@d599d1a73a3a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1525371
milestone67.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 1525371 - Fix a very subtle invalidation bug exposed by test_initial_computation.html. r=jwatt Now that there's no ArenaRefPtr, the styles aren't cleared when the shell goes away (due to an iframe becoming display: none or what not). This caused a few very confusing failures in test_initial_computation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2746a05ce7b20759a820d6f26a55a3200d8e6b8&selectedJob=226267113 Which holds on to a reference of a style on a display: none iframe[1], reframes it periodically[2], exposing this bug. For now, keep the id of the shell we got the style from around. When we support computing styles in display: none iframes this may need more work, but that's out of the scope of this bug. [1]: https://searchfox.org/mozilla-central/rev/e00ea598e52bbb35f8c45abf9c2eade17962bb5e/layout/style/test/test_initial_computation.html#56 [2]: https://searchfox.org/mozilla-central/rev/e00ea598e52bbb35f8c45abf9c2eade17962bb5e/layout/style/test/test_initial_computation.html#147 Differential Revision: https://phabricator.services.mozilla.com/D18853
layout/style/nsComputedDOMStyle.cpp
layout/style/nsComputedDOMStyle.h
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -285,17 +285,16 @@ nsComputedDOMStyle::nsComputedDOMStyle(d
                                        const nsAString& aPseudoElt,
                                        Document* aDocument,
                                        StyleType aStyleType)
     : mDocumentWeak(nullptr),
       mOuterFrame(nullptr),
       mInnerFrame(nullptr),
       mPresShell(nullptr),
       mStyleType(aStyleType),
-      mComputedStyleGeneration(0),
       mExposeVisitedStyle(false),
       mResolvedComputedStyle(false)
 #ifdef DEBUG
       ,
       mFlushedPendingReflows(false)
 #endif
 {
   MOZ_ASSERT(aElement);
@@ -741,23 +740,25 @@ void nsComputedDOMStyle::ClearComputedSt
 void nsComputedDOMStyle::SetResolvedComputedStyle(
     RefPtr<ComputedStyle>&& aContext, uint64_t aGeneration) {
   if (!mResolvedComputedStyle) {
     mResolvedComputedStyle = true;
     mElement->AddMutationObserver(this);
   }
   mComputedStyle = aContext;
   mComputedStyleGeneration = aGeneration;
+  mPresShellId = mPresShell->GetPresShellId();
 }
 
 void nsComputedDOMStyle::SetFrameComputedStyle(mozilla::ComputedStyle* aStyle,
                                                uint64_t aGeneration) {
   ClearComputedStyle();
   mComputedStyle = aStyle;
   mComputedStyleGeneration = aGeneration;
+  mPresShellId = mPresShell->GetPresShellId();
 }
 
 bool nsComputedDOMStyle::NeedsToFlush(Document* aDocument) const {
   // If mElement is not in the same document, we could do some checks to know if
   // there are some pending restyles can be ignored across documents (since we
   // will use the caller document's style), but it can be complicated and should
   // be an edge case, so we just don't bother to do the optimization in this
   // case.
@@ -840,16 +841,17 @@ void nsComputedDOMStyle::UpdateCurrentSt
 
   if (mComputedStyle) {
     // We can't rely on the undisplayed restyle generation if mElement is
     // out-of-document, since that generation is not incremented for DOM changes
     // on out-of-document elements.
     //
     // So we always need to update the style to ensure it it up-to-date.
     if (mComputedStyleGeneration == currentGeneration &&
+        mPresShellId == mPresShell->GetPresShellId() &&
         mElement->IsInComposedDoc()) {
       // Our cached style is still valid.
       return;
     }
     // We've processed some restyles, so the cached style might be out of date.
     mComputedStyle = nullptr;
   }
 
--- a/layout/style/nsComputedDOMStyle.h
+++ b/layout/style/nsComputedDOMStyle.h
@@ -523,19 +523,22 @@ class nsComputedDOMStyle final : public 
 
   /*
    * The kind of styles we should be returning.
    */
   StyleType mStyleType;
 
   /**
    * The nsComputedDOMStyle generation at the time we last resolved a style
-   * context and stored it in mComputedStyle.
+   * context and stored it in mComputedStyle, and the pres shell we got the
+   * style from. Should only be used together.
    */
-  uint64_t mComputedStyleGeneration;
+  uint64_t mComputedStyleGeneration = 0;
+
+  uint32_t mPresShellId = 0;
 
   bool mExposeVisitedStyle;
 
   /**
    * Whether we resolved a ComputedStyle last time we called
    * UpdateCurrentStyleSources.  Initially false.
    */
   bool mResolvedComputedStyle;