Bug 1232829 - Detach obsolete DocumentTimeline from refresh driver when the document is reset; r=smaug a=sylvestre
authorBrian Birtles <birtles@gmail.com>
Tue, 22 Dec 2015 10:08:19 +0900
changeset 310616 624eb61f2c315472061af83ef042ec1b18041532
parent 310615 80c32daf68127fd396979eb193d0b23b2f16c5a8
child 310617 c4c170ddf6e2517bbef38a057e7072ceaa2baa83
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, sylvestre
bugs1232829
milestone45.0a2
Bug 1232829 - Detach obsolete DocumentTimeline from refresh driver when the document is reset; r=smaug a=sylvestre
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
@@ -2109,17 +2109,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>