base::OffsetAdjuster: Refactor offset limiting logic into the base::OffsetAdjuster

Previously, almost all usages of OffsetAdjuster applied a post-processing
step to clamp the resulting offsets to a "sane" range.

This refactor moves the clamping into the base function, but as an
optional parameter.

This also reduces the API surface by removing the LimitOffset class.

BUG=732582

Review-Url: https://codereview.chromium.org/2953943003
Cr-Commit-Position: refs/heads/master@{#486205}

base/strings/utf_offset_string_conversions.cc

@@ -24,19 +24,20 @@ OffsetAdjuster::Adjustment::Adjustment(size_t original_offset,
 }
 
 // static
-void OffsetAdjuster::AdjustOffsets(
-    const Adjustments& adjustments,
-    std::vector<size_t>* offsets_for_adjustment) {
-  if (!offsets_for_adjustment || adjustments.empty())
-    return;
+void OffsetAdjuster::AdjustOffsets(const Adjustments& adjustments,
+                                   std::vector<size_t>* offsets_for_adjustment,
+                                   size_t limit) {
+  DCHECK(offsets_for_adjustment);
   for (std::vector<size_t>::iterator i(offsets_for_adjustment->begin());
        i != offsets_for_adjustment->end(); ++i)
-    AdjustOffset(adjustments, &(*i));
+    AdjustOffset(adjustments, &(*i), limit);
 }
 
 // static
 void OffsetAdjuster::AdjustOffset(const Adjustments& adjustments,
-                                  size_t* offset) {
+                                  size_t* offset,
+                                  size_t limit) {
+  DCHECK(offset);
   if (*offset == string16::npos)
     return;
   int adjustment = 0;
@@ -51,6 +52,9 @@ void OffsetAdjuster::AdjustOffset(const Adjustments& adjustments,
     adjustment += static_cast<int>(i->original_length - i->output_length);
   }
   *offset -= adjustment;
+
+  if (*offset > limit)
+    *offset = string16::npos;
 }
 
 // static
@@ -236,9 +240,10 @@ string16 UTF8ToUTF16WithAdjustments(
 string16 UTF8ToUTF16AndAdjustOffsets(
     const base::StringPiece& utf8,
     std::vector<size_t>* offsets_for_adjustment) {
-  std::for_each(offsets_for_adjustment->begin(),
-                offsets_for_adjustment->end(),
-                LimitOffset<base::StringPiece>(utf8.length()));
+  for (size_t& offset : *offsets_for_adjustment) {
+    if (offset > utf8.length())
+      offset = string16::npos;
+  }
   OffsetAdjuster::Adjustments adjustments;
   string16 result = UTF8ToUTF16WithAdjustments(utf8, &adjustments);
   OffsetAdjuster::AdjustOffsets(adjustments, offsets_for_adjustment);
@@ -248,9 +253,10 @@ string16 UTF8ToUTF16AndAdjustOffsets(
 std::string UTF16ToUTF8AndAdjustOffsets(
     const base::StringPiece16& utf16,
     std::vector<size_t>* offsets_for_adjustment) {
-  std::for_each(offsets_for_adjustment->begin(),
-                offsets_for_adjustment->end(),
-                LimitOffset<base::StringPiece16>(utf16.length()));
+  for (size_t& offset : *offsets_for_adjustment) {
+    if (offset > utf16.length())
+      offset = string16::npos;
+  }
   std::string result;
   PrepareForUTF8Output(utf16.data(), utf16.length(), &result);
   OffsetAdjuster::Adjustments adjustments;

base/strings/utf_offset_string_conversions.h

@@ -34,7 +34,8 @@ class BASE_EXPORT OffsetAdjuster {
   typedef std::vector<Adjustment> Adjustments;
 
   // Adjusts all offsets in |offsets_for_adjustment| to reflect the adjustments
-  // recorded in |adjustments|.
+  // recorded in |adjustments|.  Adjusted offsets greater than |limit| will be
+  // set to string16::npos.
   //
   // Offsets represents insertion/selection points between characters: if |src|
   // is "abcd", then 0 is before 'a', 2 is between 'b' and 'c', and 4 is at the
@@ -44,12 +45,14 @@ class BASE_EXPORT OffsetAdjuster {
   // adjusted (e.g., because it points into the middle of a multibyte sequence),
   // it will be set to string16::npos.
   static void AdjustOffsets(const Adjustments& adjustments,
-                            std::vector<size_t>* offsets_for_adjustment);
+                            std::vector<size_t>* offsets_for_adjustment,
+                            size_t limit = string16::npos);
 
   // Adjusts the single |offset| to reflect the adjustments recorded in
   // |adjustments|.
   static void AdjustOffset(const Adjustments& adjustments,
-                           size_t* offset);
+                           size_t* offset,
+                           size_t limit = string16::npos);
 
   // Adjusts all offsets in |offsets_for_unadjustment| to reflect the reverse
   // of the adjustments recorded in |adjustments|.  In other words, the offsets
@@ -97,32 +100,15 @@ BASE_EXPORT string16 UTF8ToUTF16WithAdjustments(
     const base::StringPiece& utf8,
     base::OffsetAdjuster::Adjustments* adjustments);
 // As above, but instead internally examines the adjustments and applies them
-// to |offsets_for_adjustment|.  See comments by AdjustOffsets().
+// to |offsets_for_adjustment|.  Input offsets greater than the length of the
+// input string will be set to string16::npos.  See comments by AdjustOffsets().
 BASE_EXPORT string16 UTF8ToUTF16AndAdjustOffsets(
     const base::StringPiece& utf8,
     std::vector<size_t>* offsets_for_adjustment);
-
 BASE_EXPORT std::string UTF16ToUTF8AndAdjustOffsets(
     const base::StringPiece16& utf16,
     std::vector<size_t>* offsets_for_adjustment);
 
-// Limiting function callable by std::for_each which will replace any value
-// which is greater than |limit| with npos.  Typically this is called with a
-// string length to clamp offsets into the string to [0, length] (as opposed to
-// [0, length); see comments above).
-template <typename T>
-struct LimitOffset {
-  explicit LimitOffset(size_t limit)
-    : limit_(limit) {}
-
-  void operator()(size_t& offset) {
-    if (offset > limit_)
-      offset = T::npos;
-  }
-
-  size_t limit_;
-};
-
 }  // namespace base
 
 #endif  // BASE_STRINGS_UTF_OFFSET_STRING_CONVERSIONS_H_

base/strings/utf_offset_string_conversions_unittest.cc

@@ -77,13 +77,14 @@ TEST(UTFOffsetStringConversionsTest, AdjustOffset) {
 }
 
 TEST(UTFOffsetStringConversionsTest, LimitOffsets) {
+  const OffsetAdjuster::Adjustments kNoAdjustments;
   const size_t kLimit = 10;
   const size_t kItems = 20;
   std::vector<size_t> size_ts;
-  for (size_t t = 0; t < kItems; ++t)
+  for (size_t t = 0; t < kItems; ++t) {
     size_ts.push_back(t);
-  std::for_each(size_ts.begin(), size_ts.end(),
-                LimitOffset<string16>(kLimit));
+    OffsetAdjuster::AdjustOffset(kNoAdjustments, &size_ts.back(), kLimit);
+  }
   size_t unlimited_count = 0;
   for (std::vector<size_t>::iterator ti = size_ts.begin(); ti != size_ts.end();
        ++ti) {
@@ -94,10 +95,10 @@ TEST(UTFOffsetStringConversionsTest, LimitOffsets) {
 
   // Reverse the values in the vector and try again.
   size_ts.clear();
-  for (size_t t = kItems; t > 0; --t)
+  for (size_t t = kItems; t > 0; --t) {
     size_ts.push_back(t - 1);
-  std::for_each(size_ts.begin(), size_ts.end(),
-                LimitOffset<string16>(kLimit));
+    OffsetAdjuster::AdjustOffset(kNoAdjustments, &size_ts.back(), kLimit);
+  }
   unlimited_count = 0;
   for (std::vector<size_t>::iterator ti = size_ts.begin(); ti != size_ts.end();
        ++ti) {

components/url_formatter/url_formatter.cc

@@ -418,14 +418,13 @@ base::string16 FormatUrl(const GURL& url,
                          url::Parsed* new_parsed,
                          size_t* prefix_end,
                          size_t* offset_for_adjustment) {
-  std::vector<size_t> offsets;
-  if (offset_for_adjustment)
-    offsets.push_back(*offset_for_adjustment);
-  base::string16 result =
-      FormatUrlWithOffsets(url, format_types, unescape_rules, new_parsed,
-                           prefix_end, &offsets);
-  if (offset_for_adjustment)
-    *offset_for_adjustment = offsets[0];
+  base::OffsetAdjuster::Adjustments adjustments;
+  base::string16 result = FormatUrlWithAdjustments(
+      url, format_types, unescape_rules, new_parsed, prefix_end, &adjustments);
+  if (offset_for_adjustment) {
+    base::OffsetAdjuster::AdjustOffset(adjustments, offset_for_adjustment,
+                                       result.length());
+  }
   return result;
 }
 
@@ -437,16 +436,11 @@ base::string16 FormatUrlWithOffsets(
     size_t* prefix_end,
     std::vector<size_t>* offsets_for_adjustment) {
   base::OffsetAdjuster::Adjustments adjustments;
-  const base::string16& format_url_return_value =
-      FormatUrlWithAdjustments(url, format_types, unescape_rules, new_parsed,
-                               prefix_end, &adjustments);
-  base::OffsetAdjuster::AdjustOffsets(adjustments, offsets_for_adjustment);
-  if (offsets_for_adjustment) {
-    std::for_each(
-        offsets_for_adjustment->begin(), offsets_for_adjustment->end(),
-        base::LimitOffset<std::string>(format_url_return_value.length()));
-  }
-  return format_url_return_value;
+  const base::string16& result = FormatUrlWithAdjustments(
+      url, format_types, unescape_rules, new_parsed, prefix_end, &adjustments);
+  base::OffsetAdjuster::AdjustOffsets(adjustments, offsets_for_adjustment,
+                                      result.length());
+  return result;
 }
 
 base::string16 FormatUrlWithAdjustments(
@@ -456,7 +450,7 @@ base::string16 FormatUrlWithAdjustments(
     url::Parsed* new_parsed,
     size_t* prefix_end,
     base::OffsetAdjuster::Adjustments* adjustments) {
-  DCHECK(adjustments != NULL);
+  DCHECK(adjustments);
   adjustments->clear();
   url::Parsed parsed_temp;
   if (!new_parsed)