Backout f615a0532971:154fb9a9e0bc (bug 493701) for Cpp bustage
authorNathan Froyd <froydnj@mozilla.com>
Sat, 26 Apr 2014 13:12:10 -0400
changeset 198923 73055a66d95a6580ac49d8d2cddb1b98e33b3ddc
parent 198922 0d7bf90091bb3ae78317eb34b0a46c2a32b99dcc
child 198924 8feb1c83354a2dd5ac61331deedaf1112bed7e1f
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs493701
milestone31.0a1
backs outf615a0532971b0671eea443dcd95558b3bdd3fd0
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
Backout f615a0532971:154fb9a9e0bc (bug 493701) for Cpp bustage
uriloader/base/nsDocLoader.cpp
uriloader/base/nsDocLoader.h
xpcom/glue/nsTObserverArray.h
xpcom/tests/TestObserverArray.cpp
--- a/uriloader/base/nsDocLoader.cpp
+++ b/uriloader/base/nsDocLoader.cpp
@@ -77,30 +77,34 @@ nsDocLoader::RequestInfoHashInitEntry(PL
 void
 nsDocLoader::RequestInfoHashClearEntry(PLDHashTable* table,
                                        PLDHashEntryHdr* entry)
 {
   nsRequestInfo* info = static_cast<nsRequestInfo *>(entry);
   info->~nsRequestInfo();
 }
 
-// this is used for mListenerInfoList.Contains()
-template <>
-class nsDefaultComparator <nsDocLoader::nsListenerInfo, nsIWebProgressListener*> {
-  public:
-    bool Equals(const nsDocLoader::nsListenerInfo& aInfo,
-                nsIWebProgressListener* const& aListener) const {
-      nsCOMPtr<nsIWebProgressListener> listener =
-                                       do_QueryReferent(aInfo.mWeakListener);
-      return aListener == listener;
-    }
+struct nsListenerInfo {
+  nsListenerInfo(nsIWeakReference *aListener, unsigned long aNotifyMask) 
+    : mWeakListener(aListener),
+      mNotifyMask(aNotifyMask)
+  {
+  }
+
+  // Weak pointer for the nsIWebProgressListener...
+  nsWeakPtr mWeakListener;
+
+  // Mask indicating which notifications the listener wants to receive.
+  unsigned long mNotifyMask;
 };
 
+
 nsDocLoader::nsDocLoader()
   : mParent(nullptr),
+    mListenerInfoList(8),
     mCurrentSelfProgress(0),
     mMaxSelfProgress(0),
     mCurrentTotalProgress(0),
     mMaxTotalProgress(0),
     mCompletedTotalProgress(0),
     mIsLoadingDocument(false),
     mIsRestoringDocument(false),
     mDontFlushLayout(false),
@@ -366,16 +370,25 @@ nsDocLoader::Destroy()
   if (mParent) 
   {
     mParent->RemoveChildLoader(this);
   }
 
   // Release all the information about network requests...
   ClearRequestInfoHash();
 
+  // Release all the information about registered listeners...
+  int32_t count = mListenerInfoList.Count();
+  for(int32_t i = 0; i < count; i++) {
+    nsListenerInfo *info =
+      static_cast<nsListenerInfo*>(mListenerInfoList.ElementAt(i));
+
+    delete info;
+  }
+
   mListenerInfoList.Clear();
   mListenerInfoList.Compact();
 
   mDocumentRequest = 0;
 
   if (mLoadGroup)
     mLoadGroup->SetGroupObserver(nullptr);
 
@@ -863,36 +876,54 @@ void nsDocLoader::doStopDocumentLoad(nsI
 }
 
 ////////////////////////////////////////////////////////////////////////////////////
 // The following section contains support for nsIWebProgress and related stuff
 ////////////////////////////////////////////////////////////////////////////////////
 
 NS_IMETHODIMP
 nsDocLoader::AddProgressListener(nsIWebProgressListener *aListener,
-                                 uint32_t aNotifyMask)
+                                     uint32_t aNotifyMask)
 {
-  if (mListenerInfoList.Contains(aListener)) {
+  nsresult rv;
+
+  nsListenerInfo* info = GetListenerInfo(aListener);
+  if (info) {
     // The listener is already registered!
     return NS_ERROR_FAILURE;
   }
 
   nsWeakPtr listener = do_GetWeakReference(aListener);
   if (!listener) {
     return NS_ERROR_INVALID_ARG;
   }
 
-  return mListenerInfoList.AppendElement(nsListenerInfo(listener, aNotifyMask)) ?
-         NS_OK : NS_ERROR_OUT_OF_MEMORY;
+  info = new nsListenerInfo(listener, aNotifyMask);
+  if (!info) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
+  rv = mListenerInfoList.AppendElement(info) ? NS_OK : NS_ERROR_FAILURE;
+  return rv;
 }
 
 NS_IMETHODIMP
 nsDocLoader::RemoveProgressListener(nsIWebProgressListener *aListener)
 {
-  return mListenerInfoList.RemoveElement(aListener) ? NS_OK : NS_ERROR_FAILURE;
+  nsresult rv;
+
+  nsListenerInfo* info = GetListenerInfo(aListener);
+  if (info) {
+    rv = mListenerInfoList.RemoveElement(info) ? NS_OK : NS_ERROR_FAILURE;
+    delete info;
+  } else {
+    // The listener is not registered!
+    rv = NS_ERROR_FAILURE;
+  }
+  return rv;
 }
 
 NS_IMETHODIMP
 nsDocLoader::GetDOMWindow(nsIDOMWindow **aResult)
 {
   return CallGetInterface(this, aResult);
 }
 
@@ -1144,38 +1175,16 @@ void nsDocLoader::ClearInternalProgress(
 
   mCurrentSelfProgress  = mMaxSelfProgress  = 0;
   mCurrentTotalProgress = mMaxTotalProgress = 0;
   mCompletedTotalProgress = 0;
 
   mProgressStateFlags = nsIWebProgressListener::STATE_STOP;
 }
 
-/**
- * |_code| is executed for every listener matching |_flag|
- * |listener| should be used inside |_code| as the nsIWebProgressListener var.
- */
-#define NOTIFY_LISTENERS(_flag, _code)                     \
-PR_BEGIN_MACRO                                             \
-  nsCOMPtr<nsIWebProgressListener> listener;               \
-  ListenerArray::BackwardIterator iter(mListenerInfoList); \
-  while (iter.HasMore()) {                                 \
-    nsListenerInfo &info = iter.GetNext();                 \
-    if (!(info.mNotifyMask & (_flag))) {                   \
-      continue;                                            \
-    }                                                      \
-    listener = do_QueryReferent(info.mWeakListener);       \
-    if (!listener) {                                       \
-      iter.Remove();                                       \
-      continue;                                            \
-    }                                                      \
-    _code                                                  \
-  }                                                        \
-  mListenerInfoList.Compact();                             \
-PR_END_MACRO
 
 void nsDocLoader::FireOnProgressChange(nsDocLoader *aLoadInitiator,
                                        nsIRequest *request,
                                        int64_t aProgress,
                                        int64_t aProgressMax,
                                        int64_t aProgressDelta,
                                        int64_t aTotalProgress,
                                        int64_t aMaxTotalProgress)
@@ -1192,22 +1201,48 @@ void nsDocLoader::FireOnProgressChange(n
   nsAutoCString buffer;
 
   GetURIStringFromRequest(request, buffer);
   PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, 
          ("DocLoader:%p: Progress (%s): curSelf: %d maxSelf: %d curTotal: %d maxTotal %d\n",
           this, buffer.get(), aProgress, aProgressMax, aTotalProgress, aMaxTotalProgress));
 #endif /* DEBUG */
 
-  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_PROGRESS,
+  /*
+   * First notify any listeners of the new progress info...
+   *
+   * Operate the elements from back to front so that if items get
+   * get removed from the list it won't affect our iteration
+   */
+  nsCOMPtr<nsIWebProgressListener> listener;
+  int32_t count = mListenerInfoList.Count();
+
+  while (--count >= 0) {
+    nsListenerInfo *info;
+
+    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
+    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_PROGRESS)) {
+      continue;
+    }
+
+    listener = do_QueryReferent(info->mWeakListener);
+    if (!listener) {
+      // the listener went away. gracefully pull it out of the list.
+      mListenerInfoList.RemoveElementAt(count);
+      delete info;
+      continue;
+    }
+
     // XXX truncates 64-bit to 32-bit
     listener->OnProgressChange(aLoadInitiator,request,
                                int32_t(aProgress), int32_t(aProgressMax),
                                int32_t(aTotalProgress), int32_t(aMaxTotalProgress));
-  );
+  }
+
+  mListenerInfoList.Compact();
 
   // Pass the notification up to the parent...
   if (mParent) {
     mParent->FireOnProgressChange(aLoadInitiator, request,
                                   aProgress, aProgressMax,
                                   aProgressDelta,
                                   aTotalProgress, aMaxTotalProgress);
   }
