Bug 1484496: Part 3 - Fix nsISimpleEnumerator implementations with broken contracts. r=froydnj
authorKris Maglione <maglione.k@gmail.com>
Sat, 18 Aug 2018 18:28:10 -0700
changeset 488329 15a738855cb06dd495c14fde3c8b54dde513c098
parent 488328 062e4138bfde6fb0f010d3fabb82b052b2a1b301
child 488330 8ca4845423179d1f0a2e929a827014b393564b76
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1484496
milestone63.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 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
toolkit/components/windowwatcher/nsWindowWatcher.cpp
xpcom/ds/nsEnumeratorUtils.cpp
xpcom/ds/nsObserverList.cpp
xpcom/ds/nsStringEnumerator.cpp
xpcom/io/nsLocalFileUnix.cpp
xpcom/io/nsLocalFileWin.cpp
xpfe/appshell/nsAppShellWindowEnumerator.cpp
--- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp
+++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp
@@ -200,18 +200,19 @@ nsWatcherWindowEnumerator::GetNext(nsISu
     return NS_ERROR_INVALID_ARG;
   }
 
   *aResult = nullptr;
 
   if (mCurrentPosition) {
     CallQueryInterface(mCurrentPosition->mWindow, aResult);
     mCurrentPosition = FindNext();
+    return NS_OK;
   }
