Remove Selection.containsNode assertion that is not valid.

Source/core/page/DOMSelection.cpp

 /*
  * Copyright (C) 2007, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 463 matching lines @@
     unsigned nodeIndex = n->nodeIndex();
     RefPtr<Range> selectedRange = selection.selection().toNormalizedRange();
 
     ContainerNode* parentNode = n->parentNode();
     if (!parentNode)
         return false;
 
     TrackExceptionState exceptionState;
     bool nodeFullySelected = Range::compareBoundaryPoints(parentNode, nodeIndex, selectedRange->startContainer(), selectedRange->startOffset(), exceptionState) >= 0 && !exceptionState.hadException()
         && Range::compareBoundaryPoints(parentNode, nodeIndex + 1, selectedRange->endContainer(), selectedRange->endOffset(), exceptionState) <= 0 && !exceptionState.hadException();
-    ASSERT(!exceptionState.hadException());
+    if (exceptionState.hadException())

[Yuta Kitamura, 2014/03/27 01:21:12]

In what situation does this become true?

I think it's better to add an error check above (along
with lines 471-479) to avoid the DOM exception, and keep
the ASSERT as is. Or better yet we could use
ASSERT_NO_EXCEPTION.

[Mads Ager (chromium), 2014/03/27 13:52:44]

This becomes true when the nodes that are compared are in separate document
fragments of the same document (which can happen when the document is cleared as
in the test). I don't know how to check for that in any other way that
duplicating the code from compareBoundaryPoints. That does not seem desirable.
:-/

[Yuta Kitamura, 2014/03/28 02:38:28]

Ah, I tried running a debugger and I think I've got the point here.

exceptionState's message is "The two ranges are in separate
documents" (note that this is not "separate document fragments"),
and probably this can be easily avoided by writing something like:

    if (parentNode->document() != selectedRange->startContainer()->document())
        return false;

But yes, I agree with your argument about code duplication,
so I'm OK with landing your change as-is.

(The test case is, erm, so smartly written, that I realized that
 parentNode and document.open() is necessary only after debugging...)

+        return false;
     if (nodeFullySelected)
         return true;
 
     bool nodeFullyUnselected = (Range::compareBoundaryPoints(parentNode, nodeIndex, selectedRange->endContainer(), selectedRange->endOffset(), exceptionState) > 0 && !exceptionState.hadException())
         || (Range::compareBoundaryPoints(parentNode, nodeIndex + 1, selectedRange->startContainer(), selectedRange->startOffset(), exceptionState) < 0 && !exceptionState.hadException());
     ASSERT(!exceptionState.hadException());
     if (nodeFullyUnselected)
         return false;
 
     return allowPartial || n->isTextNode();
@@ skipping 59 matching lines @@
     return node->document() == m_frame->document();
 }
 
 void DOMSelection::addConsoleError(const String& message)
 {
     if (m_treeScope)
         m_treeScope->document().addConsoleMessage(JSMessageSource, ErrorMessageLevel, message);
 }
 
 } // namespace WebCore