Move ComputeTextRects before Range::BoundingBox()

third_party/WebKit/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
@@ skipping 1458 matching lines @@
     unsigned start_offset =
         node == start_container ? start_position.OffsetInContainerNode() : 0;
     unsigned end_offset = node == end_container
                               ? end_position.OffsetInContainerNode()
                               : std::numeric_limits<unsigned>::max();
     layout_text->AbsoluteRectsForRange(rects, start_offset, end_offset);
   }
   return rects;
 }
 
+// TODO(tanvir.rizvi): We should move this implementation to VisibleUnits and
+// then this function will be removed.
+static void ComputeTextQuads(Vector<FloatQuad>*, const EphemeralRange&, bool);
+
 void Range::TextQuads(Vector<FloatQuad>& quads,
                       bool use_selection_height) const {
-  Node* start_container = &start_.Container();
-  DCHECK(start_container);
-  Node* end_container = &end_.Container();
-  DCHECK(end_container);
+  ComputeTextQuads(&quads, EphemeralRange(this), use_selection_height);
+}
 
-  Node* stop_node = PastLastNode();
-  for (Node* node = FirstNode(); node != stop_node;
-       node = NodeTraversal::Next(*node)) {
-    LayoutObject* r = node->GetLayoutObject();
-    if (!r || !r->IsText())
+static void ComputeTextQuads(Vector<FloatQuad>* quads,
+                             const EphemeralRange& range,
+                             bool use_selection_height) {
+  // start_position and end_position are offsetInAnchor.
+  const Position& start_position = range.StartPosition();
+  const Position& end_position = range.EndPosition();
+  Node* start_container = start_position.ComputeContainerNode();
+  DCHECK(start_position.IsOffsetInAnchor() || start_container);

[Xiaocheng, 2017/04/20 00:21:52]

s/||/&&/

[yosin_UTC9, 2017/04/20 01:21:01]

It is better to have two DCHECK() rather than one combined DCHECK().
When we have two DCHECK's, assertion message tells us which what assumption
broken.

BTW, |computeTextRects()| doesn't have
|DCHECK(start_position.IsOffsetInAnchor())|,
we may want to add |IsOffsetInAnchor()| in |computeTextRects()| too.

Note: |OffsetInContainerNode()| implies |IsOffsetInAnchor()|

[tanvir, 2017/04/20 05:39:17]

I will create a patch to add DCHECK for computeTextRects in future.

[tanvir, 2017/04/20 05:39:17]

Done.

[tanvir, 2017/04/20 05:39:17]

Done.

+  Node* end_container = end_position.ComputeContainerNode();
+  DCHECK(end_position.IsOffsetInAnchor() || end_container);

[Xiaocheng, 2017/04/20 00:21:52]

s/||/&&/

[yosin_UTC9, 2017/04/20 01:21:01]

It is better to have two DCHECK() rather than one combined DCHECK().
When we have two DCHECK's, assertion message tells us which what assumption
broken.

+
+  for (Node& node : range.Nodes()) {
+    LayoutObject* const layout_object = node.GetLayoutObject();
+    if (!layout_object || !layout_object->IsText())
       continue;
-    LayoutText* layout_text = ToLayoutText(r);
-    unsigned start_offset = node == start_container ? start_.Offset() : 0;
+    LayoutText* layout_text = ToLayoutText(layout_object);
+    unsigned start_offset =
+        node == start_container ? start_position.OffsetInContainerNode() : 0;
     unsigned end_offset = node == end_container
-                              ? end_.Offset()
+                              ? end_position.OffsetInContainerNode()
                               : std::numeric_limits<unsigned>::max();
-    layout_text->AbsoluteQuadsForRange(quads, start_offset, end_offset,
+    layout_text->AbsoluteQuadsForRange(*quads, start_offset, end_offset,
                                        use_selection_height);
   }
 }
 
 bool AreRangesEqual(const Range* a, const Range* b) {
   if (a == b)
     return true;
   if (!a || !b)
     return false;
   return a->StartPosition() == b->StartPosition() &&
@@ skipping 315 matching lines @@
                      .Data()
               << "start offset: " << range->startOffset()
               << ", end offset: " << range->endOffset();
   } else {
     LOG(INFO) << "Cannot show tree if range is null, or if boundary points are "
                  "invalid.";
   }
 }
 
 #endif