ShadowRoot's getSelection().getRangeAt(0) returns an incorrect Range object

Source/core/editing/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 351 matching lines @@
         return nullptr;
 
     if (index < 0 || index >= rangeCount()) {
         exceptionState.throwDOMException(IndexSizeError, String::number(index) + " is not a valid index.");
         return nullptr;
     }
 
     // If you're hitting this, you've added broken multi-range selection support
     ASSERT(rangeCount() == 1);
 
-    if (Node* shadowAncestor = selectionShadowAncestor(m_frame)) {
-        ASSERT(!shadowAncestor->isShadowRoot());
-        ContainerNode* container = shadowAncestor->parentOrShadowHostNode();
-        int offset = shadowAncestor->nodeIndex();
-        return Range::create(shadowAncestor->document(), container, offset, container, offset);
-    }
+    Position anchor = anchorPosition(visibleSelection());
+    if (!anchor.anchorNode()->isInShadowTree())
+        return m_frame->selection().firstRange();
 
-    return m_frame->selection().firstRange();
+    if (!visibleSelection().isBaseFirst())
+        return Range::create(*anchor.document(), focusNode(), focusOffset(), shadowAdjustedNode(anchor), anchorOffset());

[Yuta Kitamura, 2015/01/07 07:50:57]

Does anchorOffset() work for shadowAdjustedNode(anchor)?

My understanding is that shadowAdjustedNode() may return
a Node in a different place, thus anchorOffset() may not
work if that happens.

[kojii, 2015/01/07 08:20:43]

Yes, this |anchorOffset()| is |DOMSelection::anchorOffset()|
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

which gets offset from |shadowAdjustedNode()|.

In other words, this code actually calculates adjustedNode
twice -- once for node and once for offset within |anchorOffset()|,
which I tried to avoid in Patch Set 2, but Yosi preferred
to calculate twice to reduce the number of lines in this patch.

[Yuta Kitamura, 2015/01/07 08:47:54]

Okay, then the PS5 looks fine to me.

I don't have a strong preference on which of PS2 and PS5
is better (I somewhat agree that the PS2 has too much code
churn, while the correctness of this line in PS5 is not very
obvious). 

+    return Range::create(*anchor.document(), shadowAdjustedNode(anchor), anchorOffset(), focusNode(), focusOffset());
 }
 
 void DOMSelection::removeAllRanges()
 {
     if (!m_frame)
         return;
     m_frame->selection().clear();
 }
 
 void DOMSelection::addRange(Range* newRange)
@@ skipping 169 matching lines @@
         m_treeScope->document().addConsoleMessage(ConsoleMessage::create(JSMessageSource, ErrorMessageLevel, message));
 }
 
 void DOMSelection::trace(Visitor* visitor)
 {
     visitor->trace(m_treeScope);
     DOMWindowProperty::trace(visitor);
 }
 
 } // namespace blink