Bug 1232829 - Detach obsolete DocumentTimeline from refresh driver when the document is reset; r=smaug
authorBrian Birtles <birtles@gmail.com>
Tue, 22 Dec 2015 10:08:19 +0900
changeset 277216 84169b40100b4248f0cbdfbd0e42e27e358d5e80
parent 277215 3a1fddc20d91e8683c7c7d0b72d8911e22cad811
child 277217 0aa57628af27b14d6bba905d087498f2907302dc
push id69417
push userbbirtles@mozilla.com
push dateTue, 22 Dec 2015 01:08:40 +0000
treeherdermozilla-inbound@84169b40100b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1232829
milestone46.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 1232829 - Detach obsolete DocumentTimeline from refresh driver when the document is reset; r=smaug
dom/animation/DocumentTimeline.cpp
dom/base/nsDocument.cpp
layout/style/test/mochitest.ini
layout/style/test/test_bug1232829.html
--- a/dom/animation/DocumentTimeline.cpp
+++ b/dom/animation/DocumentTimeline.cpp
@@ -105,16 +105,18 @@ DocumentTimeline::NotifyAnimationUpdated
     }
   }
 }
 
 void
 DocumentTimeline::WillRefresh(mozilla::TimeStamp aTime)
 {
   MOZ_ASSERT(mIsObservingRefreshDriver);
+  MOZ_ASSERT(GetRefreshDriver(),
+             "Should be able to reach refresh driver from within WillRefresh");
 
   bool needsTicks = false;
   nsTArray<Animation*> animationsToRemove(mAnimations.Count());
 
   nsAutoAnimationMutationBatch mb(mDocument);
 
   for (Animation* animation = mAnimationOrder.getFirst(); animation;
        animation = animation->getNext()) {
@@ -138,20 +140,23 @@ DocumentTimeline::WillRefresh(mozilla::T
     }
   }
 
   for (Animation* animation : animationsToRemove) {
     RemoveAnimation(animation);
   }
 
   if (!needsTicks) {
-    // If another refresh driver observer destroys the nsPresContext,
-    // nsRefreshDriver will detect it and we won't be called.
+    // We already assert that GetRefreshDriver() is non-null at the beginning
+    // of this function but we check it again here to be sure that ticking
+    // animations does not have any side effects that cause us to lose the
+    // connection with the refresh driver, such as triggering the destruction
+    // of mDocument's PresShell.
     MOZ_ASSERT(GetRefreshDriver(),
-               "Refresh driver should still be valid inside WillRefresh");
+               "Refresh driver should still be valid at end of WillRefresh");
     GetRefreshDriver()->RemoveRefreshObserver(this, Flush_Style);
     mIsObservingRefreshDriver = false;
   }
 }
 
 void
 DocumentTimeline::NotifyRefreshDriverCreated(nsRefreshDriver* aDriver)
 {
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -2112,17 +2112,25 @@ nsDocument::Reset(nsIChannel* aChannel, 
     }
   }
 
   ResetToURI(uri, aLoadGroup, principal);
 
   // Note that, since mTiming does not change during a reset, the
   // navigationStart time remains unchanged and therefore any future new
   // timeline will have the same global clock time as the old one.
-  mDocumentTimeline = nullptr;
+  if (mDocumentTimeline) {
+    nsRefreshDriver* rd = mPresShell && mPresShell->GetPresContext() ?
+                          mPresShell->GetPresContext()->RefreshDriver() :
+                          nullptr;
+    if (rd) {
+      mDocumentTimeline->NotifyRefreshDriverDestroying(rd);
+    }
+    mDocumentTimeline = nullptr;
+  }
 
   nsCOMPtr<nsIPropertyBag2> bag = do_QueryInterface(aChannel);
   if (bag) {
     nsCOMPtr<nsIURI> baseURI;
     bag->GetPropertyAsInterface(NS_LITERAL_STRING("baseURI"),
                                 NS_GET_IID(nsIURI), getter_AddRefs(baseURI));
     if (baseURI) {
       mDocumentBaseURI = baseURI;
--- a/layout/style/test/mochitest.ini
+++ b/layout/style/test/mochitest.ini
@@ -126,16 +126,17 @@ support-files = file_bug829816.css
 [test_bug887741_at-rules_in_declaration_lists.html]
 [test_bug892929.html]
 [test_bug1055933.html]
 support-files = file_bug1055933_circle-xxl.png
 [test_bug1089417.html]
 support-files = file_bug1089417_iframe.html
 [test_bug1112014.html]
 [test_bug1203766.html]
+[test_bug1232829.html]
 [test_cascade.html]
 [test_ch_ex_no_infloops.html]
 [test_compute_data_with_start_struct.html]
 skip-if = toolkit == 'android'
 [test_computed_style.html]
 [test_computed_style_no_pseudo.html]
 [test_computed_style_prefs.html]
 [test_condition_text.html]
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_bug1232829.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1232829
+-->
+<head>
+<meta charset="utf-8">
+<title>Test for Bug 1232829</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+<script>
+
+/** Test for Bug 1232829 **/
+
+// This should be a crashtest but it relies on using a pop-up window which
+// isn't supported in crashtests.
+function boom() {
+  var popup = window.open("data:text/html,2");
+  setTimeout(function() {
+    var frameDoc = document.querySelector("iframe").contentDocument;
+    frameDoc.write("3");
+    frameDoc.defaultView.history.back();
+    requestAnimationFrame(function() {
+      popup.close();
+      ok(true, "Didn't crash");
+      SimpleTest.finish();
+    });
+  }, 0);
+}
+
+SimpleTest.waitForExplicitFinish();
+</script>
+</head>
+<body onload="boom()">
+  <iframe srcdoc="<style>@keyframes a { to { opacity: 0.5 } }</style>
+                  <div style='animation: a 1ms'></div>"></iframe>
+</body>
+</html>