Bug 1267568 part 1 - Add a weak ptr to nsGlobalChromeWindow to remember the pres shell we set the fullscreen change flag. r=smaug
authorXidorn Quan <quanxunzhen@gmail.com>
Tue, 03 May 2016 17:58:57 +1000
changeset 295787 dd0fd7a876d62da427a2d4587e13f1837e1b78c7
parent 295786 42f8c2e67ba766e974866dbadbafb9ad7c8e0cab
child 295788 fa640b84845393a371b96a5bc5d17147ecff9df5
push id76089
push userxquan@mozilla.com
push dateTue, 03 May 2016 07:59:48 +0000
treeherdermozilla-inbound@d95633a61869 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1267568, 1177155
milestone49.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 1267568 part 1 - Add a weak ptr to nsGlobalChromeWindow to remember the pres shell we set the fullscreen change flag. r=smaug This addresses the review comment from bug 1177155 comment 16 so that the assertion and code to avoid breaking assertion in valid path are no longer needed. MozReview-Commit-ID: GHIYQwyoejC
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -6456,38 +6456,38 @@ nsGlobalWindow::SetFullscreenInternal(Fu
     if (MakeWidgetFullscreen(this, aHMD, aReason, aFullScreen)) {
       // The rest of code for switching fullscreen is in nsGlobalWindow::
       // FinishFullscreenChange() which will be called after sizemodechange
       // event is dispatched.
       return NS_OK;
     }
   }
 
-  // If we didn't setup the widget, we may need to manually set this
-  // flag, or the assertion in FinishFullscreenChange is violated.
-  if (nsCOMPtr<nsIPresShell> presShell = mDocShell->GetPresShell()) {
-    if (!presShell->IsInFullscreenChange()) {
-      presShell->SetIsInFullscreenChange(true);
-    }
-  }
   FinishFullscreenChange(aFullScreen);
   return NS_OK;
 }
 
 bool
 nsGlobalWindow::SetWidgetFullscreen(FullscreenReason aReason, bool aIsFullscreen,
                                     nsIWidget* aWidget, nsIScreen* aScreen)
 {
   MOZ_ASSERT(IsOuterWindow());
   MOZ_ASSERT(this == GetTopInternal(), "Only topmost window should call this");
   MOZ_ASSERT(!AsOuter()->GetFrameElementInternal(), "Content window should not call this");
   MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
 
-  if (nsCOMPtr<nsIPresShell> presShell = mDocShell->GetPresShell()) {
-    presShell->SetIsInFullscreenChange(true);
+  if (!NS_WARN_IF(!IsChromeWindow())) {
+    auto chromeWin = static_cast<nsGlobalChromeWindow*>(this);
+    if (!NS_WARN_IF(chromeWin->mFullscreenPresShell)) {
+      if (nsIPresShell* shell = mDocShell->GetPresShell()) {
+        chromeWin->mFullscreenPresShell = do_GetWeakReference(shell);
+        MOZ_ASSERT(chromeWin->mFullscreenPresShell);
+        shell->SetIsInFullscreenChange(true);
+      }
+    }
   }
   nsresult rv = aReason == FullscreenReason::ForFullscreenMode ?
     // If we enter fullscreen for fullscreen mode, we want
     // the native system behavior.
     aWidget->MakeFullScreenWithNativeTransition(aIsFullscreen, aScreen) :
     aWidget->MakeFullScreen(aIsFullscreen, aScreen);
   return NS_SUCCEEDED(rv);
 }
@@ -6519,19 +6519,23 @@ nsGlobalWindow::FinishFullscreenChange(b
   // that the chrome can distinguish between browser fullscreen mode
   // and DOM fullscreen.
   FinishDOMFullscreenChange(mDoc, mFullScreen);
 
   // dispatch a "fullscreen" DOM event so that XUL apps can
   // respond visually if we are kicked into full screen mode
   DispatchCustomEvent(NS_LITERAL_STRING("fullscreen"));
 
-  if (nsCOMPtr<nsIPresShell> presShell = mDocShell->GetPresShell()) {
-    MOZ_ASSERT(presShell->IsInFullscreenChange());
-    presShell->SetIsInFullscreenChange(false);
+  if (!NS_WARN_IF(!IsChromeWindow())) {
+    auto chromeWin = static_cast<nsGlobalChromeWindow*>(this);
+    if (nsCOMPtr<nsIPresShell> shell =
+        do_QueryReferent(chromeWin->mFullscreenPresShell)) {
+      shell->SetIsInFullscreenChange(false);
+      chromeWin->mFullscreenPresShell = nullptr;
+    }
   }
 
   if (!mWakeLock && mFullScreen) {
     RefPtr<power::PowerManagerService> pmService =
       power::PowerManagerService::GetInstance();
     if (!pmService) {
       return;
     }
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -1974,16 +1974,20 @@ public:
   using nsGlobalWindow::NotifyDefaultButtonLoaded;
   using nsGlobalWindow::GetMessageManager;
   using nsGlobalWindow::GetGroupMessageManager;
   using nsGlobalWindow::BeginWindowMove;
 
   nsCOMPtr<nsIBrowserDOMWindow> mBrowserDOMWindow;
   nsCOMPtr<nsIMessageBroadcaster> mMessageManager;
   nsInterfaceHashtable<nsStringHashKey, nsIMessageBroadcaster> mGroupMessageManagers;
+  // A weak pointer to the nsPresShell that we are doing fullscreen for.
+  // The pointer being set indicates we've set the IsInFullscreenChange
+  // flag on this pres shell.
+  nsWeakPtr mFullscreenPresShell;
 };
 
 /*
  * nsGlobalModalWindow inherits from nsGlobalWindow. It is the global
  * object created for a modal content windows only (i.e. not modal
  * chrome dialogs).
  */
 class nsGlobalModalWindow : public nsGlobalWindow,