Fix a 'type-limits' warning in StringSlicer::FindValidBoundaryAfter

ui/gfx/text_elider.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // This file implements utility functions for eliding and formatting UI text.
 //
 // Note that several of the functions declared in text_elider.h are implemented
 // in this file using helper classes in an unnamed namespace.
 
 #include "ui/gfx/text_elider.h"
 
 #include <string>
 #include <vector>
 
 #include "base/files/file_path.h"
 #include "base/i18n/break_iterator.h"
 #include "base/i18n/char_iterator.h"
 #include "base/i18n/rtl.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "third_party/icu/source/common/unicode/rbbi.h"
 #include "third_party/icu/source/common/unicode/uloc.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/gfx/render_text.h"
 #include "ui/gfx/text_utils.h"
@@ skipping 109 matching lines @@
 
 size_t StringSlicer::FindValidBoundaryBefore(size_t index) const {
   DCHECK_LE(index, text_.length());
   if (index != text_.length())
     U16_SET_CP_START(text_.data(), 0, index);
   return index;
 }
 
 size_t StringSlicer::FindValidBoundaryAfter(size_t index) const {
   DCHECK_LE(index, text_.length());
-  if (index != text_.length())
-    U16_SET_CP_LIMIT(text_.data(), 0, index, text_.length());
-  return index;
+  if (index == text_.length())
+    return index;
+
+  int32_t text_index = base::checked_cast<int32_t>(index);
+  int32_t text_length = base::checked_cast<int32_t>(text_.length());
+  U16_SET_CP_LIMIT(text_.data(), 0, text_index, text_length);

[msw, 2014/10/29 20:46:43]

Didn't Peter suggest using c_str() instead of data()?

[Ben Chan, 2014/10/29 21:08:30]

I believe .c_str() isn't necessary if the length is given.

[msw, 2014/10/29 21:15:46]

If c_str() offers additional safety without serious negative implications, let's
use that; otherwise, I it's fine to continue using data() here.

[Ben Chan, 2014/10/29 21:23:36]

The only thing I can think of may be related to performance, depending on the
implementation. It won't be an issue after we move to C++11, which requires both
.data() and .c_str() to return null-terminated buffer.

+  return static_cast<size_t>(text_index);

[msw, 2014/10/29 20:46:44]

Shouldn't this check that the 32-bit index won't overflow as a 16-bit size_t or
do a saturated conversion or something?

[Ben Chan, 2014/10/29 21:08:30]

do we support a platform with 16-bit size_t? if so, 32-bit index/length doesn't
feel right either.

[msw, 2014/10/29 21:15:46]

Heh, perhaps not, I guess this ought to be fine otherwise.

 }
 
 base::string16 ElideFilename(const base::FilePath& filename,
                              const FontList& font_list,
                              float available_pixel_width) {
 #if defined(OS_WIN)
   base::string16 filename_utf16 = filename.value();
   base::string16 extension = filename.Extension();
   base::string16 rootname = filename.BaseName().RemoveExtension().value();
 #elif defined(OS_POSIX)
@@ skipping 655 matching lines @@
     index = char_iterator.getIndex();
   } else {
     // String has leading whitespace, return the elide string.
     return kElideString;
   }
 
   return string.substr(0, index) + kElideString;
 }
 
 }  // namespace gfx