No issue - Modernize nsWindowWatcher

Get rid of old hacky conditional code and use weak references.

embedding/components/windowwatcher/nsWindowWatcher.cpp

@@ -3,8 +3,6 @@
  * 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/. */
 
-//#define USEWEAKREFS // (haven't quite figured that out yet)
-
 #include "nsWindowWatcher.h"
 #include "nsAutoWindowStateHelper.h"
 
@@ -70,10 +68,6 @@
 #include "nsIXULBrowserWindow.h"
 #include "nsGlobalWindow.h"
 
-#ifdef USEWEAKREFS
-#include "nsIWeakReference.h"
-#endif
-
 using namespace mozilla;
 using namespace mozilla::dom;
 
@@ -87,20 +81,9 @@ struct nsWatcherWindowEntry
 {
 
   nsWatcherWindowEntry(mozIDOMWindowProxy* aWindow, nsIWebBrowserChrome* aChrome)
-    : mChrome(nullptr)
+    : mWindow(do_GetWeakReference(aWindow))
+    , mChrome(do_GetWeakReference(aChrome))
   {
-#ifdef USEWEAKREFS
-    mWindow = do_GetWeakReference(aWindow);
-#else
-    mWindow = aWindow;
-#endif
-    nsCOMPtr<nsISupportsWeakReference> supportsweak(do_QueryInterface(aChrome));
-    if (supportsweak) {
-      supportsweak->GetWeakReference(getter_AddRefs(mChromeWeak));
-    } else {
-      mChrome = aChrome;
-      mChromeWeak = nullptr;
-    }
     ReferenceSelf();
   }
   ~nsWatcherWindowEntry() {}
@@ -109,13 +92,8 @@ struct nsWatcherWindowEntry
   void Unlink();
   void ReferenceSelf();
 
-#ifdef USEWEAKREFS
-  nsCOMPtr<nsIWeakReference> mWindow;
-#else // still not an owning ref
-  mozIDOMWindowProxy* mWindow;
-#endif
-  nsIWebBrowserChrome* mChrome;
-  nsWeakPtr mChromeWeak;
+  nsWeakPtr mWindow;
+  nsWeakPtr mChrome;
   // each struct is in a circular, doubly-linked list
   nsWatcherWindowEntry* mYounger; // next younger in sequence
   nsWatcherWindowEntry* mOlder;
@@ -219,24 +197,17 @@ nsWatcherWindowEnumerator::GetNext(nsISupports** aResult)
 
   *aResult = nullptr;
 
-#ifdef USEWEAKREFS
   while (mCurrentPosition) {
-    CallQueryReferent(mCurrentPosition->mWindow, aResult);
-    if (*aResult) {
+    nsCOMPtr<mozIDOMWindowProxy> window =
+        do_QueryReferent(mCurrentPosition->mWindow);
+    if (window) {
+      CallQueryInterface(window, aResult);
       mCurrentPosition = FindNext();
-      break;
-    } else { // window is gone!
-      mWindowWatcher->RemoveWindow(mCurrentPosition);
+      return NS_OK;
     }
-  }
-  NS_IF_ADDREF(*aResult);
-#else
-  if (mCurrentPosition) {
-    CallQueryInterface(mCurrentPosition->mWindow, aResult);
     mCurrentPosition = FindNext();
   }
-#endif
-  return NS_OK;
+  return NS_ERROR_FAILURE;
 }
 
 nsWatcherWindowEntry*
@@ -1480,14 +1451,7 @@ nsWindowWatcher::AddWindow(mozIDOMWindowProxy* aWindow, nsIWebBrowserChrome* aCh
     // its chrome mapping and return
     info = FindWindowEntry(aWindow);
     if (info) {
-      nsCOMPtr<nsISupportsWeakReference> supportsweak(
-        do_QueryInterface(aChrome));
-      if (supportsweak) {
-        supportsweak->GetWeakReference(getter_AddRefs(info->mChromeWeak));
-      } else {
-        info->mChrome = aChrome;
-        info->mChromeWeak = nullptr;
-      }
+      info->mChrome = do_GetWeakReference(aChrome);
       return NS_OK;
     }
 
@@ -1539,38 +1503,18 @@ nsWindowWatcher::FindWindowEntry(mozIDOMWindowProxy* aWindow)
   // find the corresponding nsWatcherWindowEntry
   nsWatcherWindowEntry* info;
   nsWatcherWindowEntry* listEnd;
-#ifdef USEWEAKREFS
-  nsresult rv;
-  bool found;
-#endif
 
   info = mOldestWindow;
   listEnd = 0;
-#ifdef USEWEAKREFS
-  rv = NS_OK;
-  found = false;
-  while (info != listEnd && NS_SUCCEEDED(rv)) {
-    nsCOMPtr<mozIDOMWindowProxy> infoWindow(do_QueryReferent(info->mWindow));
-    if (!infoWindow) { // clean up dangling reference, while we're here
-      rv = RemoveWindow(info);
-    } else if (infoWindow.get() == aWindow) {
-      return info;
-    }
-
-    info = info->mYounger;
-    listEnd = mOldestWindow;
-  }
-  return 0;
-#else
   while (info != listEnd) {
-    if (info->mWindow == aWindow) {
+    nsCOMPtr<mozIDOMWindowProxy> window = do_QueryReferent(info->mWindow);
+    if (window && window == aWindow) {
       return info;
     }
     info = info->mYounger;
     listEnd = mOldestWindow;
   }
   return 0;
-#endif
 }
 
 nsresult
@@ -1596,16 +1540,11 @@ nsWindowWatcher::RemoveWindow(nsWatcherWindowEntry* aInfo)
   // send notifications.
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {
-#ifdef USEWEAKREFS
-    nsCOMPtr<nsISupports> domwin(do_QueryReferent(aInfo->mWindow));
-    if (domwin) {
-      os->NotifyObservers(domwin, "domwindowclosed", 0);
+    nsCOMPtr<mozIDOMWindowProxy> window = do_QueryReferent(aInfo->mWindow);
+    if (window) {
+      nsCOMPtr<nsISupports> domwin(do_QueryInterface(window));
+      os->NotifyObservers(domwin, "domwindowclosed", nullptr);
     }
-    // else bummer. since the window is gone, there's nothing to notify with.
-#else
-    nsCOMPtr<nsISupports> domwin(do_QueryInterface(aInfo->mWindow));
-    os->NotifyObservers(domwin, "domwindowclosed", 0);
-#endif
   }
 
   delete aInfo;
@@ -1624,12 +1563,8 @@ nsWindowWatcher::GetChromeForWindow(mozIDOMWindowProxy* aWindow,
   MutexAutoLock lock(mListLock);
   nsWatcherWindowEntry* info = FindWindowEntry(aWindow);
   if (info) {
-    if (info->mChromeWeak) {
-      return info->mChromeWeak->QueryReferent(
-        NS_GET_IID(nsIWebBrowserChrome), reinterpret_cast<void**>(aResult));
-    }
-    *aResult = info->mChrome;
-    NS_IF_ADDREF(*aResult);
+    nsCOMPtr<nsIWebBrowserChrome> chrome = do_QueryReferent(info->mChrome);
+    chrome.forget(aResult);
   }
   return NS_OK;
 }

xpfe/appshell/nsContentTreeOwner.cpp

@@ -107,6 +107,7 @@ NS_INTERFACE_MAP_BEGIN(nsContentTreeOwner)
    NS_INTERFACE_MAP_ENTRY(nsIWebBrowserChrome3)
    NS_INTERFACE_MAP_ENTRY(nsIInterfaceRequestor)
    NS_INTERFACE_MAP_ENTRY(nsIWindowProvider)
+   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
    // NOTE: This is using aggregation because there are some properties and
    // method on nsIBaseWindow (which we implement) and on
    // nsIEmbeddingSiteWindow (which we also implement) that have the same name.

xpfe/appshell/nsContentTreeOwner.h

@@ -18,6 +18,7 @@
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIWebBrowserChrome3.h"
 #include "nsIWindowProvider.h"
+#include "nsWeakReference.h"
 
 class nsXULWindow;
 class nsSiteWindow;
@@ -26,7 +27,8 @@ class nsContentTreeOwner final : public nsIDocShellTreeOwner,
                                  public nsIBaseWindow,
                                  public nsIInterfaceRequestor,
                                  public nsIWebBrowserChrome3,
-                                 public nsIWindowProvider
+                                 public nsIWindowProvider,
+                                 public nsSupportsWeakReference
 {
 friend class nsXULWindow;
 friend class nsSiteWindow;