[NaCl SDK] Add fake for ppb_host_resolver and ppb_net_address

[NaCl SDK] Add fake for ppb_host_resolver and ppb_net_address

Also add tests for gethostbyname that usese these fakes
and fix the resulting leaks found in host_resolver.cc.

Also add a third variant of ki_init() which takes a
PepperInterface instance. This can be used by the test
code to inject a fake pepper interface.

R=binji@chromium.org

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

[binji, 2014-01-28 18:13:22]

lgtm, w/ some comments and nits

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

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:140:
ppapi_->ReleaseResource(addr);
You can use a ScopedResource (see pepper_interface.h) to prevent having to clean
this up everywhere.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_intercept.h (right):

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_intercept.h:36: void
ki_init_interface(void* kp, void* pepper_interface);
Add a doc comment for this; it is useful to know that this is expecting a class
of type nacl_io::PepperInterface

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_core_interface.h
(right):

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_core_interface.h:14:
FakeCoreInterface(FakeResourceManager* manager);
explicit

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_core_interface.h:23:
FakeResourceManager* resource_manager_;
weak reference

https://codereview.chromium.org/148223005/diff/20001/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/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.cc:16:
#define FAKE_HOSTNAME "testhost.com"
example.com is more traditional, right?

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.cc:17:
#define FAKE_IP 0x01020304
In the other fakes, I allowed the user of the fake to specify the fake data to
use. This makes it simpler to add more tests, as well as centralizing the fake
inputs with the fake outputs in the test code.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.cc:85:
return 1;
is it worth testing the cases where this can return > 1?

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
File
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.h
(right):

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.h:5:
#ifndef TESTS_NACL_IO_TEST_FAKE_HOST_RESOLVER_INTERFACE_H
nit: underscore at end of guard

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.h:13:
class FakeVarManager;
nit: alphabetize
here and other new files

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
File
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_net_address_interface.cc
(right):

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_net_address_interface.cc:18:
FakeNetAddressResource(PP_NetAddress_IPv4 addr) : address(addr) {}
explicit

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_net_address_interface.cc:24:
}
nit: closing namespace comment

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

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:20: //class
FakeKernelProxy : public KernelProxy {
remove

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:38:
//FakeKernelProxy fake_proxy_;
remove

[Sam Clegg, 2014-01-28 19:55:22]

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

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/host_resolver.cc:140:
ppapi_->ReleaseResource(addr);
On 2014/01/28 18:13:22, binji wrote:
> You can use a ScopedResource (see pepper_interface.h) to prevent having to
clean
> this up everywhere.

Done.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_intercept.h (right):

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_intercept.h:36: void
ki_init_interface(void* kp, void* pepper_interface);
On 2014/01/28 18:13:22, binji wrote:
> Add a doc comment for this; it is useful to know that this is expecting a
class
> of type nacl_io::PepperInterface

Done.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_core_interface.h
(right):

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_core_interface.h:14:
FakeCoreInterface(FakeResourceManager* manager);
On 2014/01/28 18:13:22, binji wrote:
> explicit

Done.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_core_interface.h:23:
FakeResourceManager* resource_manager_;
On 2014/01/28 18:13:22, binji wrote:
> weak reference

Done.

https://codereview.chromium.org/148223005/diff/20001/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/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.cc:16:
#define FAKE_HOSTNAME "testhost.com"
On 2014/01/28 18:13:22, binji wrote:
> http://example.com is more traditional, right?

Done.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.cc:17:
#define FAKE_IP 0x01020304
On 2014/01/28 18:13:22, binji wrote:
> In the other fakes, I allowed the user of the fake to specify the fake data to
> use. This makes it simpler to add more tests, as well as centralizing the fake
> inputs with the fake outputs in the test code.

I moved these into the .h file and use them in the test code.

We can extend the fake to be more configurable if/when we need it maybe?

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.cc:85:
return 1;
On 2014/01/28 18:13:22, binji wrote:
> is it worth testing the cases where this can return > 1?

Yes, CL is a preliminary CL to the one where I add getaddrinfo.
I will write more tests for that and extent these fakes, but I wanted
to get the basic fakes for this stuff landed first.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
File
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.h
(right):

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.h:5:
#ifndef TESTS_NACL_IO_TEST_FAKE_HOST_RESOLVER_INTERFACE_H
On 2014/01/28 18:13:22, binji wrote:
> nit: underscore at end of guard

Done.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_host_resolver_interface.h:13:
class FakeVarManager;
On 2014/01/28 18:13:22, binji wrote:
> nit: alphabetize
> here and other new files

Done.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
File
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_net_address_interface.cc
(right):

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_net_address_interface.cc:18:
FakeNetAddressResource(PP_NetAddress_IPv4 addr) : address(addr) {}
On 2014/01/28 18:13:22, binji wrote:
> explicit

Done.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_net_address_interface.cc:24:
}
On 2014/01/28 18:13:22, binji wrote:
> nit: closing namespace comment

Done.

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

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:20: //class
FakeKernelProxy : public KernelProxy {
On 2014/01/28 18:13:22, binji wrote:
> remove

Done.

https://codereview.chromium.org/148223005/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/host_resolver_test.cc:38:
//FakeKernelProxy fake_proxy_;
On 2014/01/28 18:13:22, binji wrote:
> remove

Done.

[Sam Clegg, 2014-01-28 19:57:39]

Committed patchset #3 manually as r247487 (presubmit successful).