Bug 319024 - Password manager does not remember data - regression from bug 316414 part 1 - observer enumerators need to hand out the strong-ref nsIObserver-implementing object, not the nsIWeakReference-implementing object, r=darin

xpcom/ds/nsArrayEnumerator.cpp

@@ -112,8 +112,7 @@ public:
     }
 
     // specialized operator to make sure we make room for mValues
-    void* operator new (size_t size, const nsCOMArray_base& aArray,
-                        PRBool aReverse) CPP_THROW_NEW;
+    void* operator new (size_t size, const nsCOMArray_base& aArray) CPP_THROW_NEW;
     void operator delete(void* ptr) {
         ::operator delete(ptr);
     }
@@ -173,8 +172,7 @@ nsCOMArrayEnumerator::GetNext(nsISupports** aResult)
 }
 
 void*
-nsCOMArrayEnumerator::operator new (size_t size, const nsCOMArray_base& aArray,
-                                    PRBool aReverse)
+nsCOMArrayEnumerator::operator new (size_t size, const nsCOMArray_base& aArray)
     CPP_THROW_NEW
 {
     // create enough space such that mValueArray points to a large
@@ -186,20 +184,15 @@ nsCOMArrayEnumerator::operator new (size_t size, const nsCOMArray_base& aArray,
     nsCOMArrayEnumerator * result =
         NS_STATIC_CAST(nsCOMArrayEnumerator*, ::operator new(size));
 
-    if (!result)
-        return result;
-
     // now need to copy over the values, and addref each one
     // now this might seem like a lot of work, but we're actually just
     // doing all our AddRef's ahead of time since GetNext() doesn't
     // need to AddRef() on the way out
+    PRUint32 i;
     PRUint32 max = result->mArraySize = aArray.Count();
-    PRUint32 cur = aReverse ? max - 1 : 0;
-    PRUint32 incr = aReverse ? -1 : 1;
-
-    for (PRUint32 i = 0; i < max; ++i, cur += incr) {
-        result->mValueArray[cur] = aArray[i];
-        NS_IF_ADDREF(result->mValueArray[cur]);
+    for (i = 0; i<max; i++) {
+        result->mValueArray[i] = aArray[i];
+        NS_IF_ADDREF(result->mValueArray[i]);
     }
 
     return result;
@@ -207,11 +200,9 @@ nsCOMArrayEnumerator::operator new (size_t size, const nsCOMArray_base& aArray,
 
 extern NS_COM nsresult
 NS_NewArrayEnumerator(nsISimpleEnumerator* *aResult,
-                      const nsCOMArray_base& aArray,
-                      PRBool aReverse)
+                      const nsCOMArray_base& aArray)
 {
-    nsCOMArrayEnumerator *enumerator =
-        new (aArray, aReverse) nsCOMArrayEnumerator();
+    nsCOMArrayEnumerator *enumerator = new (aArray) nsCOMArrayEnumerator();
     if (!enumerator) return NS_ERROR_OUT_OF_MEMORY;
 
     NS_ADDREF(*aResult = enumerator);

xpcom/ds/nsArrayEnumerator.h

@@ -82,7 +82,6 @@ NS_NewArrayEnumerator(nsISimpleEnumerator* *result,
 // without its objects going away.
 extern NS_COM nsresult
 NS_NewArrayEnumerator(nsISimpleEnumerator* *aResult,
-                      const nsCOMArray_base& aArray,
-                      PRBool aReverse = PR_FALSE);
+                      const nsCOMArray_base& aArray);
 
 #endif

xpcom/ds/nsObserverList.cpp

@@ -39,10 +39,11 @@
 
 #include "pratom.h"
 #include "nsAutoLock.h"
-#include "nsIObserver.h"
+#include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
+#include "nsIObserver.h"
+#include "nsISimpleEnumerator.h"
 #include "nsIWeakReference.h"
-#include "nsArrayEnumerator.h"
 
 nsObserverList::nsObserverList(nsresult &rv)
 {
@@ -115,10 +116,69 @@ nsObserverList::RemoveObserver(nsIObserver* anObserver)
     return NS_OK;
 }
 
+class nsObserverEnumerator : public nsISimpleEnumerator
+{
+public:
+    NS_DECL_ISUPPORTS
+    NS_DECL_NSISIMPLEENUMERATOR
+
+    nsObserverEnumerator(nsCOMArray<nsISupports> &aObservers);
+
+private:
+    ~nsObserverEnumerator() { }
+
+    PRUint32 mIndex; // Counts down, ends at 0
+    nsCOMArray<nsISupports> mObservers;
+};
+
 nsresult
 nsObserverList::GetObserverList(nsISimpleEnumerator** anEnumerator)
 {
     nsAutoLock lock(mLock);
 
-    return NS_NewArrayEnumerator(anEnumerator, mObservers, PR_TRUE);
+    nsRefPtr<nsObserverEnumerator> e(new nsObserverEnumerator(mObservers));
+    if (!e)
+        return NS_ERROR_OUT_OF_MEMORY;
+
+    NS_ADDREF(*anEnumerator = e);
+    return NS_OK;
+}
+
+nsObserverEnumerator::nsObserverEnumerator(nsCOMArray<nsISupports> &aObservers)
+{
+    for (PRInt32 i = aObservers.Count() - 1; i >= 0; --i) {
+        nsCOMPtr<nsIWeakReference> weak(do_QueryInterface(aObservers[i]));
+        if (weak) {
+            nsCOMPtr<nsISupports> strong(do_QueryReferent(weak));
+            if (strong)
+                mObservers.AppendObject(strong);
+        }
+        else {
+            mObservers.AppendObject(aObservers[i]);
+        }
+    }
+
+    mIndex = mObservers.Count();
+}
+
+NS_IMPL_ISUPPORTS1(nsObserverEnumerator, nsISimpleEnumerator)
+
+NS_IMETHODIMP
+nsObserverEnumerator::HasMoreElements(PRBool *aResult)
+{
+    *aResult = (mIndex > 0);
+    return NS_OK;
+}
+
+NS_IMETHODIMP
+nsObserverEnumerator::GetNext(nsISupports* *aResult)
+{
+    if (!mIndex) {
+        NS_ERROR("Enumerating after HasMoreElements returned false.");
+        return NS_ERROR_UNEXPECTED;
+    }
+
+    --mIndex;
+    NS_ADDREF(*aResult = mObservers[mIndex]);
+    return NS_OK;
 }

xpcom/ds/nsObserverService.cpp

@@ -238,18 +238,6 @@ NS_IMETHODIMP nsObserverService::NotifyObservers(nsISupports *aSubject,
             nsCOMPtr<nsIObserver> observer = do_QueryInterface(observerRef);
             if (observer) 
                 observer->Observe(aSubject, aTopic, someData);
-            else
-            {  // check for weak reference.
-                nsCOMPtr<nsIWeakReference> weakRef = do_QueryInterface(observerRef);     
-                if (weakRef)                                                              
-                    weakRef->QueryReferent(NS_GET_IID(nsIObserver), getter_AddRefs(observer));
-
-                if (observer) 
-                    observer->Observe(aSubject, aTopic, someData);
-
-                PR_LOG(observerServiceLog, PR_LOG_DEBUG, ("Notification - %s\n", aTopic ? aTopic : "undefined"));
-
-            }
         }
     } while (observers);
     return NS_OK;