82803 - nsSHEnumerator::HasMoreElements() needs to check mIndex = -1

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect, P3)

Description

[David Epstein]

1. Ran mfcEmbed w/ qa tests
2. Ran tests for nsISHistory iface. Incl SH index tests which start at index=0.
3. Special test for SimpleEnumerator.
4. Execute this Code segment:

nsCOMPtr<nsISimpleEnumerator> theSimpleEnum;
rv = theSessionHistory->GetSHistoryEnumerator(getter_AddRefs(theSimpleEnum));

PRBool bMore = PR_FALSE;
nsCOMPtr<nsISupports> nextObj;
nsCOMPtr<nsIHistoryEntry> nextHistoryEntry;
while (NS_SUCCEEDED(theSimpleEnum->HasMoreElements(&bMore)) && bMore)
{
   theSimpleEnum->GetNext(getter_AddRefs(nextObj));
   if (!nextObj)
      continue;
   nextHistoryEntry = do_QueryInterface(nextObj);
   if (!nextHistoryEntry)
      continue;
   rv = nextHistoryEntry->GetURI(getter_AddRefs(theUri));
   rv = theUri->GetSpec(&uriSpec);
   // ...		 
} 

Result: It doesn't execute this while loop because HasMoreElements() initializes 
the mIndex to -1 but only checks between 0 and a positive counter. Also, if I 
replace the while() with a do/while() loop, it will execute, but the last url is 
repeated twice and we get an assert.

Expected: Check for mIndex = -1 in nsSHEnumerator::HasMoreElements(), in the 
"if" condition. Then the code segment will execute. See above url (line 804).

Comment 1

[David Epstein]

changed qa contact to depstein

Comment 2

[Radha on family leave (not reading bugmail)]

Attachment: patch to nsSHistory.cpp

Comment 3

[Radha on family leave (not reading bugmail)]

The attached patch to nsSHistory.cpp includes patch to bug 84556. Only the last
part of the diffs to line 804, belongs to this bug. The rest belong to bug 84556

Comment 4

[Judson Valeski]

r=valeski on the mIndex line change.

Comment 5

[rpotts (gone)]

sr=rpotts

Comment 6

[Radha on family leave (not reading bugmail)]

fix checked in.

Comment 7

[David Epstein]

nsSHEnumerator::HasMoreElements()is now working correctly. tested in testEmbed,
Tests.cpp, CTests::SimpleEnumTest(), using 7/9 build.