Bug 1415645 - Part 3: Don't cast to nsGlobalWindow when working with the inner window linked list, r=smaug
authorNika Layzell <nika@thelayzells.com>
Wed, 08 Nov 2017 11:19:59 -0500
changeset 392017 9a766cc68671449c2c624816dff943d597a992a6
parent 392016 4a6c2c5490f454e53d3b962ea0ce9dd29c9da298
child 392018 55c0344711b64723b7653404ed9c4cd28ca9d6d9
push id32909
push usercbrindusan@mozilla.com
push dateWed, 15 Nov 2017 22:25:14 +0000
treeherdermozilla-central@f41930a869a8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1415645
milestone59.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 1415645 - Part 3: Don't cast to nsGlobalWindow when working with the inner window linked list, r=smaug After the window split is complete, the inner window linked list won't be homogenously typed anymore, as there will be an nsGlobalWindowOuter member in addition to the nsGlobalWindowInner members. This patch changes the code to perform PRCList* pointer comparisons before casting to nsGlobalWindowInner to avoid this issue. MozReview-Commit-ID: 56q5XodtGe7
dom/base/nsGlobalWindow.cpp
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -1767,19 +1767,23 @@ nsGlobalWindow::~nsGlobalWindow()
     if (proxy) {
       js::SetProxyReservedSlot(proxy, 0, js::PrivateValue(nullptr));
     }
 
     // An outer window is destroyed with inner windows still possibly
     // alive, iterate through the inner windows and null out their
     // back pointer to this outer, and pull them out of the list of
     // inner windows.
-
-    nsGlobalWindow *w;
-    while ((w = (nsGlobalWindow *)PR_LIST_HEAD(this)) != this) {
+    //
+    // Our linked list of inner windows both contains (an nsGlobalWindowOuter),
+    // and our inner windows (nsGlobalWindowInners). This means that we need to
+    // use PRCList*. We can then compare that PRCList* to `this` to see if its an
+    // inner or outer window.
+    PRCList* w;
+    while ((w = PR_LIST_HEAD(this)) != this) {
       PR_REMOVE_AND_INIT_LINK(w);
     }
 
     DropOuterWindowDocs();
   } else {
     Telemetry::Accumulate(Telemetry::INNERWINDOWS_WITH_MUTATION_LISTENERS,
                           mMutationBits ? 1 : 0);
 
@@ -3537,19 +3541,23 @@ nsGlobalWindow::DetachFromDocShell()
   // DetachFromDocShell means the window is being torn down. Drop our
   // reference to the script context, allowing it to be deleted
   // later. Meanwhile, keep our weak reference to the script object
   // so that it can be retrieved later (until it is finalized by the JS GC).
 
   // Call FreeInnerObjects on all inner windows, not just the current
   // one, since some could be held by WindowStateHolder objects that
   // are GC-owned.
-  for (RefPtr<nsGlobalWindow> inner = (nsGlobalWindow *)PR_LIST_HEAD(this);
-       inner != this;
-       inner = (nsGlobalWindow*)PR_NEXT_LINK(inner)) {
+  RefPtr<nsGlobalWindowInner> inner;
+  for (PRCList* node = PR_LIST_HEAD(this);
+       node != this;
+       node = PR_NEXT_LINK(inner)) {
+    // This cast is safe because `node != this`. Non-this nodes are inner windows.
+    inner = static_cast<nsGlobalWindowInner*>(node);
+    MOZ_ASSERT(inner->IsInnerWindow());
     MOZ_ASSERT(!inner->mOuterWindow || inner->mOuterWindow == AsOuter());
     inner->FreeInnerObjects();
   }
 
   // Don't report that we were detached to the nsWindowMemoryReporter, as it
   // only tracks inner windows.
 
   NotifyWindowIDDestroyed("outer-window-destroyed");
@@ -10756,19 +10764,23 @@ nsGlobalWindow::ResetVRTelemetry(bool aU
 
 void
 nsGlobalWindow::SetChromeEventHandler(EventTarget* aChromeEventHandler)
 {
   MOZ_ASSERT(IsOuterWindow());
 
   SetChromeEventHandlerInternal(aChromeEventHandler);
   // update the chrome event handler on all our inner windows
-  for (nsGlobalWindow *inner = (nsGlobalWindow *)PR_LIST_HEAD(this);
-       inner != this;
-       inner = (nsGlobalWindow*)PR_NEXT_LINK(inner)) {
+  RefPtr<nsGlobalWindowInner> inner;
+  for (PRCList* node = PR_LIST_HEAD(this);
+       node != this;
+       node = PR_NEXT_LINK(inner)) {
+    // This cast is only safe if `node != this`, as nsGlobalWindowOuter is also
+    // in the list.
+    inner = static_cast<nsGlobalWindowInner*>(node);
     NS_ASSERTION(!inner->mOuterWindow || inner->mOuterWindow == AsOuter(),
                  "bad outer window pointer");
     inner->SetChromeEventHandlerInternal(aChromeEventHandler);
   }
 }
 
 static bool IsLink(nsIContent* aContent)
 {