I think I might understand what's going on with the string infinite loop I noticed on Menday. (It may or may not be a new bug, since the code may never have been tested until I tested it yesterday after fixing the bustage caused by the removal of nsLiteralChar.) This is my understanding based on reading the code. gdb isn't actually usable enough for me to check this in the debugger. But anyway, the problem can be seen if you uncomment (i.e., comment out the |#ifdef DEBUG_dbaron|) the |#define CSS_REPORT_PARSE_ERRORS| at the top of content/html/style/src/nsCSSScanner.h and then load the attached page. This should trigger the parser error in nsCSSParser.cpp, line 2354. This is a call that is basically: ReportUnexpectedToken(sc, tok, NS_LITERAL_STRING("Expected ") + nsLocalString(stopString, 1) + NS_LITERAL_STRING(" but found")) where the function is defined as: static void ReportUnexpectedToken(nsCSSScanner *sc, nsCSSToken& tok, const nsAReadableString& err) { nsAutoString error(err + NS_LITERAL_STRING(" '")); /* do other stuff */ } The first line of this function is where I see the infinite loop. I think the problem is caused by calling |operator+(nsAString&, nsAString&)| even though the first of the two arguments is really an |nsPromiseConcatenation|. This means the outermost |nsPromiseConcatenation| passed to the constructor of |nsAutoString| has its mFragmentIdentifierMask set to 1 rather than to (1<<2). Then |copy_string| is called from |nsAString::do_AppendFromReadable| which is called from the constructor of |nsAutoString| (through about 3 other functions). However, once the nsReadableFragment<PRUnichar> indicating the beginning of the nsReadingIterator<PRUnichar> is advanced to the second fragment (which is the right fragment of the innermost |nsPromiseConcatenation|), the outermost |nsPromiseConcatenation| thinks that fragment is referring to its right fragment. So when we try to advance to the third fragment (via a call to |nsReadingIterator<PRUnichar>::advance| that calls |nsReadingIterator<PRUnichar>::normalize_forward| which calls |nsPromiseConcatenation::GetReadableFragment|), the outermost promise thinks there is no next fragment and returns null and leaves the iterator's fragment pointing to the second fragment. This means that the iterator's |mPosition| is equal to |mFragment.mEnd| so |nsReadingIterator<PRUnichar>::size_forward| returns 0, but the iterator's mPosition is still not the the same as the |last| parameter to |copy_string|, so |copy_string| goes into an infinite loop with the assertion on line 77 of nsAlgorithm.h. I can see ways to fix this (at the cost of an extra virtual function call) in |operator+(nsAString&, nsAString&)| for the case when the left operand to that function is an |nsPromiseConcatenation|. I can't see a way to fix it when the right operand is an |nsPromiseConcatenation|.

I changed my mind again. I think we could fix this by adding a |virtual PRUInt32 GetFragmentIdentifierMask();| to |nsA[C]String|. It would return 0 for |nsA[C]String|, but |nsPromise[C]Concatenation| would override it and turn the result of GetFragmentIdentifierMask, and |nsPromise[C]Substring| would override it and forward the call to the string of which it's a substring. In both forms of |nsPromise[C]Concatenation::nsPromise[C]Concatenation| we would then have to ensure that |mFragmentIdentifierMask| is set to |(tmp = NS_MAX(mStrings[kLeftString]->GetFragmentIdentifierMask(), mStrings[kRightString]->GetFragmentIdentifierMask()))?tmp<<1:1|. The |nsPromise[C]Concatenation| constructor and |operator+| that take an |nsPromise[C]Concatenation| as their first argument would then not really be needed, although they would save a virtual function call and a comparison to 0.

A danger of this is that one could exceed the limit of 32 |nsPromiseConcatenation|s by concatenating in different places (perhaps recursively).

This can actually be fixed as part of the construction of an aggregate string, i.e., in this case, the concatenation. Concatenation dispatching to particular fragments is costly in the case of multifragment strings already. We need to change this to calculating the number of fragments on each leaf at construction time, and stepping down exactly the right branch. This fix will automatically fix this problem as well. The mirror of this bug is that one can't concatenate a multi-fragment string currently, and for the same reason. Moving this bug under the demesne of the other fragment work.

It sounds strange to be marking this bug as a duplicate of another, but here's the story: correct iteration requires fixing iterators, fragments, the API for |GetXXXFragment| and the routine currently named |Promises|. Fixing iteration will make this bug go away. 70083 is all about fixing iteration for just this reason. *** This bug has been marked as a duplicate of 70083 ***