Fixing a crash in InsertListCommand.

Source/core/dom/Range.cpp

 /*
  * (C) 1999 Lars Knoll (knoll@kde.org)
  * (C) 2000 Gunnstein Lye (gunnstein@netcom.no)
  * (C) 2000 Frederik Holljen (frederik.holljen@hig.no)
  * (C) 2001 Peter Kelly (pmk@post.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Motorola Mobility. 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
@@ skipping 883 matching lines @@
 
         NodeVector nodes;
         for (Node* child = firstChildInAncestorToProcess.get(); child;
             child = (direction == ProcessContentsForward) ? child->nextSibling() : child->previousSibling())
             nodes.append(child);
 
         for (NodeVector::const_iterator it = nodes.begin(); it != nodes.end(); ++it) {
             Node* child = it->get();
             switch (action) {
             case DELETE_CONTENTS:
+                // ancestor->removeChild(<child before this child>) might cause the tree change
+                // because removeChild might call dispatchSubtreeModifiedEvent.
+                // Then we should validate again.

[Yuta Kitamura, 2014/02/26 10:41:16]

nit: The first sentence sounds a little bit strange
(what's "child before this child"?).

Also, in the last sentence "validate" sounds a little
bit vague under this context.

With that in mind I'd suggest something like:

    // Prior call of ancestor->removeChild() may cause a tree change due to
DOMSubtreeModified event.
    // Therefore, we need to make sure |ancestor| is still |child|'s parent.

[yoichio, 2014/02/27 04:00:03]

Done.

+                if (ancestor != child->parentNode())

[Yuta Kitamura, 2014/02/26 10:41:16]

Ideally, we could have bulk-removal functionality
to avoid this kind of mistake, but your change seems
acceptable at this moment.

You can make the code cleaner by replacing the lines
907-909 with:

    if (ancestor == child->parentNode())
        ancestor->removeChild(child, exceptionState);

[yoichio, 2014/02/27 04:00:03]

Done.

+                    break;
                 ancestor->removeChild(child, exceptionState);
                 break;
             case EXTRACT_CONTENTS: // will remove child from ancestor
                 if (direction == ProcessContentsForward)
                     clonedContainer->appendChild(child, exceptionState);
                 else
                     clonedContainer->insertBefore(child, clonedContainer->firstChild(), exceptionState);
                 break;
             case CLONE_CONTENTS:
                 if (direction == ProcessContentsForward)
@@ skipping 972 matching lines @@
 
 void showTree(const WebCore::Range* range)
 {
     if (range && range->boundaryPointsValid()) {
         range->startContainer()->showTreeAndMark(range->startContainer(), "S", range->endContainer(), "E");
         fprintf(stderr, "start offset: %d, end offset: %d\n", range->startOffset(), range->endOffset());
     }
 }
 
 #endif