Bug 1074557 - Remove the lock in nsWindowMediator. r=Neil
authorBrian Marshall <bmarsd@gmail.com>
Thu, 16 Oct 2014 15:45:17 -0700
changeset 236656 e51b51f049d95ee749da8d859d0395d79b60e14a
parent 236655 32e40c42dc81586848b158294d0358c9fb2ec657
child 236657 06acd829f970fec501f5c2052a5f4420fd9a5904
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersNeil
bugs1074557
milestone36.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 1074557 - Remove the lock in nsWindowMediator. r=Neil
xpfe/appshell/nsWindowMediator.cpp
xpfe/appshell/nsWindowMediator.h
--- a/xpfe/appshell/nsWindowMediator.cpp
+++ b/xpfe/appshell/nsWindowMediator.cpp
@@ -48,18 +48,17 @@ nsWindowMediator::GetDOMWindow(nsIXULWin
   NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE);
 
   outDOMWindow = docShell->GetWindow();
   return outDOMWindow ? NS_OK : NS_ERROR_FAILURE;
 }
 
 nsWindowMediator::nsWindowMediator() :
   mEnumeratorList(), mOldestWindow(nullptr), mTopmostWindow(nullptr),
-  mTimeStamp(0), mSortingZOrder(false), mReady(false),
-  mListLock("nsWindowMediator.mListLock")
+  mTimeStamp(0), mSortingZOrder(false), mReady(false)
 {
 }
 
 nsWindowMediator::~nsWindowMediator()
 {
   while (mOldestWindow)
     UnregisterWindow(mOldestWindow);
 }
@@ -74,16 +73,17 @@ nsresult nsWindowMediator::Init()
   NS_ENSURE_SUCCESS(rv, rv);
 
   mReady = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsWindowMediator::RegisterWindow(nsIXULWindow* inWindow)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_STATE(mReady);
 
   if (GetInfoFor(inWindow)) {
     NS_ERROR("multiple window registration");
     return NS_ERROR_FAILURE;
   }
 
   mTimeStamp++;
@@ -91,30 +91,29 @@ NS_IMETHODIMP nsWindowMediator::Register
   // Create window info struct and add to list of windows
   nsWindowInfo* windowInfo = new nsWindowInfo(inWindow, mTimeStamp);
   if (!windowInfo)
     return NS_ERROR_OUT_OF_MEMORY;
 
   WindowTitleData winData = { inWindow, nullptr };
   mListeners.EnumerateForwards(notifyOpenWindow, &winData);
   
-  MutexAutoLock lock(mListLock);
   if (mOldestWindow)
     windowInfo->InsertAfter(mOldestWindow->mOlder, nullptr);
   else
     mOldestWindow = windowInfo;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsWindowMediator::UnregisterWindow(nsIXULWindow* inWindow)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
   nsWindowInfo *info = GetInfoFor(inWindow);
   if (info)
     return UnregisterWindow(info);
   return NS_ERROR_INVALID_ARG;
 }
 
 nsresult
 nsWindowMediator::UnregisterWindow(nsWindowInfo *inInfo)
@@ -187,66 +186,70 @@ nsWindowMediator::GetInfoFor(nsIWidget *
     listEnd = mOldestWindow;
   }
   return nullptr;
 }
 
 NS_IMETHODIMP
 nsWindowMediator::GetEnumerator(const char16_t* inType, nsISimpleEnumerator** outEnumerator)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_ARG_POINTER(outEnumerator);
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
+
   nsAppShellWindowEnumerator *enumerator = new nsASDOMWindowEarlyToLateEnumerator(inType, *this);
   if (enumerator)
     return enumerator->QueryInterface(NS_GET_IID(nsISimpleEnumerator) , (void**)outEnumerator);
 
   return NS_ERROR_OUT_OF_MEMORY;
 }
 
 NS_IMETHODIMP
 nsWindowMediator::GetXULWindowEnumerator(const char16_t* inType, nsISimpleEnumerator** outEnumerator)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_ARG_POINTER(outEnumerator);
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
+
   nsAppShellWindowEnumerator *enumerator = new nsASXULWindowEarlyToLateEnumerator(inType, *this);
   if (enumerator)
     return enumerator->QueryInterface(NS_GET_IID(nsISimpleEnumerator) , (void**)outEnumerator);
 
   return NS_ERROR_OUT_OF_MEMORY;
 }
 
 NS_IMETHODIMP
 nsWindowMediator::GetZOrderDOMWindowEnumerator(
             const char16_t *aWindowType, bool aFrontToBack,
             nsISimpleEnumerator **_retval)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_ARG_POINTER(_retval);
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
+
   nsAppShellWindowEnumerator *enumerator;
   if (aFrontToBack)
     enumerator = new nsASDOMWindowFrontToBackEnumerator(aWindowType, *this);
   else
     enumerator = new nsASDOMWindowBackToFrontEnumerator(aWindowType, *this);
   if (enumerator)
     return CallQueryInterface(enumerator, _retval);
 
   return NS_ERROR_OUT_OF_MEMORY;
 }
 
 NS_IMETHODIMP
 nsWindowMediator::GetZOrderXULWindowEnumerator(
             const char16_t *aWindowType, bool aFrontToBack,
             nsISimpleEnumerator **_retval)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_ARG_POINTER(_retval);
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
+
   nsAppShellWindowEnumerator *enumerator;
   if (aFrontToBack)
     enumerator = new nsASXULWindowFrontToBackEnumerator(aWindowType, *this);
   else
     enumerator = new nsASXULWindowBackToFrontEnumerator(aWindowType, *this);
   if (enumerator)
     return CallQueryInterface(enumerator, _retval);
 
