Avoid temporary range allocation in FrameSelection::respondToNodeModification.

Source/core/editing/VisibleSelection.cpp

 /*
  * Copyright (C) 2004, 2005, 2006 Apple Computer, 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.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 174 matching lines @@
 
 PassRefPtrWillBeRawPtr<Range> VisibleSelection::firstRange() const
 {
     if (isNone())
         return nullptr;
     Position start = m_start.parentAnchoredEquivalent();
     Position end = m_end.parentAnchoredEquivalent();
     return Range::create(*start.document(), start, end);
 }
 
+bool VisibleSelection::intersectsNode(Node* node) const
+{
+    if (isNone())
+        return false;
+    Position start = m_start.parentAnchoredEquivalent();
+    Position end = m_end.parentAnchoredEquivalent();
+    TrackExceptionState exceptionState;
+    if (Range::intersectsNode(node, start, end, exceptionState))

[yosin_UTC9, 2014/09/05 01:07:56]

nit: Just |return Range::intersectsNode(node, start, end, exceptionState);|

[yosin_UTC9, 2014/09/05 01:07:56]

Can we use |ASSERT_NO_EXCEPTION| instead of |exceptionState|?
It seems exception is thrown only for null |node|.

[Mads Ager (chromium), 2014/09/05 08:22:52]

Hmm, good point. I thought we could as well, but it turns out that there is an
issue with shadow roots. compareBoundaryPoint attempts to find a common ancestor
and fails if one of the nodes is in a shadow tree and the other one is outside
of the shadow tree. It will then throw an exception (saying that the ranges are
in different documents even though they are not).

In order to make this code behave the same as before I therefore updated the
return value to be:

return Range::intersectsNode(...) && !exceptionState.hadException();

If the points cannot be compared they do not intersect (that check was in the
FrameSelection code before and we therefore match the old behavior now).

I find the behavior around shadow roots a little odd though.
compareBoundaryPoint will throw an exception that the ranges are not in the same
document if you compare a position in a shadow tree with a position outside of
the shadow tree. I wonder if most of this code should really use
parentOrShadowHostNode instead of parentNode to make sure to continue in the
parent chain once a shadow root is encountered?

This example throws an exception (Uncaught WrongDocumentError: Failed to execute
'intersectsNode' on 'Range': The two ranges are in separate documents.):

<div id="container"></div>
<script>
if (window.testRunner)
    testRunner.dumpAsText();

var shadowRoot = container.createShadowRoot();
var div = document.createElement('div');
shadowRoot.appendChild(div);
var range = new Range();
alert(range.intersectsNode(div));
</script>

Is that the intended behavior?

In any case that behavior in independent of this change. :)

[Mads Ager (chromium), 2014/09/05 08:22:52]

Yes! Thanks! :)

+        return true;
+    return false;
+}
+
 PassRefPtrWillBeRawPtr<Range> VisibleSelection::toNormalizedRange() const
 {
     if (isNone())
         return nullptr;
 
     // Make sure we have an updated layout since this function is called
     // in the course of running edit commands which modify the DOM.
     // Failing to call this can result in equivalentXXXPosition calls returning
     // incorrect results.
     m_start.document()->updateLayout();
@@ skipping 651 matching lines @@
     sel.showTreeForThis();
 }
 
 void showTree(const blink::VisibleSelection* sel)
 {
     if (sel)
         sel->showTreeForThis();
 }
 
 #endif