Cleanup various includes and namespace references.

Cleanup various includes and namespace references.

These are mostly mechanical changes found while attempting to compile
the Android build with libc++.

BUG=427718
TBR=darin@chromium.org

Committed: https://crrev.com/e7be940cdf738454be4e7f61cd1fd0933fc91166
Cr-Commit-Position: refs/heads/master@{#302425}

[Fabrice (no longer in Chrome), 2014-10-31 12:09:54]

fdegans@chromium.org changed reviewers:
+ darin@chromium.org, gavinp@chromium.org, pasko@chromium.org,
skyostil@chromium.org, thakis@chromium.org

[Fabrice (no longer in Chrome), 2014-10-31 12:09:55]

PTAL, I think these are relatively straight-forward changes.

darin@chromium.org: Please review changes in 
third_party/
gavinp@chromium.org: Please review changes in 
net/
skyostil@chromium.org: Please review changes in 
content/browser/android
thakis@chromium.org: Please review changes in 
base/

[pasko, 2014-10-31 12:43:50]

lgtm! thank you!

[gavinp, 2014-10-31 12:54:36]

lgtm

[Nico, 2014-10-31 14:30:08]

Looks generally fine, but a few questions:

https://codereview.chromium.org/693773003/diff/1/base/debug/stack_trace_andro...
File base/debug/stack_trace_android.cc (right):

https://codereview.chromium.org/693773003/diff/1/base/debug/stack_trace_andro...
base/debug/stack_trace_android.cc:120: *os << std::string("<unknown>");
Out of interest, what happens without this? Writing char pointers to ostreams is
supposed to work, and this builds fine for me with libc++:

#include <iostream>
int main() {
  std::ostream* os = &std::cout;
  *os << "hi";
}

https://codereview.chromium.org/693773003/diff/1/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/693773003/diff/1/base/logging.h#newcode12
base/logging.h:12: #include <vector>
This file doesn't use vector anywhere from what I can tell. What's this needed
for?

https://codereview.chromium.org/693773003/diff/1/base/message_loop/message_lo...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/693773003/diff/1/base/message_loop/message_lo...
base/message_loop/message_loop.cc:11: #include "base/format_macros.h"
Likewise: What's this needed for? I don't see any PRI* use in this file.

https://codereview.chromium.org/693773003/diff/1/third_party/protobuf/README....
File third_party/protobuf/README.chromium (right):

https://codereview.chromium.org/693773003/diff/1/third_party/protobuf/README....
third_party/protobuf/README.chromium:37: and unknown_field_set.h
Did you change this upstream?

[Sami, 2014-10-31 14:33:27]

https://codereview.chromium.org/693773003/diff/1/content/browser/android/cont...
File content/browser/android/content_view_statics.cc (left):

https://codereview.chromium.org/693773003/diff/1/content/browser/android/cont...
content/browser/android/content_view_statics.cc:86: DCHECK_NE(pos,
suspended_processes_.end());
Is this working around some bug in the DCHECK_NE macro? Could we fix that
instead?

[pasko, 2014-10-31 15:38:24]

https://codereview.chromium.org/693773003/diff/1/content/browser/android/cont...
File content/browser/android/content_view_statics.cc (left):

https://codereview.chromium.org/693773003/diff/1/content/browser/android/cont...
content/browser/android/content_view_statics.cc:86: DCHECK_NE(pos,
suspended_processes_.end());
On 2014/10/31 14:33:27, Sami wrote:
> Is this working around some bug in the DCHECK_NE macro? Could we fix that
> instead?

I think *CHECK_NE macro (and similar) are better than their *CHECK(_ != _)
counterparts *only* by being able to print the values of the parameters in case
of mismatch, and only in cases we need the printing. We don't have good ways to
print iterators over collections (that I know of), so it seems better to _not_
do any useless printing (and instantiating the code to print) in this case.

In other words, not accepting CHECK_NE for iterators seems like a good feature
of the library (to me).

[Fabrice (no longer in Chrome), 2014-10-31 15:45:24]

Thanks for the review and comments everyone!
New patch + answers inline.

https://codereview.chromium.org/693773003/diff/1/base/debug/stack_trace_andro...
File base/debug/stack_trace_android.cc (right):

https://codereview.chromium.org/693773003/diff/1/base/debug/stack_trace_andro...
base/debug/stack_trace_android.cc:120: *os << std::string("<unknown>");
On 2014/10/31 14:30:08, Nico wrote:
> Out of interest, what happens without this? Writing char pointers to ostreams
is
> supposed to work, and this builds fine for me with libc++:
> 
> #include <iostream>
> int main() {
>   std::ostream* os = &std::cout;
>   *os << "hi";
> }

Maybe it works with Clang and not GCC? I noticed that happened a few times.
Either way, it's going to do an implicit conversion to std::string, right?

/usr/local/google/code/clankium/src/third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:4185:1:
note:   template argument deduction/substitution failed:
../../base/debug/stack_trace_android.cc:120:14: note:   mismatched types 'const
std::__1::basic_string<_CharT, _Traits, _Allocator>' and 'const char [10]'
       *os << "<unknown>";
              ^

https://codereview.chromium.org/693773003/diff/1/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/693773003/diff/1/base/logging.h#newcode12
base/logging.h:12: #include <vector>
On 2014/10/31 14:30:08, Nico wrote:
> This file doesn't use vector anywhere from what I can tell. What's this needed
> for?

This comes from an earlier version of the patch, removed. Thanks!
I took this opportunity to double-check every of the changes in this CL are
needed for libc++.

https://codereview.chromium.org/693773003/diff/1/base/message_loop/message_lo...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/693773003/diff/1/base/message_loop/message_lo...
base/message_loop/message_loop.cc:11: #include "base/format_macros.h"
On 2014/10/31 14:30:08, Nico wrote:
> Likewise: What's this needed for? I don't see any PRI* use in this file.

Done.

https://codereview.chromium.org/693773003/diff/1/content/browser/android/cont...
File content/browser/android/content_view_statics.cc (left):

https://codereview.chromium.org/693773003/diff/1/content/browser/android/cont...
content/browser/android/content_view_statics.cc:86: DCHECK_NE(pos,
suspended_processes_.end());
On 2014/10/31 14:33:27, Sami wrote:
> Is this working around some bug in the DCHECK_NE macro? Could we fix that
> instead?
I'm inclined to say the problem is here.
There is no conversion from std::vector<>::iterator to std::string when the
compiler tries to instantiate MakeCheckOpString in base/logging.h
DCHECK_OP is only supposed to be used for simple types, as I understand it.

https://codereview.chromium.org/693773003/diff/1/third_party/protobuf/README....
File third_party/protobuf/README.chromium (right):

https://codereview.chromium.org/693773003/diff/1/third_party/protobuf/README....
third_party/protobuf/README.chromium:37: and unknown_field_set.h
On 2014/10/31 14:30:08, Nico wrote:
> Did you change this upstream?

I'm working on that part, just not sure if that should be in github or
elsewhere, but I did ping the protobuf folks about it.

[Nico, 2014-10-31 16:10:03]

https://codereview.chromium.org/693773003/diff/1/base/debug/stack_trace_andro...
File base/debug/stack_trace_android.cc (right):

https://codereview.chromium.org/693773003/diff/1/base/debug/stack_trace_andro...
base/debug/stack_trace_android.cc:120: *os << std::string("<unknown>");
> Maybe it works with Clang and not GCC? I noticed that happened a few times.

Oh, that's a good point. Works with gcc on my mac though:

$ gcc-4.8.1 -c test.cc  -isysroot $(xcrun -show-sdk-path) -I
~/src/libcxx/include/

(I checked it's actually picking up the libcxx iostream by rerunning this with
-H. I also tried the same with -std=gnu++11.)

> Either way, it's going to do an implicit conversion to std::string, right?

On my system at least, there's an operator<<(ostream*, const char*):

$ clang -c test.cc  -Xclang -ast-dump | tail -8

`-FunctionDecl 0x103eb6e70 <test.cc:2:1, line:4:1> main 'int (void)'
  `-CompoundStmt 0x103ebaef8 <line:2:12, line:4:1>
    `-CXXOperatorCallExpr 0x103ebaeb0 <line:3:3, col:16> 'basic_ostream<char,
struct std::__1::char_traits<char> >':'class std::__1::basic_ostream<char>'
lvalue
      |-ImplicitCastExpr 0x103ebae98 <col:13> 'basic_ostream<char, struct
std::__1::char_traits<char> > &(*)(basic_ostream<char, struct
std::__1::char_traits<char> > &, const char *)' <FunctionToPointerDecay>
      | `-DeclRefExpr 0x103ebae18 <col:13> 'basic_ostream<char, struct
std::__1::char_traits<char> > &(basic_ostream<char, struct
std::__1::char_traits<char> > &, const char *)' lvalue Function 0x103eb7f00
'operator<<' 'basic_ostream<char, struct std::__1::char_traits<char> >
&(basic_ostream<char, struct std::__1::char_traits<char> > &, const char *)'
      |-DeclRefExpr 0x103eb6f48 <col:3, col:8> 'ostream':'class
std::__1::basic_ostream<char>' lvalue Var 0x103eb6830 'cout' 'ostream':'class
std::__1::basic_ostream<char>'
      `-ImplicitCastExpr 0x103ebae00 <col:16> 'const char *'
<ArrayToPointerDecay>
        `-StringLiteral 0x103eb6f80 <col:16> 'const char [3]' lvalue "hi"

(look for "operator<< on line 5)

On my system, `operator<<(basic_ostream<char, _Traits>& __os, const char*
__str)` is defined around line 846 in libcxx/include/ostream – maybe this file
is just missing an `#include <ostream>`?

https://codereview.chromium.org/693773003/diff/1/third_party/protobuf/README....
File third_party/protobuf/README.chromium (right):

https://codereview.chromium.org/693773003/diff/1/third_party/protobuf/README....
third_party/protobuf/README.chromium:37: and unknown_field_set.h
On 2014/10/31 15:45:24, Fabrice de Gans wrote:
> On 2014/10/31 14:30:08, Nico wrote:
> > Did you change this upstream?
> 
> I'm working on that part, just not sure if that should be in github or
> elsewhere, but I did ping the protobuf folks about it.

I don't know either. agl@, pliard@, rmcilory@ have successfully upstreamed
protobuf stuff (e.g. https://codereview.appspot.com/88800045/), so maybe ask
them. (But yes, looks like they moved to github recently from what I can tell.)

[Fabrice (no longer in Chrome), 2014-10-31 17:01:15]

https://codereview.chromium.org/693773003/diff/1/base/debug/stack_trace_andro...
File base/debug/stack_trace_android.cc (right):

https://codereview.chromium.org/693773003/diff/1/base/debug/stack_trace_andro...
base/debug/stack_trace_android.cc:120: *os << std::string("<unknown>");
On 2014/10/31 16:10:03, Nico wrote:
> [SNIP]
> On my system, `operator<<(basic_ostream<char, _Traits>& __os, const char*
> __str)` is defined around line 846 in libcxx/include/ostream –&nbsp;maybe this
file
> is just missing an `#include <ostream>`?
Yes, that was it, thanks!

[Nico, 2014-10-31 17:02:13]

base/ lgtm

[Sami, 2014-10-31 18:59:20]

On 2014/10/31 15:38:24, pasko wrote:
>
https://codereview.chromium.org/693773003/diff/1/content/browser/android/cont...
> File content/browser/android/content_view_statics.cc (left):
> 
>
https://codereview.chromium.org/693773003/diff/1/content/browser/android/cont...
> content/browser/android/content_view_statics.cc:86: DCHECK_NE(pos,
> suspended_processes_.end());
> On 2014/10/31 14:33:27, Sami wrote:
> > Is this working around some bug in the DCHECK_NE macro? Could we fix that
> > instead?
> 
> I think *CHECK_NE macro (and similar) are better than their *CHECK(_ != _)
> counterparts *only* by being able to print the values of the parameters in
case
> of mismatch, and only in cases we need the printing. We don't have good ways
to
> print iterators over collections (that I know of), so it seems better to _not_
> do any useless printing (and instantiating the code to print) in this case.
> 
> In other words, not accepting CHECK_NE for iterators seems like a good feature
> of the library (to me).

Ok, that makes sense. content/browser/android lgtm.

[Fabrice (no longer in Chrome), 2014-11-03 12:52:21]

Change to protobuf being upstreamed here:
https://github.com/google/protobuf/pull/72

Adding darin@ to TBR, per instructions from third_party/OWNERS

[Fabrice (no longer in Chrome), 2014-11-03 12:52:25]

The CQ bit was checked by fdegans@chromium.org

[commit-bot: I haz the power, 2014-11-03 12:53:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/693773003/40001

[commit-bot: I haz the power, 2014-11-03 13:59:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-03 13:59:56]

Patchset 3 (id:??) landed as
https://crrev.com/e7be940cdf738454be4e7f61cd1fd0933fc91166
Cr-Commit-Position: refs/heads/master@{#302425}