Simplify UTF16LE_Encode and add unittest.

Simplify UTF16LE_Encode and add unittest.

Previously, UTF16LE_Encode take an optional flag to indicate
if the returned byte string has trailing zeros. In fact, no where
needs the flag to be false. So just get rid of it so callers won't
misuse.

The bug is found by https://codereview.chromium.org/837723009

R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/0185408126529d5df7e095c5789affd4ae971375

[Bo Xu, 2015-01-21 00:03:07]

bo_xu@foxitsoftware.com changed reviewers:
+ tsepez@chromium.org

[Bo Xu, 2015-01-21 00:03:07]

Hi Tom, please review this one.

[Tom Sepez, 2015-01-21 00:08:39]

On 2015/01/21 00:03:07, Bo Xu wrote:
> Hi Tom, please review this one.

Sounds good to me.  Now that we have a unittests framework, I'm going to ask
that you copy fx_basic_bstring_unittest.cpp over to
fx_basic_wstring_unittest.cpp before making changes to
any APIs that affect fxcrt string types.

Having done so, we can add a test for this condition.

[Tom Sepez, 2015-01-21 00:16:03]

> And by copy over, I mean you can remove the existing tests for wstring
equivalents -- we'll add them later.  Just need to have one for this function.

[Bo Xu, 2015-01-21 07:59:20]

On 2015/01/21 00:16:03, Tom Sepez wrote:
> > And by copy over, I mean you can remove the existing tests for wstring
> equivalents -- we'll add them later.  Just need to have one for this function.

Test case is added. For the last one, it failed. I am not sure if I should deal
with the endianness or it is a bug.

[Tom Sepez, 2015-01-21 18:15:55]

Outstanding.

https://codereview.chromium.org/860973002/diff/40001/core/src/fxcrt/fx_basic_...
File core/src/fxcrt/fx_basic_wstring_unittest.cpp (right):

https://codereview.chromium.org/860973002/diff/40001/core/src/fxcrt/fx_basic_...
core/src/fxcrt/fx_basic_wstring_unittest.cpp:15: L"23\0456",
beware of tailing digits following a \0, the intent is clearer when uing alphas,
which is covered above. this case doesn't add much beyond that, so we can pull
it.

Can we also get a case where the high bit is set in each byte of the result to
check for sign-extension weirdness?

https://codereview.chromium.org/860973002/diff/40001/core/src/fxcrt/fx_basic_...
core/src/fxcrt/fx_basic_wstring_unittest.cpp:16: L"\x3132\x6162"  // This is
wrong? Endianness matters here?
So, what you'd have here is a wide string:
  U+3132, U+6162
which is being formatted as ut16 little endian, which I'd
expect to see as:
0x32, 0x31, 0x62, 0x61 or 21ba.

https://codereview.chromium.org/860973002/diff/40001/core/src/fxcrt/fx_basic_...
core/src/fxcrt/fx_basic_wstring_unittest.cpp:18: CFX_ByteString byte_strings[] =
{
nit: its preferable to make a single array of structures out of these so that
they don't get out of sync with each other.  Also, feel free to use a macro to
get rid of boiler plate initializations.

[Bo Xu, 2015-01-21 19:23:55]

https://codereview.chromium.org/860973002/diff/40001/core/src/fxcrt/fx_basic_...
File core/src/fxcrt/fx_basic_wstring_unittest.cpp (right):

https://codereview.chromium.org/860973002/diff/40001/core/src/fxcrt/fx_basic_...
core/src/fxcrt/fx_basic_wstring_unittest.cpp:15: L"23\0456",
On 2015/01/21 18:15:54, Tom Sepez wrote:
> beware of tailing digits following a \0, the intent is clearer when uing
alphas,
> which is covered above. this case doesn't add much beyond that, so we can pull
> it.
> 
> Can we also get a case where the high bit is set in each byte of the result to
> check for sign-extension weirdness?

Done.

https://codereview.chromium.org/860973002/diff/40001/core/src/fxcrt/fx_basic_...
core/src/fxcrt/fx_basic_wstring_unittest.cpp:16: L"\x3132\x6162"  // This is
wrong? Endianness matters here?
On 2015/01/21 18:15:54, Tom Sepez wrote:
> So, what you'd have here is a wide string:
>   U+3132, U+6162
> which is being formatted as ut16 little endian, which I'd
> expect to see as:
> 0x32, 0x31, 0x62, 0x61 or 21ba.

Oh, right, i see.

https://codereview.chromium.org/860973002/diff/40001/core/src/fxcrt/fx_basic_...
core/src/fxcrt/fx_basic_wstring_unittest.cpp:18: CFX_ByteString byte_strings[] =
{
On 2015/01/21 18:15:54, Tom Sepez wrote:
> nit: its preferable to make a single array of structures out of these so that
> they don't get out of sync with each other.  Also, feel free to use a macro to
> get rid of boiler plate initializations.

Done.

[Tom Sepez, 2015-01-21 20:01:54]

lgtm

[Bo Xu, 2015-01-21 20:17:17]

Committed patchset #5 (id:80001) manually as
0185408126529d5df7e095c5789affd4ae971375 (presubmit successful).