@@ -1260,92 +1295,215 @@ void nsDocLoader::DoFireOnStateChange(ns
   GetURIStringFromRequest(aRequest, buffer);
   PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, 
          ("DocLoader:%p: Status (%s): code: %x\n",
          this, buffer.get(), aStateFlags));
 #endif /* DEBUG */
 
   NS_ASSERTION(aRequest, "Firing OnStateChange(...) notification with a NULL request!");
 
-  NOTIFY_LISTENERS(((aStateFlags >> 16) & nsIWebProgress::NOTIFY_STATE_ALL),
+  /*                                                                           
+   * First notify any listeners of the new state info...
+   *
+   * Operate the elements from back to front so that if items get
+   * get removed from the list it won't affect our iteration
+   */
+  nsCOMPtr<nsIWebProgressListener> listener;
+  int32_t count = mListenerInfoList.Count();
+  int32_t notifyMask = (aStateFlags >> 16) & nsIWebProgress::NOTIFY_STATE_ALL;
+
+  while (--count >= 0) {
+    nsListenerInfo *info;
+
+    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
+    if (!info || !(info->mNotifyMask & notifyMask)) {
+      continue;
+    }
+
+    listener = do_QueryReferent(info->mWeakListener);
+    if (!listener) {
+      // the listener went away. gracefully pull it out of the list.
+      mListenerInfoList.RemoveElementAt(count);
+      delete info;
+      continue;
+    }
+
     listener->OnStateChange(aProgress, aRequest, aStateFlags, aStatus);
-  );
+  }
+
+  mListenerInfoList.Compact();
 }
 
 
 
 void
 nsDocLoader::FireOnLocationChange(nsIWebProgress* aWebProgress,
                                   nsIRequest* aRequest,
                                   nsIURI *aUri,
                                   uint32_t aFlags)
 {
-  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_LOCATION,
+  /*                                                                           
+   * First notify any listeners of the new state info...
+   *
+   * Operate the elements from back to front so that if items get
+   * get removed from the list it won't affect our iteration
+   */
+  nsCOMPtr<nsIWebProgressListener> listener;
+  int32_t count = mListenerInfoList.Count();
+
+  while (--count >= 0) {
+    nsListenerInfo *info;
+
+    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
+    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_LOCATION)) {
+      continue;
+    }
+
+    listener = do_QueryReferent(info->mWeakListener);
+    if (!listener) {
+      // the listener went away. gracefully pull it out of the list.
+      mListenerInfoList.RemoveElementAt(count);
+      delete info;
+      continue;
+    }
+
     PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, ("DocLoader [%p] calling %p->OnLocationChange", this, listener.get()));
     listener->OnLocationChange(aWebProgress, aRequest, aUri, aFlags);
