Bug 1137009 - part 1 - Always save dirty window attributes but do not persist them in nsXULWindow::SavePersistentAttributes() directly. r=enndeakin, a=lizzard
authorXidorn Quan <quanxunzhen@gmail.com>
Thu, 24 Sep 2015 11:39:22 +1000
changeset 296209 7e06450a934c166a81645b56b1818f6e024e6397
parent 296208 1c3b3f30e9f8925c632fc4d5be5f2af1a81b9e1b
child 296210 735ebc504963a025a007611bad8c9bf7f05c9ed7
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersenndeakin, lizzard
bugs1137009
milestone43.0a2
Bug 1137009 - part 1 - Always save dirty window attributes but do not persist them in nsXULWindow::SavePersistentAttributes() directly. r=enndeakin, a=lizzard The attributes should eventually be saved via the AttributeChanged callback in XULDocument if one is specified in the "persist" attribute.
xpfe/appshell/nsWebShellWindow.cpp
xpfe/appshell/nsXULWindow.cpp
xpfe/appshell/nsXULWindow.h
--- a/xpfe/appshell/nsWebShellWindow.cpp
+++ b/xpfe/appshell/nsWebShellWindow.cpp
@@ -414,18 +414,18 @@ nsWebShellWindow::WindowActivated()
 
   // focusing the window could cause it to close, so keep a reference to it
   nsCOMPtr<nsIDOMWindow> window = mDocShell ? mDocShell->GetWindow() : nullptr;
   nsCOMPtr<nsIFocusManager> fm = do_GetService(FOCUSMANAGER_CONTRACTID);
   if (fm && window)
     fm->WindowRaised(window);
 
   if (mChromeLoaded) {
-    PersistentAttributesDirty(PAD_POSITION | PAD_SIZE | PAD_MISC);
-    SavePersistentAttributes();
+    SetAttributesDirty(PAD_POSITION | PAD_SIZE | PAD_MISC);
+    SaveAttributes();
    }
 }
 
 void
 nsWebShellWindow::WindowDeactivated()
 {
   nsCOMPtr<nsIXULWindow> xulWindow(this);
 
@@ -507,24 +507,24 @@ nsWebShellWindow::SetPersistenceTimer(ui
     }
   }
 
   nsRefPtr<WebShellWindowTimerCallback> callback =
     new WebShellWindowTimerCallback(this);
   mSPTimer->InitWithCallback(callback, SIZE_PERSISTENCE_TIMEOUT,
                              nsITimer::TYPE_ONE_SHOT);
 
-  PersistentAttributesDirty(aDirtyFlags);
+  SetAttributesDirty(aDirtyFlags);
 }
 
 void
 nsWebShellWindow::FirePersistenceTimer()
 {
   MutexAutoLock lock(mSPTimerLock);
-  SavePersistentAttributes();
+  SaveAttributes();
 }
 
 
 //----------------------------------------
 // nsIWebProgessListener implementation
 //----------------------------------------
 NS_IMETHODIMP
 nsWebShellWindow::OnProgressChange(nsIWebProgress *aProgress,
@@ -767,14 +767,14 @@ NS_IMETHODIMP nsWebShellWindow::Destroy(
     webProgress->RemoveProgressListener(this);
   }
 
   nsCOMPtr<nsIXULWindow> kungFuDeathGrip(this);
   {
     MutexAutoLock lock(mSPTimerLock);
     if (mSPTimer) {
       mSPTimer->Cancel();
-      SavePersistentAttributes();
+      SaveAttributes();
       mSPTimer = nullptr;
     }
   }
   return nsXULWindow::Destroy();
 }
--- a/xpfe/appshell/nsXULWindow.cpp
+++ b/xpfe/appshell/nsXULWindow.cpp
@@ -217,18 +217,18 @@ NS_IMETHODIMP nsXULWindow::SetZLevel(uin
     nsSizeMode sizeMode = mWindow->SizeMode();
     if (sizeMode == nsSizeMode_Maximized || sizeMode == nsSizeMode_Fullscreen) {
       return NS_ERROR_FAILURE;
     }
   }
 
   // do it
   mediator->SetZLevel(this, aLevel);
