Commit c5184815 authored by Andrew McCreight's avatar Andrew McCreight
Browse files

Bug 1535403 - Take indirection into account for the CC optimizations for the...

Bug 1535403 - Take indirection into account for the CC optimizations for the outer window wrapper. r=peterv

Most wrapper cached C++ objects are held alive by their wrapper. The
cycle collector takes advantage of this in many classes and ignores
the C++ object if the wrapper is marked black.

However, this is not true for the outer window's wrapper. Instead, the
outer window's wrapper keeps the inner window alive. The inner window
usually keeps its outer window alive, but not after it has been
unlinked. For reasons I do not yet understand, the outer window's
wrapper can be kept alive after the inner window it is a proxy for is
unlinked.

This patch fixes the cycle collector optimization for the outer window
by only applying it if the outer window still has a weak reference to
the inner window, which it will until the inner no longer holds the
outer alive. This in turn fixes, or at least helps fix, window leaks
seen intermittently when the lifetime of outer windows and docshells
are tied together.

The code comment is based on a review comment by peterv.

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

--HG--
extra : moz-landing-system : lando
parent f5e6644f
Loading
Loading
Loading
Loading
+6 −1
Original line number Diff line number Diff line
@@ -1464,8 +1464,13 @@ bool nsGlobalWindowOuter::IsBlackForCC(bool aTracingNeeded) {
    return false;
  }

  // Unlike most wrappers, the outer window wrapper is not a wrapper for
  // the outer window. Instead, the outer window wrapper holds the inner
  // window binding object, which in turn holds the nsGlobalWindowInner, which
  // has a strong reference to the nsGlobalWindowOuter. We're using the
  // mInnerWindow pointer as a flag for that whole chain.
  return (nsCCUncollectableMarker::InGeneration(GetMarkedCCGeneration()) ||
          HasKnownLiveWrapper()) &&
          (mInnerWindow && HasKnownLiveWrapper())) &&
         (!aTracingNeeded || HasNothingToTrace(ToSupports(this)));
}