-  );
+  }
+
+  mListenerInfoList.Compact();
 
   // Pass the notification up to the parent...
   if (mParent) {
     mParent->FireOnLocationChange(aWebProgress, aRequest, aUri, aFlags);
   }
 }
 
 void
 nsDocLoader::FireOnStatusChange(nsIWebProgress* aWebProgress,
                                 nsIRequest* aRequest,
                                 nsresult aStatus,
                                 const char16_t* aMessage)
 {
-  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_STATUS,
+  /*                                                                           
+   * First notify any listeners of the new state info...
+   *
+   * Operate the elements from back to front so that if items get
+   * get removed from the list it won't affect our iteration
+   */
+  nsCOMPtr<nsIWebProgressListener> listener;
+  int32_t count = mListenerInfoList.Count();
+
+  while (--count >= 0) {
+    nsListenerInfo *info;
+
+    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
+    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_STATUS)) {
+      continue;
+    }
+
+    listener = do_QueryReferent(info->mWeakListener);
+    if (!listener) {
+      // the listener went away. gracefully pull it out of the list.
+      mListenerInfoList.RemoveElementAt(count);
+      delete info;
+      continue;
+    }
+
     listener->OnStatusChange(aWebProgress, aRequest, aStatus, aMessage);
-  );
+  }
+  mListenerInfoList.Compact();
   
   // Pass the notification up to the parent...
   if (mParent) {
     mParent->FireOnStatusChange(aWebProgress, aRequest, aStatus, aMessage);
   }
 }
 
 bool
 nsDocLoader::RefreshAttempted(nsIWebProgress* aWebProgress,
                               nsIURI *aURI,
                               int32_t aDelay,
                               bool aSameURI)
 {
   /*
    * Returns true if the refresh may proceed,
    * false if the refresh should be blocked.
+   *
+   * First notify any listeners of the refresh attempt...
+   *
+   * Iterate the elements from back to front so that if items
+   * get removed from the list it won't affect our iteration
    */
   bool allowRefresh = true;
+  int32_t count = mListenerInfoList.Count();
 
-  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_REFRESH,
+  while (--count >= 0) {
+    nsListenerInfo *info;
+
+    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
+    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_REFRESH)) {
+      continue;
+    }
+
+    nsCOMPtr<nsIWebProgressListener> listener =
+      do_QueryReferent(info->mWeakListener);
+    if (!listener) {
+      // the listener went away. gracefully pull it out of the list.
+      mListenerInfoList.RemoveElementAt(count);
+      delete info;
+      continue;
+    }
+
     nsCOMPtr<nsIWebProgressListener2> listener2 =
