Introduce EphemeralRange::nodes() helper to traverse over a range.

third_party/WebKit/Source/core/editing/EditingStyle.cpp

 /*
  * Copyright (C) 2007, 2008, 2009 Apple Computer, Inc.
  * Copyright (C) 2010, 2011 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.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 1382 matching lines @@
 
     Node* node = position.anchorNode();
     if (!node)
         return NaturalWritingDirection;
 
     Position end;
     if (selection.isRange()) {
         end = mostBackwardCaretPosition(selection.end());
 
         DCHECK(end.document());
-        Node* pastLast = Range::create(*end.document(), position.parentAnchoredEquivalent(), end.parentAnchoredEquivalent())->pastLastNode();

[Andrey Kraynov, 2016/08/16 10:02:11]

This is an example how we can avoid creating Range objects now.

[yosin_UTC9, 2016/08/17 01:45:31]

ACK

We should avoid to use |Position::compouteContainerNode()| instead of
|Position::parentAnchoredEquivalenent()|.
However, we've not found an example for diff of |computeContainerNode()| and
|parentAnchoredEquivalenent()| yet.

-        for (Node* n = node; n && n != pastLast; n = NodeTraversal::next(*n)) {

[Andrey Kraynov, 2016/08/16 10:02:11]

I have a question about check |n && n != pastLast|.
My version of |inRange| currently uses TraversalNextIterator that won't check
for NULL objects returned from NodeTraversal::next.
Should I create and use special CheckedNextIterator instead of
TraversalNextIterator?
Is that check really makes sence?

[yosin_UTC9, 2016/08/17 01:45:31]

Please add |DCHECK(n)| just in case. |n| should not be |nullptr| in range.
What happened when we pass empty range to |inRange()|?

[Andrey Kraynov, 2016/08/17 09:58:15]

Done.
I expect no iterations in this case.
Added a test for it.

-            if (!n->isStyledElement())
+        for (Node& n : inRange(position.parentAnchoredEquivalent(), end.parentAnchoredEquivalent())) {
+            if (!n.isStyledElement())
                 continue;
 
-            CSSComputedStyleDeclaration* style = CSSComputedStyleDeclaration::create(n);
+            CSSComputedStyleDeclaration* style = CSSComputedStyleDeclaration::create(&n);
             const CSSValue* unicodeBidi = style->getPropertyCSSValue(CSSPropertyUnicodeBidi);
             if (!unicodeBidi || !unicodeBidi->isPrimitiveValue())
                 continue;
 
             CSSValueID unicodeBidiValue = toCSSPrimitiveValue(unicodeBidi)->getValueID();
             if (isUnicodeBidiNestedOrMultipleEmbeddings(unicodeBidiValue))
                 return NaturalWritingDirection;
         }
     }
 
@@ skipping 338 matching lines @@
 {
     for (Node* ancestor = node; ancestor; ancestor = ancestor->parentNode()) {
         CSSComputedStyleDeclaration* ancestorStyle = CSSComputedStyleDeclaration::create(ancestor);
         if (!hasTransparentBackgroundColor(ancestorStyle))
             return ancestorStyle->getPropertyCSSValue(CSSPropertyBackgroundColor);
     }
     return nullptr;
 }
 
 } // namespace blink