@@ -265,27 +268,25 @@ nsWindowMediator::RemoveEnumerator(nsApp
   return mEnumeratorList.RemoveElement(inEnumerator);
 }
 
 // Returns the window of type inType ( if null return any window type ) which has the most recent
 // time stamp
 NS_IMETHODIMP
 nsWindowMediator::GetMostRecentWindow(const char16_t* inType, nsIDOMWindow** outWindow)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_ARG_POINTER(outWindow);
   *outWindow = nullptr;
   if (!mReady)
     return NS_OK;
 
   // Find the most window with the highest time stamp that matches
   // the requested type
-
-  MutexAutoLock lock(mListLock);
   nsWindowInfo *info = MostRecentWindowInfo(inType);
-
   if (info && info->mWindow) {
     nsCOMPtr<nsIDOMWindow> DOMWindow;
     if (NS_SUCCEEDED(GetDOMWindow(info->mWindow, DOMWindow))) {  
       *outWindow = DOMWindow;
       NS_ADDREF(*outWindow);
       return NS_OK;
     }
     return NS_ERROR_FAILURE;
@@ -351,33 +352,33 @@ nsWindowMediator::GetCurrentInnerWindowW
   nsCOMPtr<nsIDOMWindow> ret = do_QueryInterface(outer);
   ret.forget(aWindow);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsWindowMediator::UpdateWindowTimeStamp(nsIXULWindow* inWindow)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
   nsWindowInfo *info = GetInfoFor(inWindow);
   if (info) {
     // increment the window's time stamp
     info->mTimeStamp = ++mTimeStamp;
     return NS_OK;
   }
   return NS_ERROR_FAILURE; 
 }
 
 NS_IMETHODIMP
 nsWindowMediator::UpdateWindowTitle(nsIXULWindow* inWindow,
                                     const char16_t* inTitle)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
   if (GetInfoFor(inWindow)) {
     WindowTitleData winData = { inWindow, inTitle };
     mListeners.EnumerateForwards(notifyWindowTitleChange, &winData);
   }
 
   return NS_OK;
 }
 
@@ -391,16 +392,17 @@ NS_IMETHODIMP
 nsWindowMediator::CalculateZPosition(
                 nsIXULWindow   *inWindow,
                 uint32_t        inPosition,
                 nsIWidget      *inBelow,
                 uint32_t       *outPosition,
                 nsIWidget     **outBelow,
                 bool           *outAltered)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_ARG_POINTER(outBelow);
   NS_ENSURE_STATE(mReady);
 
   *outBelow = nullptr;
 
   if (!inWindow || !outPosition || !outAltered)
     return NS_ERROR_NULL_POINTER;
 
@@ -421,18 +423,16 @@ nsWindowMediator::CalculateZPosition(
     *outBelow = inBelow;
     NS_IF_ADDREF(*outBelow);
     return NS_OK;
   }
 
   uint32_t inZ;
   GetZLevel(inWindow, &inZ);
 