-      do_QueryReferent(info.mWeakListener);
+      do_QueryReferent(info->mWeakListener);
     if (!listener2)
       continue;
 
     bool listenerAllowedRefresh;
     nsresult listenerRV = listener2->OnRefreshAttempted(
         aWebProgress, aURI, aDelay, aSameURI, &listenerAllowedRefresh);
     if (NS_FAILED(listenerRV))
       continue;
 
     allowRefresh = allowRefresh && listenerAllowedRefresh;
-  );
+  }
+
+  mListenerInfoList.Compact();
 
   // Pass the notification up to the parent...
   if (mParent) {
     allowRefresh = allowRefresh &&
       mParent->RefreshAttempted(aWebProgress, aURI, aDelay, aSameURI);
   }
 
   return allowRefresh;
 }
 
+nsListenerInfo * 
+nsDocLoader::GetListenerInfo(nsIWebProgressListener *aListener)
+{
+  int32_t i, count;
+  nsListenerInfo *info;
+
+  nsCOMPtr<nsISupports> listener1 = do_QueryInterface(aListener);
+  count = mListenerInfoList.Count();
+  for (i=0; i<count; i++) {
+    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(i));
+
+    NS_ASSERTION(info, "There should NEVER be a null listener in the list");
+    if (info) {
+      nsCOMPtr<nsISupports> listener2 = do_QueryReferent(info->mWeakListener);
+      if (listener1 == listener2)
+        return info;
+    }
+  }
+  return nullptr;
+}
+
 nsresult nsDocLoader::AddRequestInfo(nsIRequest *aRequest)
 {
   if (!PL_DHashTableOperate(&mRequestInfoHash, aRequest, PL_DHASH_ADD)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   return NS_OK;
 }
@@ -1460,19 +1618,45 @@ NS_IMETHODIMP nsDocLoader::OnSecurityCha
 {
   //
   // Fire progress notifications out to any registered nsIWebProgressListeners.  
   //
   
   nsCOMPtr<nsIRequest> request = do_QueryInterface(aContext);
   nsIWebProgress* webProgress = static_cast<nsIWebProgress*>(this);
 
-  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_SECURITY,
+  /*                                                                           
+   * First notify any listeners of the new state info...
+   *
+   * Operate the elements from back to front so that if items get
+   * get removed from the list it won't affect our iteration
+   */
+  nsCOMPtr<nsIWebProgressListener> listener;
+  int32_t count = mListenerInfoList.Count();
+
+  while (--count >= 0) {
+    nsListenerInfo *info;
+
+    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
+    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_SECURITY)) {
+      continue;
+    }
+
+    listener = do_QueryReferent(info->mWeakListener);
+    if (!listener) {
+      // the listener went away. gracefully pull it out of the list.
+      mListenerInfoList.RemoveElementAt(count);
+      delete info;
+      continue;
+    }
+
     listener->OnSecurityChange(webProgress, request, aState);
-  );
+  }
+
+  mListenerInfoList.Compact();
 
   // Pass the notification up to the parent...
   if (mParent) {
     mParent->OnSecurityChange(aContext, aState);
   }
   return NS_OK;
 }
 
--- a/uriloader/base/nsDocLoader.h
+++ b/uriloader/base/nsDocLoader.h
@@ -12,30 +12,33 @@
 #include "nsIDocumentLoader.h"
 #include "nsIWebProgress.h"
 #include "nsIWebProgressListener.h"
 #include "nsIRequestObserver.h"
 #include "nsWeakReference.h"
 #include "nsILoadGroup.h"
 #include "nsCOMArray.h"
 #include "nsTObserverArray.h"
