Make SurroundingText work for a Range.

Source/core/editing/SurroundingText.cpp

 /*
  * 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 21 matching lines @@
 #include "core/editing/SurroundingText.h"
 
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
 #include "core/dom/Position.h"
 #include "core/dom/Range.h"
 #include "core/editing/TextIterator.h"
 
 namespace WebCore {
 
+SurroundingText::SurroundingText(const Range& range, unsigned maxLength)
+    : m_startOffsetInContent(0)
+    , m_endOffsetInContent(0)
+{
+    process(range.startPosition(), range.endPosition(), maxLength);
+}
+
 SurroundingText::SurroundingText(const Position& position, unsigned maxLength)
-    : m_positionOffsetInContent(0)
+    : m_startOffsetInContent(0)
+    , m_endOffsetInContent(0)
 {
+    process(position, position, maxLength);
+}
+
+void SurroundingText::process(const Position& startPosition, const Position& endPosition, unsigned maxLength)

[Yuta Kitamura, 2014/06/11 09:22:57]

The function name "process" is pretty much clueless;
I'd name this as "initialize".

+{
+    ASSERT(startPosition.document() == endPosition.document());
+
     const unsigned halfMaxLength = maxLength / 2;
 
-    Document* document = position.document();
-    // The |position| will have no document if it is null (as in no position).
+    Document* document = startPosition.document();
+    // The position will have no document if it is null (as in no position).
     if (!document)
         return;
 
     // The forward range starts at the selection end and ends at the document's
     // end. It will then be updated to only contain the text in the text in the
     // right range around the selection.
-    RefPtrWillBeRawPtr<Range> forwardRange = Range::create(*document, position, lastPositionInNode(document->documentElement()).parentAnchoredEquivalent());
+    RefPtrWillBeRawPtr<Range> forwardRange = Range::create(*document, endPosition, lastPositionInNode(document->documentElement()).parentAnchoredEquivalent());
     CharacterIterator forwardIterator(forwardRange.get(), TextIteratorStopsOnFormControls);
+    // FIXME: why do we stop going trough the text if we were not able to select something on the right?
     if (!forwardIterator.atEnd())
         forwardIterator.advance(maxLength - halfMaxLength);
 
     forwardRange = forwardIterator.range();
-    if (!forwardRange || !Range::create(*document, position, forwardRange->startPosition())->text().length()) {
+    if (!forwardRange || !Range::create(*document, endPosition, forwardRange->startPosition())->text().length()) {
         ASSERT(forwardRange);
         return;
     }
 
     // Same as with the forward range but with the backward range. The range
     // starts at the document's start and ends at the selection start and will
     // be updated.
-    RefPtrWillBeRawPtr<Range> backwardsRange = Range::create(*document, firstPositionInNode(document->documentElement()).parentAnchoredEquivalent(), position);
+    RefPtrWillBeRawPtr<Range> backwardsRange = Range::create(*document, firstPositionInNode(document->documentElement()).parentAnchoredEquivalent(), startPosition);
     BackwardsCharacterIterator backwardsIterator(backwardsRange.get(), TextIteratorStopsOnFormControls);
     if (!backwardsIterator.atEnd())
         backwardsIterator.advance(halfMaxLength);
 
     backwardsRange = backwardsIterator.range();
     if (!backwardsRange) {
         ASSERT(backwardsRange);
         return;
     }
 
-    m_positionOffsetInContent = Range::create(*document, backwardsRange->endPosition(), position)->text().length();
+    m_startOffsetInContent = Range::create(*document, backwardsRange->endPosition(), startPosition)->text().length();

[yosin_UTC9, 2014/06/11 01:04:28]

nit: Can we defer calculating start/end offset in content? It seems we don't
need to have |m_startOffsetInContent| and |m_endOffsetInContent|.
This is very optional.

[mlamouri (slow - plz ping), 2014/06/11 09:19:54]

This is as optional as m_positionOffsetInContent and we currently compute it, so
I'm not sure why we should stop doing it now?

This said, regardless of what we currently do, I would not delay calculating
m_startOffsetInContent and m_endOffsetInContent because it would require
SurroundingText to keep a reference on |startPosition| and |endPosition| such as
|backwardsRange|. It is a lot of things to keep in memory just so we can compute
to integers. In addition, all callers use startOffsetInContent and/or
endOffsetInContent and I would expect this to stay as is given that
SurroundingText will return the content() be the caller might unlikely know what
is the initial Positions in that content().

+    m_endOffsetInContent = Range::create(*document, backwardsRange->endPosition(), endPosition)->text().length();
     m_contentRange = Range::create(*document, backwardsRange->endPosition(), forwardRange->startPosition());
     ASSERT(m_contentRange);
 }
 
 PassRefPtrWillBeRawPtr<Range> SurroundingText::rangeFromContentOffsets(unsigned startOffsetInContent, unsigned endOffsetInContent)
 {
     if (startOffsetInContent >= endOffsetInContent || endOffsetInContent > content().length())
         return nullptr;
 
     CharacterIterator iterator(m_contentRange.get());
@@ skipping 14 matching lines @@
     return Range::create(*start.document(), start, end);
 }
 
 String SurroundingText::content() const
 {
     if (m_contentRange)
         return m_contentRange->text();
     return String();
 }
 
-unsigned SurroundingText::positionOffsetInContent() const
+unsigned SurroundingText::startOffsetInContent() const
 {
-    return m_positionOffsetInContent;
+    return m_startOffsetInContent;
+}
+
+unsigned SurroundingText::endOffsetInContent() const
+{
+    return m_endOffsetInContent;
 }
 
 } // namespace WebCore