[NaCl SDK] nacl_io: implement getaddrinfo()

[NaCl SDK] nacl_io: implement getaddrinfo()

Change gethostbyname such that is implemented in
terms of getaddrinfo().

Change lookups such that purely numeric lookups don't
rely on the PPAPI interface at all.

Add lock to gethostbyname() so that it at least won't
crash when run from multiple threads.

I'm sure there are still edge cases where this version
of getaddrinfo does the wrong thing, but I've tried to
cover all simple uses with unittests.

BUG=315197
R=binji@chromium.org

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

[Sam Clegg, 2014-01-29 17:30:26]

If you like I could split the updated fakes into their own CL, but it kinda
makes sense to land the fakes at the same time as the code that uses them I
guess.

[binji, 2014-01-29 19:18:52]

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/host_resolver.cc (right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:25: pp_hints->family =
PP_NETADDRESS_FAMILY_IPV6;
default value for else?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:37: memset(ai, 0,
sizeof(*ai));
Technically this is unnecessary because addrinfo is POD and you called new
addrinfo with parentheses above: see http://stackoverflow.com/a/620402

Probably obscure enough of a rule to leave as is (or comment) however.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:62: }
default case?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:99: case
PP_ERROR_NOACCESS:
getaddrinfo seems to return EAI_ errors, not PP_ errors.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:160: for (int i = 0; i
< num_addresses; i++) {
Now that I look at it closely, this loop is wrong. I think i++ should be moved
past the continue guard below; otherwise we'll drop addresses at the end of the
list.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:162: continue;
This continue is broken; it needs to always do

current = current->ai_next.

Perhaps the loop should be a while loop like above?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:192: free(cur);
move this below the other frees

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:225: case 0:
AF_UNSPEC?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:241: *result = NULL;
set this to NULL earlier in the function?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:267: if
(hints->ai_family == AF_INET6 || hints->ai_family == 0) {
AF_UNSPEC?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:270:
CreateAddrInfo(hints, (sockaddr*)&addr_in6, NULL, result, &end);
Is it correct to return both v4 and v6 addresses in this case?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:299: callback.func =
NULL;
Just use PP_BlockUntilComplete()?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:328:
addr.Reset(resolver_iface->GetNetAddress(resolver, 0));
just construct this directly, like above?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:335: return EAI_NONAME;
Maybe EAI_NODATA is more correct here?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:346: assert(false);
If this fires, it is a PPAPI failure, not ours. Maybe we should be resilient to
that?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc (right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc:1274: return
host_resolver_.getaddrinfo(node, service, hints, res);
In the man pages it says EAI_SYSTEM should set errno. Probably makes sense to
set that here...?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/syscalls/gai_strerror.c (right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/syscalls/gai_strerror.c:1: // Copyright
2013 The Chromium Authors. All rights reserved.
2014

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/syscalls/gai_strerror.c:11: #ifdef
__GLIBC__
Isn't this already implemented in glibc?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
File
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.cc
(right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.cc:58:
if (!fake_hostname.empty() && fake_hostname == std::string(hostname)) {
fake_hostname == hostname

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_resource_manager.h
(right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_resource_manager.h:34:
FakeResourceTracker* Get(PP_Resource handle, bool silent=false);
I don't like bool parameters to functions; can you make this an enum instead?
(also, where is this used?)

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
File native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc (right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:42:
(FakeHostResolverInterface*)pepper_->GetHostResolverInterface();
nit: use C++-style cast

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:51: void
AddFakeAddres(int family) {
sp: AddFakeAddress

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:54: // Each new
address we add is FAKE_IP incremented by 1
Maybe cleaner to just have a limited fixed list of addresses? Then you wouldn't
have to duplicate the logic here and below in the test code.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:88: //#define
USE_SYSTEM_NS
what is NS? NameService?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:152:
ASSERT_EQ(AF_INET, in->sin_family);
free addrinfo here?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:218:
ASSERT_GT(count, 1);
ASSERT_EQ(2, COUNT), Or could it be more than that?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:249:
TEST_F(FakeHostResolverTest, Getaddrinfo_Multi) {
doesn't this need to be ifndef'd too?

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:254: // Add three
fake address on top of the initial one
four

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:265: ASSERT_EQ(0,
ki_getaddrinfo(FAKE_HOSTNAME, NULL, &hints, &ai));
ki_freeaddrinfo for each call to ki_getaddrinfo

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:275:
ASSERT_NE(NULL_ADDR, current->ai_addr);
Move these tests into a function, to simplify? They're duplicated quite a bit

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:341: // Looking
up purly numeric hostnames should work without PPAPI
sp: purely

[Sam Clegg, 2014-01-29 22:06:12]

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/host_resolver.cc (right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:25: pp_hints->family =
PP_NETADDRESS_FAMILY_IPV6;
On 2014/01/29 19:18:52, binji wrote:
> default value for else?

Default value is UNSPECIFIED which is 0.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:37: memset(ai, 0,
sizeof(*ai));
On 2014/01/29 19:18:52, binji wrote:
> Technically this is unnecessary because addrinfo is POD and you called new
> addrinfo with parentheses above: see http://stackoverflow.com/a/620402
> 
> Probably obscure enough of a rule to leave as is (or comment) however.

Very interesting.  I removed the parens and kept the memset

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:62: }
On 2014/01/29 19:18:52, binji wrote:
> default case?

We only support these to families.  I've added an assert here in the default
case.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:99: case
PP_ERROR_NOACCESS:
On 2014/01/29 19:18:52, binji wrote:
> getaddrinfo seems to return EAI_ errors, not PP_ errors.

Good call.  Added tests for that too.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:160: for (int i = 0; i
< num_addresses; i++) {
On 2014/01/29 19:18:52, binji wrote:
> Now that I look at it closely, this loop is wrong. I think i++ should be moved
> past the continue guard below; otherwise we'll drop addresses at the end of
the
> list.

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:162: continue;
On 2014/01/29 19:18:52, binji wrote:
> This continue is broken; it needs to always do
> 
> current = current->ai_next.
> 
> Perhaps the loop should be a while loop like above?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:192: free(cur);
On 2014/01/29 19:18:52, binji wrote:
> move this below the other frees

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:225: case 0:
On 2014/01/29 19:18:52, binji wrote:
> AF_UNSPEC?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:241: *result = NULL;
On 2014/01/29 19:18:52, binji wrote:
> set this to NULL earlier in the function?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:267: if
(hints->ai_family == AF_INET6 || hints->ai_family == 0) {
On 2014/01/29 19:18:52, binji wrote:
> AF_UNSPEC?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:270:
CreateAddrInfo(hints, (sockaddr*)&addr_in6, NULL, result, &end);
On 2014/01/29 19:18:52, binji wrote:
> Is it correct to return both v4 and v6 addresses in this case?

Yes. At least this is what glibc/linux does.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:299: callback.func =
NULL;
On 2014/01/29 19:18:52, binji wrote:
> Just use PP_BlockUntilComplete()?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:328:
addr.Reset(resolver_iface->GetNetAddress(resolver, 0));
On 2014/01/29 19:18:52, binji wrote:
> just construct this directly, like above?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:335: return EAI_NONAME;
On 2014/01/29 19:18:52, binji wrote:
> Maybe EAI_NODATA is more correct here?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:346: assert(false);
On 2014/01/29 19:18:52, binji wrote:
> If this fires, it is a PPAPI failure, not ours. Maybe we should be resilient
to
> that?

This is resilient, at least in release builds it will just skip over the
address.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc (right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc:1274: return
host_resolver_.getaddrinfo(node, service, hints, res);
On 2014/01/29 19:18:52, binji wrote:
> In the man pages it says EAI_SYSTEM should set errno. Probably makes sense to
> set that here...?

That would be a little more complicated, since our internal
getaddrinfo could then need to return two error codes somehow.
I'm not sure we have any more fine-grained information to
store in errno anyway, so I'll leave this is a TODO if thats
OK.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/syscalls/gai_strerror.c (right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/syscalls/gai_strerror.c:1: // Copyright
2013 The Chromium Authors. All rights reserved.
On 2014/01/29 19:18:52, binji wrote:
> 2014

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/syscalls/gai_strerror.c:11: #ifdef
__GLIBC__
On 2014/01/29 19:18:52, binji wrote:
> Isn't this already implemented in glibc?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_resource_manager.h
(right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_resource_manager.h:34:
FakeResourceTracker* Get(PP_Resource handle, bool silent=false);
On 2014/01/29 19:18:52, binji wrote:
> I don't like bool parameters to functions; can you make this an enum instead?
> (also, where is this used?)

I just reverted this part of the change

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
File native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc (right):

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:42:
(FakeHostResolverInterface*)pepper_->GetHostResolverInterface();
On 2014/01/29 19:18:52, binji wrote:
> nit: use C++-style cast

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:51: void
AddFakeAddres(int family) {
On 2014/01/29 19:18:52, binji wrote:
> sp: AddFakeAddress

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:88: //#define
USE_SYSTEM_NS
On 2014/01/29 19:18:52, binji wrote:
> what is NS? NameService?

Added comment

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:152:
ASSERT_EQ(AF_INET, in->sin_family);
On 2014/01/29 19:18:52, binji wrote:
> free addrinfo here?

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:254: // Add three
fake address on top of the initial one
On 2014/01/29 19:18:52, binji wrote:
> four

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:265: ASSERT_EQ(0,
ki_getaddrinfo(FAKE_HOSTNAME, NULL, &hints, &ai));
On 2014/01/29 19:18:52, binji wrote:
> ki_freeaddrinfo for each call to ki_getaddrinfo

Done.

https://codereview.chromium.org/99933002/diff/120001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:275:
ASSERT_NE(NULL_ADDR, current->ai_addr);
On 2014/01/29 19:18:52, binji wrote:
> Move these tests into a function, to simplify? They're duplicated quite a bit

Done.

[binji, 2014-01-30 00:36:52]

lgtm

https://codereview.chromium.org/99933002/diff/140001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/host_resolver.cc (right):

https://codereview.chromium.org/99933002/diff/140001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:36: addrinfo* ai =
reinterpret_cast<addrinfo*>(malloc(sizeof(addrinfo)));
this can just be a static_cast

[Sam Clegg, 2014-02-04 22:33:19]

The CQ bit was checked by sbc@chromium.org

[commit-bot: I haz the power, 2014-02-04 23:01:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sbc@chromium.org/99933002/270001

[commit-bot: I haz the power, 2014-02-05 01:24:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sbc@chromium.org/99933002/270001

[Paweł Hajdan Jr., 2014-02-05 19:50:34]

The CQ bit was unchecked by phajdan.jr@chromium.org

[Paweł Hajdan Jr., 2014-02-05 19:53:40]

The CQ bit was checked by phajdan.jr@chromium.org

[commit-bot: I haz the power, 2014-02-06 04:18:58]

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

[commit-bot: I haz the power, 2014-02-06 04:18:59]

Retried try job too often on linux_chromeos for step(s) app_list_unittests,
ash_unittests, aura_unittests, base_unittests, browser_tests,
cacheinvalidation_unittests, check_deps, check_deps2git, chromeos_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
dbus_unittests, device_unittests, events_unittests, google_apis_unittests,
gpu_unittests, interactive_ui_tests, ipc_tests, jingle_unittests,
media_unittests, net_unittests, ppapi_unittests, printing_unittests,
sandbox_linux_unittests, sql_unittests, sync_unit_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[Sam Clegg, 2014-02-06 16:54:30]

The CQ bit was checked by sbc@chromium.org

[commit-bot: I haz the power, 2014-02-06 16:55:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sbc@chromium.org/99933002/270001

[commit-bot: I haz the power, 2014-02-06 16:55:22]

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

[commit-bot: I haz the power, 2014-02-06 16:55:23]

Failed to apply patch for
native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c
  Hunk #1 FAILED at 6.
  1 out of 1 hunk FAILED -- saving rejects to file
native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c.rej

Patch:       native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c
Index: native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c
diff --git a/native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c
b/native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c
index
472eed25591f70da095f42f6a61ca9b97d734fe6..3b7958a66fb85af61cbb664239a0ff04e938ef85
100644
--- a/native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c
+++ b/native_client_sdk/src/libraries/nacl_io/syscalls/ioctl.c
@@ -6,7 +6,6 @@
 
 #include "nacl_io/kernel_intercept.h"
 #include "nacl_io/kernel_wrap.h"
-#include "nacl_io/dbgprint.h"
 
 int ioctl(int fd, unsigned long request, ...) {
   va_list ap;

[Sam Clegg, 2014-02-06 20:10:47]

The CQ bit was checked by sbc@chromium.org

[commit-bot: I haz the power, 2014-02-06 20:12:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sbc@chromium.org/99933002/1050001

[commit-bot: I haz the power, 2014-02-06 21:32:47]

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

[commit-bot: I haz the power, 2014-02-06 21:32:48]

Retried try job too often on android_aosp for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...

[Sam Clegg, 2014-02-06 21:55:47]

Committed patchset #8 manually as r249510 (presubmit successful).