-  PersistentAttributesDirty(PAD_MISC);
-  SavePersistentAttributes();
+  SetAttributesDirty(PAD_MISC);
+  SaveAttributes();
 
   nsCOMPtr<nsIContentViewer> cv;
   mDocShell->GetContentViewer(getter_AddRefs(cv));
   if (cv) {
     nsCOMPtr<nsIDocument> doc = cv->GetDocument();
     if (doc) {
       ErrorResult rv;
       nsRefPtr<dom::Event> event =
@@ -541,18 +541,18 @@ NS_IMETHODIMP nsXULWindow::SetPosition(i
   nsresult rv = mWindow->Move(aX * invScale, aY * invScale);
   NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
   if (!mChromeLoaded) {
     // If we're called before the chrome is loaded someone obviously wants this
     // window at this position. We don't persist this one-time position.
     mIgnoreXULPosition = true;
     return NS_OK;
   }
-  PersistentAttributesDirty(PAD_POSITION);
-  SavePersistentAttributes();
+  SetAttributesDirty(PAD_POSITION);
+  SaveAttributes();
   return NS_OK;
 }
 
 NS_IMETHODIMP nsXULWindow::GetPosition(int32_t* aX, int32_t* aY)
 {
   return GetPositionAndSize(aX, aY, nullptr, nullptr);
 }
 
@@ -573,18 +573,18 @@ NS_IMETHODIMP nsXULWindow::SetSize(int32
     // If we're called before the chrome is loaded someone obviously wants this
     // window at this size & in the normal size mode (since it is the only mode
     // in which setting dimensions makes sense). We don't persist this one-time
     // size.
     mIgnoreXULSize = true;
     mIgnoreXULSizeMode = true;
     return NS_OK;
   }
-  PersistentAttributesDirty(PAD_SIZE);
-  SavePersistentAttributes();
+  SetAttributesDirty(PAD_SIZE);
+  SaveAttributes();
   return NS_OK;
 }
 
 NS_IMETHODIMP nsXULWindow::GetSize(int32_t* aCX, int32_t* aCY)
 {
   return GetPositionAndSize(nullptr, nullptr, aCX, aCY);
 }
 
@@ -607,18 +607,18 @@ NS_IMETHODIMP nsXULWindow::SetPositionAn
   if (!mChromeLoaded) {
     // If we're called before the chrome is loaded someone obviously wants this
     // window at this size and position. We don't persist this one-time setting.
     mIgnoreXULPosition = true;
     mIgnoreXULSize = true;
     mIgnoreXULSizeMode = true;
     return NS_OK;
   }
-  PersistentAttributesDirty(PAD_POSITION | PAD_SIZE);
-  SavePersistentAttributes();
+  SetAttributesDirty(PAD_POSITION | PAD_SIZE);
+  SaveAttributes();
   return NS_OK;
 }
 
 NS_IMETHODIMP nsXULWindow::GetPositionAndSize(int32_t* x, int32_t* y, int32_t* cx,
    int32_t* cy)
 {
   nsIntRect rect;
 
@@ -1424,32 +1424,27 @@ void nsXULWindow::SyncAttributesToWidget
 
   // "macanimationtype" attribute
   windowElement->GetAttribute(NS_LITERAL_STRING("macanimationtype"), attr);
   if (attr.EqualsLiteral("document")) {
     mWindow->SetWindowAnimationType(nsIWidget::eDocumentWindowAnimation);
   }
 }
 
