Fix build issues in base/ for Android x64

Fix build issues in base/ for Android x64

This CL fixes several issues when building files under base/ for
Android x64.

BUG=346626
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=254124

[Yang Gu, 2014-02-25 09:00:41]

We will begin to send out patches to fix build issues for Android x64. We plan
to do this directory by directory, and this is the first one. 
Please have a review, thanks!

[rmcilroy, 2014-02-25 11:01:36]

https://codereview.chromium.org/179373003/diff/1/base/i18n/rtl.cc
File base/i18n/rtl.cc (right):

https://codereview.chromium.org/179373003/diff/1/base/i18n/rtl.cc#newcode268
base/i18n/rtl.cc:268: text->insert(pos, num, kLeftToRightMark);
Is this change really necessary?  This file is already being compiled on Linux
in x64 isn't it?

https://codereview.chromium.org/179373003/diff/1/base/json/json_writer.cc
File base/json/json_writer.cc (right):

https://codereview.chromium.org/179373003/diff/1/base/json/json_writer.cc#new...
base/json/json_writer.cc:99: real.insert(static_cast<size_t>(0),
static_cast<size_t>(1), '0');
ditto here.

[Yang Gu, 2014-02-26 10:28:18]

Android and Linux use different string library. If I don't change this part, I
would get following errors:

************************************************************
../../base/json/json_writer.cc: In member function 'bool
base::JSONWriter::BuildJSONString(const base::Value*, size_t)':
../../base/json/json_writer.cc:99:32: error: call of overloaded 'insert(unsigned
int, unsigned int, char)' is ambiguous
         real.insert(0U, 1U, '0');
                                ^
../../base/json/json_writer.cc:99:32: note: candidates are:
In file included from
/workspace/project/acr64/chromium/src/third_party/android_tools/ndk//sources/cxx-stl/stlport/stlport/string:29:0,
                 from ../../base/json/json_writer.h:8,
                 from ../../base/json/json_writer.cc:5:
/workspace/project/acr64/chromium/src/third_party/android_tools/ndk//sources/cxx-stl/stlport/stlport/stl/_string.h:628:10:
note: std::basic_string<_CharT, _Traits, _Alloc>::_Self&
std::basic_string<_CharT, _Traits, _Alloc>::insert(std::basic_string<_CharT,
_Traits, _Alloc>::size_type, const _CharT*, std::basic_string<_CharT, _Traits,
_Alloc>::size_type) [with _CharT = char; _Traits = std::char_traits<char>;
_Alloc = std::allocator<char>; std::basic_string<_CharT, _Traits, _Alloc>::_Self
= std::basic_string<char, std::char_traits<char>, std::allocator<char> >;
std::basic_string<_CharT, _Traits, _Alloc>::size_type = long unsigned int] <near
match>
   _Self& insert(size_type __pos, const _CharT* __s, size_type __n) {
          ^
/workspace/project/acr64/chromium/src/third_party/android_tools/ndk//sources/cxx-stl/stlport/stlport/stl/_string.h:628:10:
note:   no known conversion for argument 2 from 'unsigned int' to 'const char*'
/workspace/project/acr64/chromium/src/third_party/android_tools/ndk//sources/cxx-stl/stlport/stlport/stl/_string.h:649:10:
note: std::basic_string<_CharT, _Traits, _Alloc>::_Self&
std::basic_string<_CharT, _Traits, _Alloc>::insert(std::basic_string<_CharT,
_Traits, _Alloc>::size_type, std::basic_string<_CharT, _Traits,
_Alloc>::size_type, _CharT) [with _CharT = char; _Traits =
std::char_traits<char>; _Alloc = std::allocator<char>; std::basic_string<_CharT,
_Traits, _Alloc>::_Self = std::basic_string<char, std::char_traits<char>,
std::allocator<char> >; std::basic_string<_CharT, _Traits, _Alloc>::size_type =
long unsigned int]
   _Self& insert(size_type __pos, size_type __n, _CharT __c) {
          ^
/workspace/project/acr64/chromium/src/third_party/android_tools/ndk//sources/cxx-stl/stlport/stlport/stl/_string.h:668:8:
note: void std::basic_string<_CharT, _Traits,
_Alloc>::insert(std::basic_string<_CharT, _Traits, _Alloc>::iterator, size_t,
_CharT) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc =
std::allocator<char>; std::basic_string<_CharT, _Traits, _Alloc>::iterator =
char*; std::basic_string<_CharT, _Traits, _Alloc>::value_type = char; size_t =
long unsigned int]
   void insert(iterator __p, size_t __n, _CharT __c);
        ^
/workspace/project/acr64/chromium/src/third_party/android_tools/ndk//sources/cxx-stl/stlport/stlport/stl/_string.h:781:8:
note: void std::basic_string<_CharT, _Traits,
_Alloc>::insert(std::basic_string<_CharT, _Traits, _Alloc>::iterator, const
_CharT*, const _CharT*) [with _CharT = char; _Traits = std::char_traits<char>;
_Alloc = std::allocator<char>; std::basic_string<_CharT, _Traits,
_Alloc>::iterator = char*; std::basic_string<_CharT, _Traits,
_Alloc>::value_type = char] <near match>
   void insert(iterator __p, const _CharT* __f, const _CharT* __l) {
        ^
/workspace/project/acr64/chromium/src/third_party/android_tools/ndk//sources/cxx-stl/stlport/stlport/stl/_string.h:781:8:
note:   no known conversion for argument 3 from 'char' to 'const char*'

************************************************************

From above log, you may see the ambiguous definition at line 649 and 668, which
are extracted here.
Line 649:
  _Self& insert(size_type __pos, size_type __n, _CharT __c) {
    if (__pos > size())
      this->_M_throw_out_of_range();
    if (__n > max_size() - size())
      this->_M_throw_length_error();
    insert(begin() + __pos, __n, __c);
    return *this;
  }

Line 668:
  void insert(iterator __p, size_t __n, _CharT __c);

So is this a problem for you? For a better portable version, we may change it to
something like "static_cast<string16::size_type>(0)". Note that this kind of
problem happens here and there in current code, mostly in tests. So we ask for
your careful review on this. Thanks!

[rmcilroy, 2014-02-26 15:14:31]

OK, thanks for sending the error message.  After some investigation with Torne
we discovered why this occurs only on Android 64-bit and not on other platforms.
 For future reference, the reasons are: 
 1) Android uses stlport, unlike Linux which uses gnu-stl.
 2) stlport typedefs "std::basic_string::iterator" to a raw pointer (in this
case _CharT*), whereas gnu-stl defines "std::basic_string::iterator" as a value
type.
 3) A 0 literal can represent a pointer type (since NULL == 0)
 4) On 64-bit, size_t is an unsigned long, therefore the compiler needs to do an
