[NaCl SDK] nacl_io: Add new fakes

[NaCl SDK] nacl_io: Add fakes for more types of PP_Var.

Add fakes for VAR_ARRAY and VAR_ARRAY_BUFFER

R=binji@chromium.org

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

[binji, 2014-01-17 01:31:14]

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/example.dsc (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/example.dsc:26:
'fake_var_array_buffer_interface.cc',
nit: alphabetize

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_messaging_interface.h
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_messaging_interface.h:21: explicit
FakeMessagingInterface(FakeVarManager* manager,
remove explicit for 2 arg constructor

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_pepper_interface_html5_fs.h
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_pepper_interface_html5_fs.h:153:
FakeVarManager* var_manager_;
weak reference

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File
native_client_sdk/src/tests/nacl_io_test/fake_pepper_interface_url_loader.cc
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_pepper_interface_url_loader.cc:500:
: var_interface_(&var_manager_), url_loader_interface_(&core_interface_),
nit: one line per variable

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc:8:
#include "gtest/gtest.h"
used?

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc:27:
FakeVarData* data = manager_->GetVarData(var);
NULL check? Here and below

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.h
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.h:1: //
Copyright (c) 2013 The Chromium Authors. All rights reserved.
2014

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_interface.cc
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_interface.cc:24:
FakeVarData* data = manager_->GetVarData(var);
NULL check? here and the rest of the file

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_interface.h
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_interface.h:20:
FakeVarArrayInterface(FakeVarManager* manager);
explicit

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_interface.h (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_interface.h:17:
FakeVarInterface(FakeVarManager* manager);
explicit

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_interface.h:25:
FakeVarManager* manager_;
DISALLOW_COPY_AND_ASSIGN?

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc:59: default:
array buffer?

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc:70:
std::vector<PP_Var>& vector = GetVarData(var)->array_value;
NULL check?

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc:113: Destroy(var);
erase item in var_map_

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_manager.h (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_manager.h:43: typedef
std::map<Id, FakeVarData> VarMap;
nit: group typedefs together

[Sam Clegg, 2014-01-17 22:40:11]

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/example.dsc (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/example.dsc:26:
'fake_var_array_buffer_interface.cc',
On 2014/01/17 01:31:14, binji wrote:
> nit: alphabetize

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_messaging_interface.h
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_messaging_interface.h:21: explicit
FakeMessagingInterface(FakeVarManager* manager,
On 2014/01/17 01:31:14, binji wrote:
> remove explicit for 2 arg constructor

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_pepper_interface_html5_fs.h
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_pepper_interface_html5_fs.h:153:
FakeVarManager* var_manager_;
On 2014/01/17 01:31:14, binji wrote:
> weak reference

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File
native_client_sdk/src/tests/nacl_io_test/fake_pepper_interface_url_loader.cc
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_pepper_interface_url_loader.cc:500:
: var_interface_(&var_manager_), url_loader_interface_(&core_interface_),
On 2014/01/17 01:31:14, binji wrote:
> nit: one line per variable

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc:8:
#include "gtest/gtest.h"
On 2014/01/17 01:31:14, binji wrote:
> used?

It is now :)

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc:27:
FakeVarData* data = manager_->GetVarData(var);
On 2014/01/17 01:31:14, binji wrote:
> NULL check? Here and below

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.h
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.h:1: //
Copyright (c) 2013 The Chromium Authors. All rights reserved.
On 2014/01/17 01:31:14, binji wrote:
> 2014

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_interface.cc
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_interface.cc:24:
FakeVarData* data = manager_->GetVarData(var);
On 2014/01/17 01:31:14, binji wrote:
> NULL check? here and the rest of the file

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_interface.h
(right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_interface.h:20:
FakeVarArrayInterface(FakeVarManager* manager);
On 2014/01/17 01:31:14, binji wrote:
> explicit

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_interface.h (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_interface.h:17:
FakeVarInterface(FakeVarManager* manager);
On 2014/01/17 01:31:14, binji wrote:
> explicit

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_interface.h:25:
FakeVarManager* manager_;
On 2014/01/17 01:31:14, binji wrote:
> DISALLOW_COPY_AND_ASSIGN?

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc:59: default:
On 2014/01/17 01:31:14, binji wrote:
> array buffer?

Done.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc:113: Destroy(var);
On 2014/01/17 01:31:14, binji wrote:
> erase item in var_map_

I was thinking of leaving the var_map alone until the manager is destroyed. 
Then we don't reuse Id's, and we can check the everything has a refcount of zero
at shutdown.

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_manager.h (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_manager.h:43: typedef
std::map<Id, FakeVarData> VarMap;
On 2014/01/17 01:31:14, binji wrote:
> nit: group typedefs together

Done.

[binji, 2014-01-17 22:59:17]

lgtm

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
File native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc (right):

https://codereview.chromium.org/137853023/diff/1/native_client_sdk/src/tests/...
native_client_sdk/src/tests/nacl_io_test/fake_var_manager.cc:113: Destroy(var);
On 2014/01/17 22:40:11, Sam Clegg wrote:
> On 2014/01/17 01:31:14, binji wrote:
> > erase item in var_map_
> 
> I was thinking of leaving the var_map alone until the manager is destroyed. 
> Then we don't reuse Id's, and we can check the everything has a refcount of
zero
> at shutdown.
> 

OK, sounds good. Maybe add a comment for that?

https://codereview.chromium.org/137853023/diff/350001/native_client_sdk/src/t...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc
(right):

https://codereview.chromium.org/137853023/diff/350001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc:28:
EXPECT_NE(static_cast<FakeVarData*>(NULL), data);
AFAIK, this will just print an error and then crash. I suppose that's OK.

https://codereview.chromium.org/137853023/diff/350001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc:41:
ASSERT_EQ(PP_VARTYPE_ARRAY_BUFFER, var.type);
ASSERT_EQ is irrelevant here -- it will just do EXPECT_EQ followed by return.

[Sam Clegg, 2014-01-17 23:50:30]

https://codereview.chromium.org/137853023/diff/350001/native_client_sdk/src/t...
File native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc
(right):

https://codereview.chromium.org/137853023/diff/350001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/fake_var_array_buffer_interface.cc:28:
EXPECT_NE(static_cast<FakeVarData*>(NULL), data);
On 2014/01/17 22:59:18, binji wrote:
> AFAIK, this will just print an error and then crash. I suppose that's OK.

Yes, I'd rather do ASSERT_EQ but that doesn't work in non-void functions I
think.

[Sam Clegg, 2014-01-18 01:10:45]

Committed patchset #2 manually as r245696 (presubmit successful).