-NS_IMETHODIMP nsXULWindow::SavePersistentAttributes()
+void nsXULWindow::SaveAttributes()
 {
   // can happen when the persistence timer fires at an inopportune time
   // during window shutdown
-  if (!mDocShell)
-    return NS_ERROR_FAILURE;
+  if (!mDocShell) {
+    return;
+  }
 
   nsCOMPtr<dom::Element> docShellElement = GetWindowDOMElement();
-  if (!docShellElement)
-    return NS_ERROR_FAILURE;
-
-  nsAutoString   persistString;
-  docShellElement->GetAttribute(PERSIST_ATTRIBUTE, persistString);
-  if (persistString.IsEmpty()) { // quick check which sometimes helps
-    mPersistentAttributesDirty = 0;
-    return NS_OK;
+  if (!docShellElement) {
+    return;
   }
 
   // get our size, position and mode to persist
   nsIntRect rect;
   bool gotRestoredBounds = NS_SUCCEEDED(mWindow->GetRestoredBounds(rect));
 
   CSSToLayoutDeviceScale scale = mWindow->GetDefaultScale();
 
@@ -1460,90 +1455,63 @@ NS_IMETHODIMP nsXULWindow::SavePersisten
     if (NS_SUCCEEDED(parent->GetPosition(&parentX, &parentY))) {
       rect.x -= parentX;
       rect.y -= parentY;
     }
   }
 
   char                        sizeBuf[10];
   nsAutoString                sizeString;
-  nsAutoString                windowElementId;
-  nsCOMPtr<nsIDOMXULDocument> ownerXULDoc;
-
-  // fetch docShellElement's ID and XUL owner document
-  ownerXULDoc = do_QueryInterface(docShellElement->OwnerDoc());
-  if (docShellElement->IsXULElement()) {
-    docShellElement->GetId(windowElementId);
-  }
 
   ErrorResult rv;
   // (only for size elements which are persisted)
   if ((mPersistentAttributesDirty & PAD_POSITION) && gotRestoredBounds) {
-    if (persistString.Find("screenX") >= 0) {
-      PR_snprintf(sizeBuf, sizeof(sizeBuf), "%d", NSToIntRound(rect.x / scale.scale));
-      sizeString.AssignWithConversion(sizeBuf);
-      docShellElement->SetAttribute(SCREENX_ATTRIBUTE, sizeString, rv);
-      if (ownerXULDoc) // force persistence in case the value didn't change
-        ownerXULDoc->Persist(windowElementId, SCREENX_ATTRIBUTE);
-    }
-    if (persistString.Find("screenY") >= 0) {
-      PR_snprintf(sizeBuf, sizeof(sizeBuf), "%d", NSToIntRound(rect.y / scale.scale));
-      sizeString.AssignWithConversion(sizeBuf);
-      docShellElement->SetAttribute(SCREENY_ATTRIBUTE, sizeString, rv);
-      if (ownerXULDoc)
-        ownerXULDoc->Persist(windowElementId, SCREENY_ATTRIBUTE);
-    }
+    PR_snprintf(sizeBuf, sizeof(sizeBuf), "%d", NSToIntRound(rect.x / scale.scale));
+    sizeString.AssignWithConversion(sizeBuf);
+    docShellElement->SetAttribute(SCREENX_ATTRIBUTE, sizeString, rv);
+
+    PR_snprintf(sizeBuf, sizeof(sizeBuf), "%d", NSToIntRound(rect.y / scale.scale));
+    sizeString.AssignWithConversion(sizeBuf);
+    docShellElement->SetAttribute(SCREENY_ATTRIBUTE, sizeString, rv);
   }
 
   if ((mPersistentAttributesDirty & PAD_SIZE) && gotRestoredBounds) {
-    if (persistString.Find("width") >= 0) {
-      PR_snprintf(sizeBuf, sizeof(sizeBuf), "%d", NSToIntRound(rect.width / scale.scale));
-      sizeString.AssignWithConversion(sizeBuf);
-      docShellElement->SetAttribute(WIDTH_ATTRIBUTE, sizeString, rv);
-      if (ownerXULDoc)
-        ownerXULDoc->Persist(windowElementId, WIDTH_ATTRIBUTE);
-    }
-    if (persistString.Find("height") >= 0) {
-      PR_snprintf(sizeBuf, sizeof(sizeBuf), "%d", NSToIntRound(rect.height / scale.scale));
-      sizeString.AssignWithConversion(sizeBuf);
-      docShellElement->SetAttribute(HEIGHT_ATTRIBUTE, sizeString, rv);
-      if (ownerXULDoc)
-        ownerXULDoc->Persist(windowElementId, HEIGHT_ATTRIBUTE);
-    }
+    PR_snprintf(sizeBuf, sizeof(sizeBuf), "%d", NSToIntRound(rect.width / scale.scale));
+    sizeString.AssignWithConversion(sizeBuf);
+    docShellElement->SetAttribute(WIDTH_ATTRIBUTE, sizeString, rv);
+
+    PR_snprintf(sizeBuf, sizeof(sizeBuf), "%d", NSToIntRound(rect.height / scale.scale));
+    sizeString.AssignWithConversion(sizeBuf);
+    docShellElement->SetAttribute(HEIGHT_ATTRIBUTE, sizeString, rv);
   }
 
   if (mPersistentAttributesDirty & PAD_MISC) {
     nsSizeMode sizeMode = mWindow->SizeMode();
 
     if (sizeMode != nsSizeMode_Minimized) {
       if (sizeMode == nsSizeMode_Maximized)
         sizeString.Assign(SIZEMODE_MAXIMIZED);
       else if (sizeMode == nsSizeMode_Fullscreen)
         sizeString.Assign(SIZEMODE_FULLSCREEN);
       else
         sizeString.Assign(SIZEMODE_NORMAL);
       docShellElement->SetAttribute(MODE_ATTRIBUTE, sizeString, rv);
-      if (ownerXULDoc && persistString.Find("sizemode") >= 0)
-        ownerXULDoc->Persist(windowElementId, MODE_ATTRIBUTE);
     }
-    if (persistString.Find("zlevel") >= 0) {
-      uint32_t zLevel;
-      nsCOMPtr<nsIWindowMediator> mediator(do_GetService(NS_WINDOWMEDIATOR_CONTRACTID));
-      if (mediator) {
-        mediator->GetZLevel(this, &zLevel);
-        PR_snprintf(sizeBuf, sizeof(sizeBuf), "%lu", (unsigned long)zLevel);
-        sizeString.AssignWithConversion(sizeBuf);
-        docShellElement->SetAttribute(ZLEVEL_ATTRIBUTE, sizeString, rv);
-        ownerXULDoc->Persist(windowElementId, ZLEVEL_ATTRIBUTE);
-      }
+
+    uint32_t zLevel;
+    nsCOMPtr<nsIWindowMediator> mediator(do_GetService(NS_WINDOWMEDIATOR_CONTRACTID));
+    if (mediator) {
+      mediator->GetZLevel(this, &zLevel);
+      PR_snprintf(sizeBuf, sizeof(sizeBuf), "%lu", (unsigned long)zLevel);
+      sizeString.AssignWithConversion(sizeBuf);
+      docShellElement->SetAttribute(ZLEVEL_ATTRIBUTE, sizeString, rv);
     }
   }
 
   mPersistentAttributesDirty = 0;
-  return NS_OK;
 }
 
 NS_IMETHODIMP nsXULWindow::GetWindowDOMWindow(nsIDOMWindow** aDOMWindow)
 {
   NS_ENSURE_STATE(mDocShell);
 
   if (!mDOMWindow)
     mDOMWindow = mDocShell->GetWindow();
@@ -2003,17 +1971,17 @@ bool nsXULWindow::GetContentScrollbarVis
     if (prefValue == nsIScrollable::Scrollbar_Never)
       return false;
   }
 
   return true;
 }
 
 // during spinup, attributes that haven't been loaded yet can't be dirty
