Bug 818371. Don't fire visibility change events until we're done upda…

…ting vsibility states in the entire docshell tree. r=smaug
dothq · Dec 6, 2012 · 5b75fd5 · 5b75fd5
1 parent f12bcf0
commit 5b75fd5
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 29 deletions.
diff --git a/content/base/public/nsIDocument.h b/content/base/public/nsIDocument.h
@@ -79,8 +79,8 @@ class Element;
 } // namespace mozilla
 
 #define NS_IDOCUMENT_IID \
-{ 0xd69b94c2, 0x92ed, 0x4baa, \
-  { 0x82, 0x08, 0x56, 0xe4, 0xc4, 0xb3, 0xf3, 0xc8 } }
+{ 0xcc604bdc, 0xd55e, 0x4918, \
+ { 0xaa, 0x82, 0xb2, 0xde, 0xbf, 0x01, 0x09, 0x5d } }
 
 // Flag for AddStyleSheet().
 #define NS_STYLESHEET_FROM_CATALOG                (1 << 0)
@@ -1669,7 +1669,9 @@ class nsIDocument : public nsINode
 #undef DEPRECATED_OPERATION
   void WarnOnceAbout(DeprecatedOperations aOperation, bool asError = false);
 
-  virtual void PostVisibilityUpdateEvent() = 0;
+  // This method may fire a DOM event; if it does so it will happen
+  // synchronously if aFireEventSync is true, asynchronously otherwise.
+  virtual void UpdateVisibilityState(bool aFireEventSync) = 0;
 
   bool IsSyntheticDocument() { return mIsSyntheticDocument; }
 

diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp
@@ -7063,7 +7063,7 @@ nsDocument::OnPageShow(bool aPersisted,
     SetImagesNeedAnimating(true);
   }
 