-  return NS_OK;
+  return NS_ERROR_FAILURE;
 }
 
 nsWatcherWindowEntry*
 nsWatcherWindowEnumerator::FindNext()
 {
   nsWatcherWindowEntry* info;
 
   if (!mCurrentPosition) {
--- a/xpcom/ds/nsEnumeratorUtils.cpp
+++ b/xpcom/ds/nsEnumeratorUtils.cpp
@@ -68,17 +68,17 @@ EmptyEnumeratorImpl::HasMore(bool* aResu
 {
   *aResult = false;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EmptyEnumeratorImpl::GetNext(nsISupports** aResult)
 {
-  return NS_ERROR_UNEXPECTED;
+  return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 EmptyEnumeratorImpl::GetNext(nsACString& aResult)
 {
   return NS_ERROR_UNEXPECTED;
 }
 
@@ -139,17 +139,17 @@ NS_IMETHODIMP
 nsSingletonEnumerator::GetNext(nsISupports** aResult)
 {
   MOZ_ASSERT(aResult != 0, "null ptr");
   if (!aResult) {
     return NS_ERROR_NULL_POINTER;
   }
 
   if (mConsumed) {
-    return NS_ERROR_UNEXPECTED;
+    return NS_ERROR_FAILURE;
   }
 
   mConsumed = true;
 
   *aResult = mValue;
   NS_ADDREF(*aResult);
   return NS_OK;
 }
@@ -241,17 +241,17 @@ NS_IMETHODIMP
 nsUnionEnumerator::GetNext(nsISupports** aResult)
 {
   MOZ_ASSERT(aResult != 0, "null ptr");
   if (!aResult) {
     return NS_ERROR_NULL_POINTER;
   }
 
   if (mConsumed) {
-    return NS_ERROR_UNEXPECTED;
+    return NS_ERROR_FAILURE;
   }
 
   if (!mAtSecond) {
     return mFirstEnumerator->GetNext(aResult);
   }
 
   return mSecondEnumerator->GetNext(aResult);
 }
--- a/xpcom/ds/nsObserverList.cpp
+++ b/xpcom/ds/nsObserverList.cpp
@@ -124,16 +124,15 @@ nsObserverEnumerator::HasMoreElements(bo
   *aResult = (mIndex < mObservers.Count());
   return NS_OK;
 }
 
 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]);
   ++mIndex;
   return NS_OK;
 }
--- a/xpcom/ds/nsStringEnumerator.cpp
+++ b/xpcom/ds/nsStringEnumerator.cpp
@@ -112,16 +112,20 @@ NS_IMETHODIMP
 nsStringEnumerator::HasMoreElements(bool* aResult)
 {
   return HasMore(aResult);
 }
 
 NS_IMETHODIMP
 nsStringEnumerator::GetNext(nsISupports** aResult)
 {
+  if (mIndex >= mArray->Length()) {
+    return NS_ERROR_FAILURE;
+  }
+
   if (mIsUnicode) {
     nsSupportsString* stringImpl = new nsSupportsString();
     if (!stringImpl) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
     stringImpl->SetData(mArray->ElementAt(mIndex++));
     *aResult = stringImpl;
--- a/xpcom/io/nsLocalFileUnix.cpp
+++ b/xpcom/io/nsLocalFileUnix.cpp
@@ -174,16 +174,19 @@ nsDirEnumeratorUnix::HasMoreElements(boo
 NS_IMETHODIMP
 nsDirEnumeratorUnix::GetNext(nsISupports** aResult)
 {
   nsCOMPtr<nsIFile> file;
   nsresult rv = GetNextFile(getter_AddRefs(file));
   if (NS_FAILED(rv)) {
     return rv;
   }
+  if (!file) {
+    return NS_ERROR_FAILURE;
+  }
   file.forget(aResult);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDirEnumeratorUnix::GetNextEntry()
 {
   do {
--- a/xpcom/io/nsLocalFileWin.cpp
+++ b/xpcom/io/nsLocalFileWin.cpp
@@ -764,16 +764,19 @@ public:
   NS_IMETHOD GetNext(nsISupports** aResult) override
   {
     nsresult rv;
     bool hasMore;
     rv = HasMoreElements(&hasMore);
     if (NS_FAILED(rv)) {
       return rv;
     }
+    if (!hasMore) {
+      return NS_ERROR_FAILURE;
+    }
 
     mNext.forget(aResult);
     return NS_OK;
   }
 
   NS_IMETHOD GetNextFile(nsIFile** aResult) override
   {
     *aResult = nullptr;
@@ -3623,17 +3626,17 @@ NS_IMETHODIMP
 nsDriveEnumerator::GetNext(nsISupports** aNext)
 {
   /* GetLogicalDrives stored in mDrives is a concatenation
    * of null terminated strings, followed by a null terminator.
    * mStartOfCurrentDrive is an iterator pointing at the first
    * character of the current drive. */
   if (*mStartOfCurrentDrive == L'\0') {
     *aNext = nullptr;
-    return NS_OK;
+    return NS_ERROR_FAILURE;
   }
 
   nsAString::const_iterator driveEnd = mStartOfCurrentDrive;
   FindCharInReadable(L'\0', driveEnd, mEndOfDrivesString);
   nsString drive(Substring(mStartOfCurrentDrive, driveEnd));
   mStartOfCurrentDrive = ++driveEnd;
 
   nsIFile* file;
--- a/xpfe/appshell/nsAppShellWindowEnumerator.cpp
+++ b/xpfe/appshell/nsAppShellWindowEnumerator.cpp
@@ -203,17 +203,17 @@ NS_IMETHODIMP nsASDOMWindowEnumerator::G
   *retval = nullptr;
   while (mCurrentPosition) {
     nsCOMPtr<nsPIDOMWindowOuter> domWindow;
     nsWindowMediator::GetDOMWindow(mCurrentPosition->mWindow, domWindow);
     mCurrentPosition = FindNext();
     if (domWindow)
       return CallQueryInterface(domWindow, retval);
   }
-  return NS_OK;
+  return NS_ERROR_FAILURE;
 }
 
 //
 // nsASXULWindowEnumerator
 //
 
 nsASXULWindowEnumerator::nsASXULWindowEnumerator(
     const char16_t* aTypeString,
@@ -230,18 +230,19 @@ NS_IMETHODIMP nsASXULWindowEnumerator::G
 {
   if (!retval)
     return NS_ERROR_INVALID_ARG;
 
   *retval = nullptr;
   if (mCurrentPosition) {
     CallQueryInterface(mCurrentPosition->mWindow, retval);
     mCurrentPosition = FindNext();
+    return NS_OK;
   }
-  return NS_OK;
+  return NS_ERROR_FAILURE;
 }
 
 //
 // nsASDOMWindowEarlyToLateEnumerator
 //
 
 nsASDOMWindowEarlyToLateEnumerator::nsASDOMWindowEarlyToLateEnumerator(
     const char16_t *aTypeString,