-void nsXULWindow::PersistentAttributesDirty(uint32_t aDirtyFlags)
+void nsXULWindow::SetAttributesDirty(uint32_t aDirtyFlags)
 {
   mPersistentAttributesDirty |= aDirtyFlags & mPersistentAttributesMask;
 }
 
 NS_IMETHODIMP nsXULWindow::ApplyChromeFlags()
 {
   nsCOMPtr<dom::Element> window = GetWindowDOMElement();
   NS_ENSURE_TRUE(window, NS_ERROR_FAILURE);
--- a/xpfe/appshell/nsXULWindow.h
+++ b/xpfe/appshell/nsXULWindow.h
@@ -91,17 +91,17 @@ protected:
 
    void OnChromeLoaded();
    void StaggerPosition(int32_t &aRequestedX, int32_t &aRequestedY,
                         int32_t aSpecWidth, int32_t aSpecHeight);
    bool       LoadPositionFromXUL();
    bool       LoadSizeFromXUL();
    bool       LoadMiscPersistentAttributesFromXUL();
    void       SyncAttributesToWidget();
-   NS_IMETHOD SavePersistentAttributes();
+   void       SaveAttributes();
 
    NS_IMETHOD GetWindowDOMWindow(nsIDOMWindow** aDOMWindow);
    mozilla::dom::Element* GetWindowDOMElement() const;
 
    // See nsIDocShellTreeOwner for docs on next two methods
    nsresult ContentShellAdded(nsIDocShellTreeItem* aContentShell,
                                           bool aPrimary, bool aTargetable,
                                           const nsAString& aID);
@@ -114,17 +114,17 @@ protected:
 
    void       EnableParent(bool aEnable);
    bool       ConstrainToZLevel(bool aImmediate, nsWindowZ *aPlacement,
                                 nsIWidget *aReqBelow, nsIWidget **aActualBelow);
    void       PlaceWindowLayersBehind(uint32_t aLowLevel, uint32_t aHighLevel,
                                       nsIXULWindow *aBehind);
    void       SetContentScrollbarVisibility(bool aVisible);
    bool       GetContentScrollbarVisibility();
-   void       PersistentAttributesDirty(uint32_t aDirtyFlags);
+   void       SetAttributesDirty(uint32_t aDirtyFlags);
 
    nsChromeTreeOwner*      mChromeTreeOwner;
    nsContentTreeOwner*     mContentTreeOwner;
    nsContentTreeOwner*     mPrimaryContentTreeOwner;
    nsCOMPtr<nsIWidget>     mWindow;
    nsCOMPtr<nsIDocShell>   mDocShell;
    nsCOMPtr<nsIDOMWindow>  mDOMWindow;
    nsCOMPtr<nsIWeakReference> mParentWindow;