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 73984 9954f0235bc83ce3d0ea4c70e05f6998c8e892b4
parent 73983 3b5da2d845387075947c25c91a554d01803b70e1
child 73985 e2f2476215cd7b5780d66a74d93a57241300b9e8
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
reviewerssmaug, roc
bugs675121
milestone8.0a1
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>