Bug 1484496: Part 3 - Fix nsISimpleEnumerator implementations with broken contracts. r=froydnj

The nsISimpleEnumerator contract specifies that GetNext() returns
NS_ERROR_FAILURE when iteration is complete. Several implementations, however,
either return NS_OK and a null result, or return some other error code, when
iteration is complete.

Since my initial implementation of the JS iteration stubs rely on the
contract-defined behavior of GetNext(), these need to be fixed before it can
land.

Differential Revision: https://phabricator.services.mozilla.com/D3726

--HG--
extra : rebase_source : aab0395df52e18ccff5b0a2489a983013bf484b1
extra : histedit_source : a5644f0a88799b4463af9dd01dfec33b373b1f58

Author: kmaglione

toolkit/components/windowwatcher/nsWindowWatcher.cpp

@@ -205,8 +205,9 @@ nsWatcherWindowEnumerator::GetNext(nsISupports** aResult)
   if (mCurrentPosition) {
     CallQueryInterface(mCurrentPosition->mWindow, aResult);
     mCurrentPosition = FindNext();
+    return NS_OK;
   }
-  return NS_OK;
+  return NS_ERROR_FAILURE;
 }
 
 nsWatcherWindowEntry*

xpcom/ds/nsEnumeratorUtils.cpp

@@ -73,7 +73,7 @@ EmptyEnumeratorImpl::HasMore(bool* aResult)
 NS_IMETHODIMP
 EmptyEnumeratorImpl::GetNext(nsISupports** aResult)
 {
-  return NS_ERROR_UNEXPECTED;
+  return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
@@ -144,7 +144,7 @@ nsSingletonEnumerator::GetNext(nsISupports** aResult)
   }
 
   if (mConsumed) {
-    return NS_ERROR_UNEXPECTED;
+    return NS_ERROR_FAILURE;
   }
 
   mConsumed = true;
@@ -246,7 +246,7 @@ nsUnionEnumerator::GetNext(nsISupports** aResult)
   }
 
   if (mConsumed) {
-    return NS_ERROR_UNEXPECTED;
+    return NS_ERROR_FAILURE;
   }
 
   if (!mAtSecond) {

xpcom/ds/nsObserverList.cpp

@@ -129,8 +129,7 @@ NS_IMETHODIMP
 nsObserverEnumerator::GetNext(nsISupports** aResult)
 {
   if (mIndex == mObservers.Count()) {
-    NS_ERROR("Enumerating after HasMoreElements returned false.");
-    return NS_ERROR_UNEXPECTED;
+    return NS_ERROR_FAILURE;
   }
 
   NS_ADDREF(*aResult = mObservers[mIndex]);

xpcom/ds/nsStringEnumerator.cpp

@@ -117,6 +117,10 @@ nsStringEnumerator::HasMoreElements(bool* aResult)
 NS_IMETHODIMP
 nsStringEnumerator::GetNext(nsISupports** aResult)
 {
+  if (mIndex >= mArray->Length()) {
+    return NS_ERROR_FAILURE;
+  }
+
   if (mIsUnicode) {
     nsSupportsString* stringImpl = new nsSupportsString();
     if (!stringImpl) {

xpcom/io/nsLocalFileUnix.cpp

@@ -179,6 +179,9 @@ nsDirEnumeratorUnix::GetNext(nsISupports** aResult)
   if (NS_FAILED(rv)) {
     return rv;
   }
+  if (!file) {
+    return NS_ERROR_FAILURE;
+  }
   file.forget(aResult);
   return NS_OK;
 }

xpcom/io/nsLocalFileWin.cpp

@@ -769,6 +769,9 @@ class nsDirEnumerator final
     if (NS_FAILED(rv)) {
       return rv;
     }
+    if (!hasMore) {
+      return NS_ERROR_FAILURE;
+    }
 
     mNext.forget(aResult);
     return NS_OK;
@@ -3628,7 +3631,7 @@ nsDriveEnumerator::GetNext(nsISupports** aNext)
    * character of the current drive. */
   if (*mStartOfCurrentDrive == L'\0') {
     *aNext = nullptr;
-    return NS_OK;
+    return NS_ERROR_FAILURE;
   }
 
   nsAString::const_iterator driveEnd = mStartOfCurrentDrive;

xpfe/appshell/nsAppShellWindowEnumerator.cpp

@@ -208,7 +208,7 @@ NS_IMETHODIMP nsASDOMWindowEnumerator::GetNext(nsISupports **retval)
     if (domWindow)
       return CallQueryInterface(domWindow, retval);
   }
-  return NS_OK;
+  return NS_ERROR_FAILURE;
 }
 
 //
@@ -235,8 +235,9 @@ NS_IMETHODIMP nsASXULWindowEnumerator::GetNext(nsISupports **retval)
   if (mCurrentPosition) {
     CallQueryInterface(mCurrentPosition->mWindow, retval);
     mCurrentPosition = FindNext();
+    return NS_OK;
   }
-  return NS_OK;
+  return NS_ERROR_FAILURE;
 }
 
 //