+#include "nsVoidArray.h"
 #include "nsString.h"
 #include "nsIChannel.h"
 #include "nsIProgressEventSink.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIChannelEventSink.h"
 #include "nsISecurityEventSink.h"
 #include "nsISupportsPriority.h"
 #include "nsCOMPtr.h"
 #include "pldhash.h"
 #include "nsAutoPtr.h"
 
 #include "mozilla/LinkedList.h"
 
+struct nsListenerInfo;
+
 /****************************************************************************
  * nsDocLoader implementation...
  ****************************************************************************/
 
 #define NS_THIS_DOCLOADER_IMPL_CID                    \
  { /* b4ec8387-98aa-4c08-93b6-6d23069c06f2 */         \
      0xb4ec8387,                                      \
      0x98aa,                                          \
@@ -90,30 +93,16 @@ public:
     // Remove aChild from our childlist.  This nulls out the child's mParent
     // pointer.
     nsresult RemoveChildLoader(nsDocLoader *aChild);
     // Add aChild to our child list.  This will set aChild's mParent pointer to
     // |this|.
     nsresult AddChildLoader(nsDocLoader* aChild);
     nsDocLoader* GetParent() const { return mParent; }
 
-    struct nsListenerInfo {
-      nsListenerInfo(nsIWeakReference *aListener, unsigned long aNotifyMask) 
-        : mWeakListener(aListener),
-          mNotifyMask(aNotifyMask)
-      {
-      }
-
-      // Weak pointer for the nsIWebProgressListener...
-      nsWeakPtr mWeakListener;
-
-      // Mask indicating which notifications the listener wants to receive.
-      unsigned long mNotifyMask;
-    };
-
 protected:
     virtual ~nsDocLoader();
 
     virtual nsresult SetDocLoaderParent(nsDocLoader * aLoader);
 
     bool IsBusy();
 
     void Destroy();
@@ -255,18 +244,17 @@ protected:
     // for owning pointers and raw COM interface pointers for weak
     // (ie, non owning) references. If you add any members to this
     // class, please make the ownership explicit (pinkerton, scc).
   
     nsCOMPtr<nsIRequest>       mDocumentRequest;       // [OWNER] ???compare with document
 
     nsDocLoader*               mParent;                // [WEAK]
 
-    typedef nsAutoTObserverArray<nsListenerInfo, 8> ListenerArray;
-    ListenerArray              mListenerInfoList;
+    nsVoidArray                mListenerInfoList;
 
     nsCOMPtr<nsILoadGroup>        mLoadGroup;
     // We hold weak refs to all our kids
     nsTObserverArray<nsDocLoader*> mChildList;
 
     // The following member variables are related to the new nsIWebProgress 
     // feedback interfaces that travis cooked up.
     int32_t mProgressStateFlags;
@@ -311,16 +299,18 @@ private:
     
     // DocLoaderIsEmpty should be called whenever the docloader may be empty.
     // This method is idempotent and does nothing if the docloader is not in
     // fact empty.  This method _does_ make sure that layout is flushed if our
     // loadgroup has no active requests before checking for "real" emptiness if
     // aFlushLayout is true.
     void DocLoaderIsEmpty(bool aFlushLayout);
 
+    nsListenerInfo *GetListenerInfo(nsIWebProgressListener* aListener);
+
     int64_t GetMaxTotalProgress();
 
     nsresult AddRequestInfo(nsIRequest* aRequest);
     void RemoveRequestInfo(nsIRequest* aRequest);
     nsRequestInfo *GetRequestInfo(nsIRequest* aRequest);
     void ClearRequestInfoHash();
     int64_t CalculateMaxProgress();
     static PLDHashOperator CalcMaxProgressCallback(PLDHashTable* table,
--- a/xpcom/glue/nsTObserverArray.h
+++ b/xpcom/glue/nsTObserverArray.h
@@ -237,21 +237,16 @@ class nsAutoTObserverArray : protected n
     // Removes all observers and collapses all iterators to the beginning of
     // the array. The result is that forward iterators will see all elements
     // in the array.
     void Clear() {
       mArray.Clear();
       ClearIterators();
     }
 
-    // Compact the array to minimize the memory it uses
-    void Compact() {
-      mArray.Compact();
-    }
-
     // Returns the number of bytes on the heap taken up by this object, not
     // including sizeof(*this).
     size_t SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
       return mArray.SizeOfExcludingThis(mallocSizeOf);
     }
 
     //
     // Iterators
@@ -349,55 +344,16 @@ class nsAutoTObserverArray : protected n
           NS_ASSERTION(HasMore(), "iterating beyond end of array");
           return base_type::mArray.ElementAt(base_type::mPosition++);
         }
 
       private:
         ForwardIterator mEnd;
     };
 