-  MutexAutoLock lock(mListLock);
-
   if (inPosition == nsIWindowMediator::zLevelBelow) {
     // locate inBelow. use topmost if it can't be found or isn't in the
     // z-order list
     info = GetInfoFor(inBelow);
     if (!info || (info->mYounger != info && info->mLower == info))
       info = mTopmostWindow;
     else
       found = true;
@@ -521,31 +521,31 @@ nsWindowMediator::CalculateZPosition(
 }
 
 NS_IMETHODIMP
 nsWindowMediator::SetZPosition(
                 nsIXULWindow *inWindow,
                 uint32_t      inPosition,
                 nsIXULWindow *inBelow)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   nsWindowInfo *inInfo,
                *belowInfo;
 
   if ((inPosition != nsIWindowMediator::zLevelTop &&
        inPosition != nsIWindowMediator::zLevelBottom &&
        inPosition != nsIWindowMediator::zLevelBelow) ||
       !inWindow) {
     return NS_ERROR_INVALID_ARG;
   }
 
   if (mSortingZOrder) // don't fight SortZOrder()
     return NS_OK;
 
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
 
   /* Locate inWindow and unlink it from the z-order list.
      It's important we look for it in the age list, not the z-order list.
      This is because the former is guaranteed complete, while
      now may be this window's first exposure to the latter. */
   inInfo = GetInfoFor(inWindow);
   if (!inInfo)
     return NS_ERROR_INVALID_ARG;
@@ -592,18 +592,18 @@ nsWindowMediator::GetZLevel(nsIXULWindow
     // this goes off during window destruction
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsWindowMediator::SetZLevel(nsIXULWindow *aWindow, uint32_t aZLevel)
 {
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
   NS_ENSURE_STATE(mReady);
-  MutexAutoLock lock(mListLock);
 
   nsWindowInfo *info = GetInfoFor(aWindow);
   NS_ASSERTION(info, "setting z level of unregistered window");
   if (!info)
     return NS_ERROR_FAILURE;
 
   if (info->mZLevel != aZLevel) {
     bool lowered = info->mZLevel > aZLevel;
@@ -617,17 +617,17 @@ nsWindowMediator::SetZLevel(nsIXULWindow
 }
 
 /*   Fix potentially out-of-order windows by performing an insertion sort
    on the z-order list. The method will work no matter how broken the
    list, but its assumed usage is immediately after one window's z level
    has been changed, so one window is potentially out of place. Such a sort
    is most efficiently done in a particular direction. Use this one
    if a window's z level has just been reduced, so the sort is most efficiently
-   done front to back. Assumes caller has locked mListLock.
+   done front to back.
      Note it's hardly worth going to all the trouble to write two versions
    of this method except that if we choose the inefficient sorting direction,
    on slow systems windows could visibly bubble around the window that
    was moved.
 */
 void
 nsWindowMediator::SortZOrderFrontToBack()
 {
@@ -772,29 +772,19 @@ nsWindowMediator::RemoveListener(nsIWind
 }
 
 NS_IMETHODIMP
 nsWindowMediator::Observe(nsISupports* aSubject,
                           const char* aTopic,
                           const char16_t* aData)
 {
   if (!strcmp(aTopic, "xpcom-shutdown") && mReady) {
-    // Unregistering a window may cause its destructor to run, causing it to
-    // call into the window mediator, try to acquire mListLock, and deadlock.
-    // Our solution is to hold strong refs to all windows until we release
-    // mListLock.
-    nsTArray<nsCOMPtr<nsIXULWindow> > windows;
-
-    {
-      MutexAutoLock lock(mListLock);
-      while (mOldestWindow) {
-        windows.AppendElement(mOldestWindow->mWindow);
-        UnregisterWindow(mOldestWindow);
-      }
-    }
+    MOZ_RELEASE_ASSERT(NS_IsMainThread());
+    while (mOldestWindow)
+      UnregisterWindow(mOldestWindow);
     mReady = false;
   }
   return NS_OK;
 }
 
 bool
 notifyOpenWindow(nsIWindowMediatorListener *aListener, void* aData)
 {
--- a/xpfe/appshell/nsWindowMediator.h
+++ b/xpfe/appshell/nsWindowMediator.h
@@ -1,17 +1,16 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsWindowMediator_h_
 #define nsWindowMediator_h_
 
-#include "mozilla/Mutex.h"
 #include "nsCOMPtr.h"
 #include "nsIWindowMediator.h"
 #include "nsIObserver.h"
 #include "nsTArray.h"
 #include "nsXPIDLString.h"
 #include "nsWeakReference.h"
 #include "nsCOMArray.h"
 
@@ -19,17 +18,16 @@ class nsAppShellWindowEnumerator;
 class nsASXULWindowEarlyToLateEnumerator;
 class nsASDOMWindowEarlyToLateEnumerator;
 class nsASDOMWindowFrontToBackEnumerator;
 class nsASXULWindowFrontToBackEnumerator;
 class nsASDOMWindowBackToFrontEnumerator;
 class nsASXULWindowBackToFrontEnumerator;
 class nsIWindowMediatorListener;
 struct nsWindowInfo;
-struct PRLock;
 
 class nsWindowMediator :
   public nsIWindowMediator,
   public nsIObserver,
   public nsSupportsWeakReference
 {
 friend class nsAppShellWindowEnumerator;
 friend class nsASXULWindowEarlyToLateEnumerator;
@@ -66,14 +64,13 @@ private:
   void          SortZOrderBackToFront();
 
   nsTArray<nsAppShellWindowEnumerator*> mEnumeratorList;
   nsWindowInfo *mOldestWindow;
   nsWindowInfo *mTopmostWindow;
   int32_t       mTimeStamp;
   bool          mSortingZOrder;
   bool          mReady;
-  mozilla::Mutex mListLock;
 
   nsCOMArray<nsIWindowMediatorListener> mListeners;
 };
 
 #endif