Reintroduce password support to NativeTextfieldViews

base/utf_offset_string_conversions.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
 #ifndef BASE_UTF_OFFSET_STRING_CONVERSIONS_H_
 #define BASE_UTF_OFFSET_STRING_CONVERSIONS_H_
 #pragma once
 
 #include <string>
 #include <vector>
 
 #include "base/base_export.h"
 #include "base/string16.h"
 #include "base/string_piece.h"
 
+// Returns false if s[index-1] is a high surrogate and s[index] is a low
+// surrogate, true otherwise.
+BASE_EXPORT bool IsValidUtf16Index(const string16& s, size_t index);

[xji, 2012/02/18 01:28:47]

Maybe IsValidCharacterIndex or IsValidCodePointIndex is more accurate.

[benrg, 2012/02/24 19:07:44]

Changed to IsValidCodePointIndex.

+
+// |Utf16IndexToOffset| and |Utf16OffsetToIndex| are similar to GLib's

[xji, 2012/02/18 01:28:47]

Maybe add a comment like "Converts between UTF16 index and character offset".

do we add "|" around function names?

[benrg, 2012/02/22 02:25:43]

I don't know the rules, but there are many function names surrounded with || in
the source already.

+// |g_utf8_pointer_to_offset| and |g_utf8_offset_to_pointer|, but using string16

[msw, 2012/02/22 00:33:26]

I guess you're not converting a single position (like GLib's functions) so you
can cache values in RenderTextLinux for efficiency. This seems like a bad
solution; can't you cache offset string pointers in the places that loop over
the string, or similar instead?

If not, You should consult a file/dir owner to make sure this extra complexity
is worth the efficiency gain in its single use case. I suspect these functions
would belong somewhere less general.

You should also call out the reasoning for the extra complexity here, otherwise,
others might use the functions inefficiently, defeating the purpose.

[benrg, 2012/02/22 02:25:43]

These functions use (string16&, size_t) pairs where the glib functions use
char*. Other than that, there's no difference:

  g_utf8_pointer_to_offset(s+i, s+j) <-> Utf16IndexToOffset(s, i, j)

  g_utf8_offset_to_pointer(s+i, d) <-> Utf16OffsetToIndex(s, i, d)

I was going to use string16::iterator in place of char*, but that's unsafe (in
the offset-to-index case) when the string might start with a low surrogate or
end with a high surrogate. To detect the string boundary you need a reference to
the string, and at that point it's easier (I think) to use size_t indices. This
issue theoretically exists with g_utf8_offset_to_pointer as well, but not in the
way I use it, since there's no way for an attacker to pass us bad UTF-8.

+// instead of UTF-8 C strings. They count the number of 16-bit words between the
+// indices that are not the second half of a valid surrogate pair. When the
+// string is valid UTF-16 and the indices satisfy IsValidUtf16Index (the usual
+// case), this is equal to the number of code points between the indices. When
+// the string contains no surrogate pairs, Utf16IndexToOffset(s, i, j) == i - j

[xji, 2012/02/18 01:28:47]

== j - i

[benrg, 2012/02/24 19:07:44]

Oops. Fixed in rewritten documentation.

+// and Utf16OffsetToIndex(s, i, k) == i + k.
+BASE_EXPORT ptrdiff_t Utf16IndexToOffset(const string16& s,

[msw, 2012/02/22 00:33:26]

This function isn't converting an index to an offset, it's converting the
difference between two indices into an offset. Perhaps rename this function
Utf16IndicesToOffset or something that clarifies its actual operation.

[benrg, 2012/02/22 02:25:43]

The same is true of g_utf8_pointer_to_offset. I had to choose between consistent
and accurate naming. Which is not to say that I made the right choice.
Utf16IndicesToOffset sounds reasonable, but it's still strange for the "offset"
to be a number of code points rather than a number of array elements.

[benrg, 2012/02/24 19:07:44]

I couldn't think of better names for the functions, but I renamed the
parameters. I think it helps.

+                                         size_t from,
+                                         size_t to);
+BASE_EXPORT size_t Utf16OffsetToIndex(const string16& s,

[msw, 2012/02/22 00:33:26]

Similarly, this function isn't just converting an offset to an index, it's
converting an offset from a position to an index... It's more complex than the
function name conveys.

+                                      size_t pos,
+                                      ptrdiff_t offset);
+
 // Like the conversions in utf_string_conversions.h, but also takes one or more
 // offsets (|offset[s]_for_adjustment|) into the source strings, each offset
 // will be adjusted to point at the same logical place in the result strings.
 // If this isn't possible because an offset points past the end of the source
 // strings or into the middle of a multibyte sequence, the offending offset will
 // be set to string16::npos. |offset[s]_for_adjustment| may be NULL.
 BASE_EXPORT bool UTF8ToUTF16AndAdjustOffset(const char* src,
                                             size_t src_len,
                                             string16* output,
                                             size_t* offset_for_adjustment);
@@ skipping 55 matching lines @@
   void Add(const Adjustment& adjustment);
 
  private:
   void AdjustOffset(std::vector<size_t>::iterator offset);
 
   std::vector<size_t>* offsets_for_adjustment_;
   std::vector<Adjustment> adjustments_;
 };
 
 #endif  // BASE_UTF_OFFSET_STRING_CONVERSIONS_H_