-    // Iterates the array backward from end to start. mPosition points
-    // to the element that was returned last.
-    // Elements:
-    // - prepended to the array during iteration *will* be traversed,
-    //   unless the iteration already arrived at the first element
-    // - appended during iteration *will not* be traversed
-    // - removed during iteration *will not* be traversed.
-    class BackwardIterator : protected Iterator {
-      public:
-        typedef nsAutoTObserverArray<T, N> array_type;
-        typedef Iterator                   base_type;
-
-        BackwardIterator(const array_type& aArray)
-          : Iterator(aArray.Length(), aArray) {
-        }
-
-        // Returns true if there are more elements to iterate.
-        // This must precede a call to GetNext(). If false is
-        // returned, GetNext() must not be called.
-        bool HasMore() const {
-          return base_type::mPosition > 0;
-        }
-
-        // Returns the next element and steps one step. This must
-        // be preceded by a call to HasMore().
-        // @return The next observer.
-        elem_type& GetNext() {
-          NS_ASSERTION(HasMore(), "iterating beyond start of array");
-          return base_type::mArray.ElementAt(--base_type::mPosition);
-        }
-
-        // Removes the element at the current iterator position.
-        // (the last element returned from |GetNext()|)
-        // This will not affect the next call to |GetNext()|
-        void Remove() {
-          return base_type::mArray.RemoveElementAt(base_type::mPosition);
-        }
-    };
-
   protected:
     nsAutoTArray<T, N> mArray;
 };
 
 template<class T>
 class nsTObserverArray : public nsAutoTObserverArray<T, 0> {
   public:
     typedef nsAutoTObserverArray<T, 0>       base_type;
--- a/xpcom/tests/TestObserverArray.cpp
+++ b/xpcom/tests/TestObserverArray.cpp
@@ -126,58 +126,21 @@ int main(int argc, char **argv)
   // Prepends
   arr.PrependElementUnlessExists(3);
   static int test19Expected[] = { 3, 4, 7, 2, 8 };
   DO_TEST(ForwardIterator, test19Expected, { /* nothing */ });
 
   arr.PrependElementUnlessExists(7);
   DO_TEST(ForwardIterator, test19Expected, { /* nothing */ });
 
-  DO_TEST(ForwardIterator, test19Expected,
+  // Commented out because it fails; bug 474369 will fix
+  /*  DO_TEST(ForwardIterator, test19Expected,
           if (count == 1) {
             arr.PrependElementUnlessExists(9);
           }
           );
 
   static int test22Expected[] = { 9, 3, 4, 7, 2, 8 };
   DO_TEST(ForwardIterator, test22Expected, { });
-
-  // BackwardIterator
-  static int test23Expected[] = { 8, 2, 7, 4, 3, 9 };
-  DO_TEST(BackwardIterator, test23Expected, );
-
-  // Removals
-  static int test24Expected[] = { 8, 2, 7, 4, 9 };
-  DO_TEST(BackwardIterator, test24Expected,
-          if (count == 1) arr.RemoveElementAt(1);
-          );
-
-  // Appends
-  DO_TEST(BackwardIterator, test24Expected,
-          if (count == 1) arr.AppendElement(1);
-          );
-
-  static int test26Expected[] = { 1, 8, 2, 7, 4, 9 };
-  DO_TEST(BackwardIterator, test26Expected, );
-
-  // Prepends
-  static int test27Expected[] = { 1, 8, 2, 7, 4, 9, 3 };
-  DO_TEST(BackwardIterator, test27Expected,
-          if (count == 1) arr.PrependElementUnlessExists(3);
-          );
-
-  // Removal using Iterator
-  DO_TEST(BackwardIterator, test27Expected,
-          if (count == 1) iter.Remove();
-          );
-
-  static int test28Expected[] = { 1, 8, 2, 7, 4, 3 };
-  DO_TEST(BackwardIterator, test28Expected, );
-
-  /**
-   * Note: _code is executed before the call to GetNext(), it can therefore not
-   * test the case of prepending when the BackwardIterator already returned the
-   * first element.
-   * In that case BackwardIterator does not traverse the newly prepended Element
-   */
-
+  */
+  
   return rv;
 }