-  UpdateVisibilityState();
+  UpdateVisibilityState(true);
 
   nsCOMPtr<nsIDOMEventTarget> target = aDispatchStartTarget;
   if (!target) {
@@ -7125,7 +7125,7 @@ nsDocument::OnPageHide(bool aPersisted,
 
   mVisible = false;
 
-  UpdateVisibilityState();
+  UpdateVisibilityState(true);
 
   EnumerateExternalResources(NotifyPageHide, &aPersisted);
   EnumerateFreezableElements(NotifyActivityChanged, nullptr);
@@ -9482,25 +9482,37 @@ nsDocument::GetMozPointerLockElement(nsIDOMElement** aPointerLockedElement)
 #undef TOUCH_EVENT
 #undef EVENT
 
-void
-nsDocument::UpdateVisibilityState()
+/* virtual */ void
+nsDocument::UpdateVisibilityState(bool aFireEventSync)
 {
   VisibilityState oldState = mVisibilityState;
   mVisibilityState = GetVisibilityState();
   if (oldState != mVisibilityState) {
-    nsContentUtils::DispatchTrustedEvent(this, static_cast<nsIDocument*>(this),
-                                         NS_LITERAL_STRING("visibilitychange"),
-                                         /* bubbles = */ true,
-                                         /* cancelable = */ false);
-    nsContentUtils::DispatchTrustedEvent(this, static_cast<nsIDocument*>(this),
-                                         NS_LITERAL_STRING("mozvisibilitychange"),
-                                         /* bubbles = */ true,
-                                         /* cancelable = */ false);
+    if (aFireEventSync) {
+      FireVisibilityChangeEvent();
+    } else {
+      nsCOMPtr<nsIRunnable> event =
+        NS_NewRunnableMethod(this, &nsDocument::FireVisibilityChangeEvent);
+      NS_DispatchToMainThread(event);
+    }
 
     EnumerateFreezableElements(NotifyActivityChanged, nullptr);
   }
 }
 
+void
+nsDocument::FireVisibilityChangeEvent()
+{
+  nsContentUtils::DispatchTrustedEvent(this, static_cast<nsIDocument*>(this),
+                                       NS_LITERAL_STRING("visibilitychange"),
+                                       /* bubbles = */ true,
+                                       /* cancelable = */ false);
+  nsContentUtils::DispatchTrustedEvent(this, static_cast<nsIDocument*>(this),
+                                       NS_LITERAL_STRING("mozvisibilitychange"),
+                                       /* bubbles = */ true,
+                                       /* cancelable = */ false);
+}
+
 nsDocument::VisibilityState
 nsDocument::GetVisibilityState() const
 {
@@ -9519,14 +9531,6 @@ nsDocument::GetVisibilityState() const
   return eVisible;
 }
 
-/* virtual */ void
-nsDocument::PostVisibilityUpdateEvent()
-{
-  nsCOMPtr<nsIRunnable> event =
-    NS_NewRunnableMethod(this, &nsDocument::UpdateVisibilityState);
-  NS_DispatchToMainThread(event);
-}
-
 NS_IMETHODIMP
 nsDocument::GetMozHidden(bool* aHidden)
 {

diff --git a/content/base/src/nsDocument.h b/content/base/src/nsDocument.h
@@ -995,11 +995,8 @@ class nsDocument : public nsIDocument,
   bool SetPointerLock(Element* aElement, int aCursorStyle);
   static void UnlockPointer();
 
-  // This method may fire a DOM event; if it does so it will happen
-  // synchronously.
-  void UpdateVisibilityState();
-  // Posts an event to call UpdateVisibilityState
-  virtual void PostVisibilityUpdateEvent();
+  virtual void UpdateVisibilityState(bool aFireEventSync);
+  void FireVisibilityChangeEvent();
 
   virtual void DocSizeOfExcludingThis(nsWindowSizes* aWindowSizes) const;
   // DocSizeOfIncludingThis is inherited from nsIDocument.

diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -5211,7 +5211,7 @@ nsDocShell::SetIsActive(bool aIsActive)
       win->SetIsBackground(!aIsActive);
       nsCOMPtr<nsIDocument> doc = do_QueryInterface(win->GetExtantDocument());
       if (doc) {
-          doc->PostVisibilityUpdateEvent();
+          doc->UpdateVisibilityState(false);
       }
   }
 

diff --git a/docshell/test/chrome/Makefile.in b/docshell/test/chrome/Makefile.in
@@ -96,6 +96,7 @@ MOCHITEST_CHROME_FILES =	\
 		test_bug789773.xul \
 		test_bug754029.xul \
 		bug754029_window.xul \
+		test_bug818371.xul \
     docshell_helpers.js \
     generic.html \
     $(NULL)

diff --git a/docshell/test/chrome/test_bug818371.xul b/docshell/test/chrome/test_bug818371.xul
@@ -0,0 +1,43 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
+<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=818371
+-->
+<window title="Mozilla Bug 818371"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=818371"
+     target="_blank">Mozilla Bug 818371</a>
+  </body>
+
+  <browser id="b" src="data:text/html,&lt;iframe&gt;&lt;/iframe&gt;"></browser>
+
+  <!-- test code goes here -->
+  <script type="application/javascript">
+  <![CDATA[
+  /** Test for Bug 818371 **/
+  SimpleTest.waitForExplicitFinish();
+  addLoadEvent(function() {
+    function listener(e) {
+      ok(e.target.hidden, "Document should now be hidden");
+      ok(e.target.defaultView.frames[0].document.hidden,
+         "Subdocument should now be hidden");
+      e.target.removeEventListener("visibilitychange", listener);
+      SimpleTest.finish();
+    }
+
+    var doc = frames[0].document;
+    ok(!doc.hidden, "Document should be visible");
+    ok(!frames[0].frames[0].document.hidden,
+       "Subdocument should now be hidden");
+    doc.addEventListener("visibilitychange", listener);
+    $("b").docShell.isActive = false;
+  });
+
+  ]]>
+  </script>
+</window>