Bug 1061202 - Make sure to not double-add app-theme-changed observers when a document has OnPageShow called on it twice without an OnPageHide call in between. r=khuey, a=bajaj
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 03 Sep 2014 00:09:42 -0400
changeset 224599 f71e2da53622860cfdbcb72cf578563afa849fd2
parent 224598 c15a9479d6338501f9c726e0d77726a341ed0796
child 224600 5443cac376fbd5bf48e8e3834e58beaaecc867ae
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey, bajaj
bugs1061202
milestone34.0a2
Bug 1061202 - Make sure to not double-add app-theme-changed observers when a document has OnPageShow called on it twice without an OnPageHide call in between. r=khuey, a=bajaj
content/base/src/nsDocument.cpp
content/base/src/nsDocument.h
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -1596,16 +1596,22 @@ nsDocument::~nsDocument()
 #ifdef PR_LOGGING
   if (gDocumentLeakPRLog)
     PR_LOG(gDocumentLeakPRLog, PR_LOG_DEBUG,
            ("DOCUMENT %p destroyed", this));
 #endif
 
   NS_ASSERTION(!mIsShowing, "Destroying a currently-showing document");
 
+  // Note: This assert is only non-fatal because mochitest-bc triggers
+  // it... as well as the preceding assert about !mIsShowing.
+  NS_ASSERTION(!mObservingAppThemeChanged,
+               "Document leaked to shutdown, then the observer service dropped "
+               "its ref to us so we were able to go away.");
+
   if (IsTopLevelContentDocument()) {
     //don't report for about: pages
     nsCOMPtr<nsIPrincipal> principal = GetPrincipal();
     nsCOMPtr<nsIURI> uri;
     principal->GetURI(getter_AddRefs(uri));
     bool isAboutScheme = true;
     if (uri) {
       uri->SchemeIs("about", &isAboutScheme);
@@ -8868,17 +8874,20 @@ nsDocument::OnPageShow(bool aPersisted,
   // Dispatch observer notification to notify observers page is shown.
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   nsIPrincipal *principal = GetPrincipal();
   os->NotifyObservers(static_cast<nsIDocument*>(this),
                       nsContentUtils::IsSystemPrincipal(principal) ?
                         "chrome-page-shown" :
                         "content-page-shown",
                       nullptr);
-  os->AddObserver(this, "app-theme-changed", /* ownsWeak */ false);
+  if (!mObservingAppThemeChanged) {
+    os->AddObserver(this, "app-theme-changed", /* ownsWeak */ false);
+    mObservingAppThemeChanged = true;
+  }
 
   DispatchPageTransition(target, NS_LITERAL_STRING("pageshow"), aPersisted);
 }
 
 static bool
 NotifyPageHide(nsIDocument* aDocument, void* aData)
 {
   const bool* aPersistedPtr = static_cast<const bool*>(aData);
@@ -8944,16 +8953,17 @@ nsDocument::OnPageHide(bool aPersisted,
     nsIPrincipal* principal = GetPrincipal();
     os->NotifyObservers(static_cast<nsIDocument*>(this),
                         nsContentUtils::IsSystemPrincipal(principal) ?
                           "chrome-page-hidden" :
                           "content-page-hidden",
                         nullptr);
 
     os->RemoveObserver(this, "app-theme-changed");
+    mObservingAppThemeChanged = false;
   }
 
   DispatchPageTransition(target, NS_LITERAL_STRING("pagehide"), aPersisted);
 
   mVisible = false;
 
   UpdateVisibilityState();
 
--- a/content/base/src/nsDocument.h
+++ b/content/base/src/nsDocument.h
@@ -1593,16 +1593,22 @@ public:
   friend class nsPointerLockPermissionRequest;
   friend class nsCallRequestFullScreen;
   // When set, trying to lock the pointer doesn't require permission from the
   // user.
   bool mAllowRelocking:1;
 
   bool mAsyncFullscreenPending:1;
 
+  // Whether we're observing the "app-theme-changed" observer service
+  // notification.  We need to keep track of this because we might get multiple
+  // OnPageShow notifications in a row without an OnPageHide in between, if
+  // we're getting document.open()/close() called on us.
+  bool mObservingAppThemeChanged:1;
+
   // Keeps track of whether we have a pending
   // 'style-sheet-applicable-state-changed' notification.
   bool mSSApplicableStateNotificationPending:1;
 
   uint32_t mCancelledPointerLockRequests;
 
   uint8_t mXMLDeclarationBits;