implicit conversion of 0U (unsigned int) to a type which can be used in an one
of the insert function overloads.
 5) Because '0U' could be implicitly converted to either a pointer or a size_t,
the compiler gets an ambiguous overload between:
      insert(iterator __p, size_t __n, _CharT __c) and
      insert(size_type __pos, size_type __n, _CharT __c)

We should really consider fixing stlport to define iterator as a value type
rather than a raw pointer which would address this (I will start a separate
conversation on whether we can do this), but for now lets go with the explicit
size_t static cast.  Note, another option would be to change "0U" to "0UL",
which would avoid the need for an implicit conversion (assuming size_t is also
an "unsigned long", which may not be the case on all platforms), but it is
probably clearer to do the static cast.

https://codereview.chromium.org/179373003/diff/1/base/i18n/rtl.cc
File base/i18n/rtl.cc (right):

https://codereview.chromium.org/179373003/diff/1/base/i18n/rtl.cc#newcode268
base/i18n/rtl.cc:268: text->insert(pos, num, kLeftToRightMark);
Please just do a static_cast<size_t>(0) here (and for all below).

[Yang Gu, 2014-02-27 07:21:36]

Thanks for the detailed analysis! I have followed your advice to use static_cast
directly, so please have a review again.

[rmcilroy, 2014-02-27 10:57:29]

lgtm

You will need to get an LGTM from someone who is an owner of base.

[bulach, 2014-02-27 15:36:01]

lgtm for base/android, thanks!

+mark, OWNERS review please for:
  base/base.gyp
  base/i18n/rtl.cc
  base/json/json_writer.cc

[Mark Mentovai, 2014-02-27 15:38:03]

LGTM in base.

[Yang Gu, 2014-02-28 02:13:03]

The CQ bit was checked by yang.gu@intel.com

[commit-bot: I haz the power, 2014-02-28 02:14:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yang.gu@intel.com/179373003/20001

[commit-bot: I haz the power, 2014-02-28 02:51:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-28 02:51:45]

Retried try job too often on win_rel for step(s) app_list_unittests,
ash_unittests, aura_unittests, cacheinvalidation_unittests, cc_unittests,
check_deps, check_deps2git, chrome_elf_unittests, chromedriver_unittests,
components_unittests, compositor_unittests, content_browsertests,
content_unittests, crypto_unittests, events_unittests, gfx_unittests,
google_apis_unittests, gpu_unittests, installer_util_unittests, ipc_tests,
jingle_unittests, media_unittests, mini_installer_test, nacl_integration,
ppapi_unittests, printing_unittests, remoting_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests, telemetry_perf_unittests,
telemetry_unittests, views_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Yang Gu, 2014-02-28 13:54:48]

The CQ bit was checked by yang.gu@intel.com

[commit-bot: I haz the power, 2014-02-28 13:55:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yang.gu@intel.com/179373003/20001

[commit-bot: I haz the power, 2014-02-28 15:25:36]

Change committed as 254124