Bug 675121. Unregister animation frame callbacks from the refresh driver while we have event handling suppressed. r=smaug,roc
authorBoris Zbarsky <bzbarsky@mit.edu>
Sun, 07 Aug 2011 22:24:28 -0400
changeset 73968 9954f0235bc8
parent 73967 3b5da2d84538
child 73969 e2f2476215cd
push id1039
push userbzbarsky@mozilla.com
push date2011-08-08 02:36 +0000
treeherdermozilla-inbound@840e9e5c5059 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, roc
bugs675121
milestone8.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 675121. Unregister animation frame callbacks from the refresh driver while we have event handling suppressed. r=smaug,roc
content/base/public/nsIDocument.h
content/base/src/nsDocument.cpp
content/base/src/nsDocument.h
content/base/test/Makefile.in
content/base/test/file_bug675121.sjs
content/base/test/test_bug675121.html
--- a/content/base/public/nsIDocument.h
+++ b/content/base/public/nsIDocument.h
@@ -1326,16 +1326,20 @@ public:
    * Unsuppress event handling.
    * @param aFireEvents If PR_TRUE, delayed events (focus/blur) will be fired
    *                    asynchronously.
    */
   virtual void UnsuppressEventHandlingAndFireEvents(PRBool aFireEvents) = 0;
 
   PRUint32 EventHandlingSuppressed() const { return mEventsSuppressed; }
 
+  bool IsEventHandlingEnabled() {
+    return !EventHandlingSuppressed() && mScriptGlobalObject;
+  }
+
   /**
    * Increment the number of external scripts being evaluated.
    */
   void BeginEvaluatingExternalScript() { ++mExternalScriptsBeingEvaluated; }
 
   /**
    * Decrement the number of external scripts being evaluated.
    */
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -3191,28 +3191,31 @@ nsDocument::doCreateShell(nsPresContext*
   rv = shell->Init(this, aContext, aViewManager, aStyleSet, aCompatMode);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Note: we don't hold a ref to the shell (it holds a ref to us)
   mPresShell = shell;
 
   mExternalResourceMap.ShowViewers();
 
-  if (mScriptGlobalObject) {
-    RescheduleAnimationFrameNotifications();
-  }
+  MaybeRescheduleAnimationFrameNotifications();
 
   shell.swap(*aInstancePtrResult);
 
   return NS_OK;
 }
 
 void
