Merge 96632 - Source/WebCore: Hold refptr to identified previous sibling within findPlaceForCounter.

Source/WebCore/rendering/RenderCounter.cpp

 /**
  * Copyright (C) 2004 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 285 matching lines @@
 static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString& identifier, bool isReset, CounterNode*& parent, CounterNode*& previousSibling)
 {
     // We cannot stop searching for counters with the same identifier before we also
     // check this renderer, because it may affect the positioning in the tree of our counter.
     RenderObject* searchEndRenderer = previousSiblingOrParent(counterOwner);
     // We check renderers in preOrder from the renderer that our counter is attached to
     // towards the begining of the document for counters with the same identifier as the one
     // we are trying to find a place for. This is the next renderer to be checked.
     RenderObject* currentRenderer = previousInPreOrder(counterOwner);
     previousSibling = 0;
+    RefPtr<CounterNode> previousSiblingProtector = 0;
+
     while (currentRenderer) {
         CounterNode* currentCounter = makeCounterNode(currentRenderer, identifier, false);
         if (searchEndRenderer == currentRenderer) {
             // We may be at the end of our search.
             if (currentCounter) {
                 // We have a suitable counter on the EndSearchRenderer.
-                if (previousSibling) { // But we already found another counter that we come after.
+                if (previousSiblingProtector) { // But we already found another counter that we come after.
                     if (currentCounter->actsAsReset()) {
                         // We found a reset counter that is on a renderer that is a sibling of ours or a parent.
                         if (isReset && areRenderersElementsSiblings(currentRenderer, counterOwner)) {
                             // We are also a reset counter and the previous reset was on a sibling renderer
                             // hence we are the next sibling of that counter if that reset is not a root or
                             // we are a root node if that reset is a root.
                             parent = currentCounter->parent();
                             previousSibling = parent ? currentCounter : 0;
                             return parent;
                         }
                         // We are not a reset node or the previous reset must be on an ancestor of our owner renderer
                         // hence we must be a child of that reset counter.
                         parent = currentCounter;
                         // In some cases renders can be reparented (ex. nodes inside a table but not in a column or row).
                         // In these cases the identified previousSibling will be invalid as its parent is different from
                         // our identified parent.
-                        if (previousSibling->parent() != currentCounter)
-                            previousSibling = 0;
+                        if (previousSiblingProtector->parent() != currentCounter)
+                            previousSiblingProtector = 0;
+
+                        previousSibling = previousSiblingProtector.get();
                         return true;
                     }
                     // CurrentCounter, the counter at the EndSearchRenderer, is not reset.
                     if (!isReset || !areRenderersElementsSiblings(currentRenderer, counterOwner)) {
                         // If the node we are placing is not reset or we have found a counter that is attached
                         // to an ancestor of the placed counter's owner renderer we know we are a sibling of that node.
-                        ASSERT(currentCounter->parent() == previousSibling->parent());
+                        ASSERT(currentCounter->parent() == previousSiblingProtector->parent());
                         parent = currentCounter->parent();
+                        previousSibling = previousSiblingProtector.get();
                         return true;
                     }
                 } else { 
                     // We are at the potential end of the search, but we had no previous sibling candidate
                     // In this case we follow pretty much the same logic as above but no ASSERTs about 
                     // previousSibling, and when we are a sibling of the end counter we must set previousSibling
                     // to currentCounter.
                     if (currentCounter->actsAsReset()) {
                         if (isReset && areRenderersElementsSiblings(currentRenderer, counterOwner)) {
                             parent = currentCounter->parent();
                             previousSibling = currentCounter;
                             return parent;
                         }
                         parent = currentCounter;
+                        previousSibling = previousSiblingProtector.get();
                         return true;
                     }
                     if (!isReset || !areRenderersElementsSiblings(currentRenderer, counterOwner)) {
                         parent = currentCounter->parent();
                         previousSibling = currentCounter;
                         return true;
                     }
-                    previousSibling = currentCounter;
+                    previousSiblingProtector = currentCounter;
                 }
             }
             // We come here if the previous sibling or parent of our owner renderer had no
             // good counter, or we are a reset node and the counter on the previous sibling
             // of our owner renderer was not a reset counter.
             // Set a new goal for the end of the search.
             searchEndRenderer = previousSiblingOrParent(currentRenderer);
         } else {
             // We are searching descendants of a previous sibling of the renderer that the
             // counter being placed is attached to.
             if (currentCounter) {
                 // We found a suitable counter.
-                if (previousSibling) {
+                if (previousSiblingProtector) {
                     // Since we had a suitable previous counter before, we should only consider this one as our 
                     // previousSibling if it is a reset counter and hence the current previousSibling is its child.
                     if (currentCounter->actsAsReset()) {
-                        previousSibling = currentCounter;
+                        previousSiblingProtector = currentCounter;
                         // We are no longer interested in previous siblings of the currentRenderer or their children
                         // as counters they may have attached cannot be the previous sibling of the counter we are placing.
                         currentRenderer = parentElement(currentRenderer)->renderer();
                         continue;
                     }
                 } else
-                    previousSibling = currentCounter;
+                    previousSiblingProtector = currentCounter;
                 currentRenderer = previousSiblingOrParent(currentRenderer);
                 continue;
             }
         }
         // This function is designed so that the same test is not done twice in an iteration, except for this one
         // which may be done twice in some cases. Rearranging the decision points though, to accommodate this 
         // performance improvement would create more code duplication than is worthwhile in my oppinion and may further
         // impede the readability of this already complex algorithm.
-        if (previousSibling)
+        if (previousSiblingProtector)
             currentRenderer = previousSiblingOrParent(currentRenderer);
         else
             currentRenderer = previousInPreOrder(currentRenderer);
     }
     return false;
 }
 
 static CounterNode* makeCounterNode(RenderObject* object, const AtomicString& identifier, bool alwaysCreateCounter)
 {
     ASSERT(object);
@@ skipping 301 matching lines @@
             fprintf(stderr, "    ");
         fprintf(stderr, "%p N:%p P:%p PS:%p NS:%p C:%p\n",
             current, current->node(), current->parent(), current->previousSibling(),
             current->nextSibling(), current->m_hasCounterNodeMap?
             counterName ? WebCore::counterMaps().get(current)->get(identifier.impl()).get() : (WebCore::CounterNode*)1 : (WebCore::CounterNode*)0);
     }
     fflush(stderr);
 }
 
 #endif // NDEBUG