-nsDocument::RescheduleAnimationFrameNotifications()
-{
+nsDocument::MaybeRescheduleAnimationFrameNotifications()
+{
+  if (!mPresShell || !IsEventHandlingEnabled()) {
+    // bail out for now, until one of those conditions changes
+    return;
+  }
+
   nsRefreshDriver* rd = mPresShell->GetPresContext()->RefreshDriver();
   if (mHavePendingPaint) {
     rd->ScheduleBeforePaintEvent(this);
   }
   if (!mAnimationFrameListeners.IsEmpty()) {
     rd->ScheduleAnimationFrameListeners(this);
   }
 }
@@ -3223,17 +3226,17 @@ nsIDocument::TakeAnimationFrameListeners
   aListeners.AppendElements(mAnimationFrameListeners);
   mAnimationFrameListeners.Clear();
 }
 
 void
 nsDocument::DeleteShell()
 {
   mExternalResourceMap.HideViewers();
-  if (mScriptGlobalObject) {
+  if (IsEventHandlingEnabled()) {
     RevokeAnimationFrameNotifications();
   }
   mPresShell = nsnull;
 }
 
 void
 nsDocument::RevokeAnimationFrameNotifications()
 {
@@ -3771,17 +3774,17 @@ nsDocument::SetScriptGlobalObject(nsIScr
                     "Clearing window pointer while animations are unpaused");
 #endif // MOZ_SMIL
 
   if (mScriptGlobalObject && !aScriptGlobalObject) {
     // We're detaching from the window.  We need to grab a pointer to
     // our layout history state now.
     mLayoutHistoryState = GetLayoutHistoryState();
 
-    if (mPresShell) {
+    if (mPresShell && !EventHandlingSuppressed()) {
       RevokeAnimationFrameNotifications();
     }
 
     // Also make sure to remove our onload blocker now if we haven't done it yet
     if (mOnloadBlockCount != 0) {
       nsCOMPtr<nsILoadGroup> loadGroup = GetDocumentLoadGroup();
       if (loadGroup) {
         loadGroup->RemoveRequest(mOnloadBlocker, nsnull, NS_OK);
@@ -3832,19 +3835,17 @@ nsDocument::SetScriptGlobalObject(nsIScr
                      "Unexpected container or script global?");
 #endif
         PRBool allowDNSPrefetch;
         docShell->GetAllowDNSPrefetch(&allowDNSPrefetch);
         mAllowDNSPrefetch = allowDNSPrefetch;
       }
     }
 
-    if (mPresShell) {
-      RescheduleAnimationFrameNotifications();
-    }
+    MaybeRescheduleAnimationFrameNotifications();
   }
 
   // Remember the pointer to our window (or lack there of), to avoid
   // having to QI every time it's asked for.
   nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(mScriptGlobalObject);
   mWindow = window;
 }
 
@@ -7643,16 +7644,20 @@ SuppressEventHandlingInDocument(nsIDocum
 {
   aDocument->SuppressEventHandling(*static_cast<PRUint32*>(aData));
   return PR_TRUE;
 }
 
 void
 nsDocument::SuppressEventHandling(PRUint32 aIncrease)
 {
+  if (mEventsSuppressed == 0 && aIncrease != 0 && mPresShell &&
+      mScriptGlobalObject) {
+    RevokeAnimationFrameNotifications();
+  }
   mEventsSuppressed += aIncrease;
   EnumerateSubDocuments(SuppressEventHandlingInDocument, &aIncrease);
 }
 
 static void
 FireOrClearDelayedEvents(nsTArray<nsCOMPtr<nsIDocument> >& aDocuments,
                          PRBool aFireEvents)
 {
@@ -7802,23 +7807,18 @@ GetAndUnsuppressSubDocuments(nsIDocument
   docs->AppendElement(aDocument);
   aDocument->EnumerateSubDocuments(GetAndUnsuppressSubDocuments, docs);
   return PR_TRUE;
 }
 
 void
 nsDocument::UnsuppressEventHandlingAndFireEvents(PRBool aFireEvents)
 {
-  if (mEventsSuppressed > 0) {
-    --mEventsSuppressed;
-  }
-
   nsTArray<nsCOMPtr<nsIDocument> > documents;
-  documents.AppendElement(this);
-  EnumerateSubDocuments(GetAndUnsuppressSubDocuments, &documents);
+  GetAndUnsuppressSubDocuments(this, &documents);
 
   if (aFireEvents) {
     NS_DispatchToCurrentThread(new nsDelayedEventDispatcher(documents));
   } else {
     FireOrClearDelayedEvents(documents, PR_FALSE);
   }
 }
 
@@ -8037,30 +8037,31 @@ nsIDocument::CreateStaticClone(nsISuppor
 }
 
 void
 nsIDocument::ScheduleBeforePaintEvent(nsIAnimationFrameListener* aListener)
 {
   if (aListener) {
     PRBool alreadyRegistered = !mAnimationFrameListeners.IsEmpty();
     if (mAnimationFrameListeners.AppendElement(aListener) &&
-        !alreadyRegistered && mPresShell) {
+        !alreadyRegistered && mPresShell && IsEventHandlingEnabled()) {
       mPresShell->GetPresContext()->RefreshDriver()->
         ScheduleAnimationFrameListeners(this);
     }
 
     return;
   }
 
   if (!mHavePendingPaint) {
     // We don't want to use GetShell() here, because we want to schedule the
     // paint even if we're frozen.  Either we'll get unfrozen and then the
     // event will fire, or we'll quietly go away at some point.
     mHavePendingPaint =
       !mPresShell ||
+      !IsEventHandlingEnabled() ||
       mPresShell->GetPresContext()->RefreshDriver()->
         ScheduleBeforePaintEvent(this);
   }
 
 }
 
 nsresult
 nsDocument::GetStateObject(nsIVariant** aState)
--- a/content/base/src/nsDocument.h
+++ b/content/base/src/nsDocument.h
@@ -860,17 +860,20 @@ public:
 #endif // MOZ_SMIL
 
   void SetImagesNeedAnimating(PRBool aAnimating);
 
   virtual void SuppressEventHandling(PRUint32 aIncrease);
 
   virtual void UnsuppressEventHandlingAndFireEvents(PRBool aFireEvents);
   
-  void DecreaseEventSuppression() { --mEventsSuppressed; }
+  void DecreaseEventSuppression() {
+    --mEventsSuppressed;
+    MaybeRescheduleAnimationFrameNotifications();
+  }
 
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(nsDocument,
                                                          nsIDocument)
 
   void DoNotifyPossibleTitleChange();
 
   nsExternalResourceMap& ExternalResourceMap()
   {
@@ -1144,18 +1147,19 @@ private:
   // Just like EnableStyleSheetsForSet, but doesn't check whether
   // aSheetSet is null and allows the caller to control whether to set
   // aSheetSet as the preferred set in the CSSLoader.
   void EnableStyleSheetsForSetInternal(const nsAString& aSheetSet,
                                        PRBool aUpdateCSSLoader);
 
   // Revoke any pending notifications due to mozRequestAnimationFrame calls
   void RevokeAnimationFrameNotifications();
-  // Reschedule any notifications we need to handle mozRequestAnimationFrame
-  void RescheduleAnimationFrameNotifications();
+  // Reschedule any notifications we need to handle
+  // mozRequestAnimationFrame, if it's OK to do so.
+  void MaybeRescheduleAnimationFrameNotifications();
 
   // These are not implemented and not supported.
   nsDocument(const nsDocument& aOther);
   nsDocument& operator=(const nsDocument& aOther);
 
   nsCOMPtr<nsISupports> mXPathEvaluatorTearoff;
 
   // The layout history state that should be used by nodes in this
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -496,16 +496,18 @@ include $(topsrcdir)/config/rules.mk
 		accesscontrol.resource^headers^ \
 		invalid_accesscontrol.resource \
 		invalid_accesscontrol.resource^headers^ \
 		somedatas.resource \
 		somedatas.resource^headers^ \
 		delayedServerEvents.sjs \
 		test_bug664916.html \
 		test_bug666604.html \
+		test_bug675121.html \
+		file_bug675121.sjs \
 		$(NULL)
 
 _CHROME_FILES =	\
 		test_bug357450.js \
 		$(NULL)
 
 # This test fails on the Mac for some reason
 ifneq (,$(filter gtk2 windows,$(MOZ_WIDGET_TOOLKIT)))
new file mode 100644
--- /dev/null
+++ b/content/base/test/file_bug675121.sjs
@@ -0,0 +1,15 @@
+var timer;
+
+function handleRequest(request, response)
+{
+  response.setHeader("Cache-Control", "no-cache", false);
+  response.setHeader("Content-Type", "text/plain", false);
+  response.write("Responded");
+  response.processAsync();
+  timer = Components.classes["@mozilla.org/timer;1"]
+    .createInstance(Components.interfaces.nsITimer);
+  timer.initWithCallback(function() {
+      response.finish();
+    // 50ms certainly be enough for one refresh driver firing to happen!
+    }, 50, Components.interfaces.nsITimer.TYPE_ONE_SHOT);
+}
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug675121.html
@@ -0,0 +1,46 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=675121
+-->
+<head>
+  <title>Test for Bug 675121</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=675121">Mozilla Bug 675121</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 675121 **/
+var callbackFired = false;
+var xhrInProgress = false;
+function f() {
+  callbackFired = true;
+  if (!xhrInProgress) {
+    SimpleTest.finish();
+  }
+}
+
+window.mozRequestAnimationFrame(f);
+var xhr = new XMLHttpRequest();
+xhr.open("GET", "file_bug675121.sjs", false);
+xhrInProgress = true;
+xhr.send();
+xhrInProgress = false;
+is(xhr.responseText, "Responded", "Should have a response by now");
+is(callbackFired, false, "Callback should not fire during sync XHR");
+
+if (!callbackFired) {
+  SimpleTest.waitForExplicitFinish();
+}
+</script>
+</pre>
+</body>
+</html>