[NaCl SDK] Refactor FakeFileSystemInterface.

[NaCl SDK] Refactor FakeFileSystemInterface.

Move FakeFileSystemInterface from fake_pepper_interface_html5_fs.{cc,h} to
fake_file_system_interface.{cc,h} to share the code with the future code.
depot_tools/clang-format was run so the formatting of code is accepted
in the code review by policy. It was run in all the {.cc,.h} files,
and the code formats of some files were changed.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk

Committed: https://crrev.com/9b7e1ceacd7932dc4b1d129bf5245c9de381eb9d
Cr-Commit-Position: refs/heads/master@{#429186}

[chanpatorikku, 2016-10-27 17:45:50]

Again, me nad the refactored code.  PTAL.

https://codereview.chromium.org/2455973003/diff/1/native_client_sdk/src/tests...
File
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_file_system_interface.cc
(right):

https://codereview.chromium.org/2455973003/diff/1/native_client_sdk/src/tests...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_file_system_interface.cc:16:
class FakeInstanceResource : public FakeResource {
FakeInstanceResource used to be used by FakePepperInterfaceHtml5Fs and
FakeFileSystemInterface in fake_pepper_interface_html5_fs.cc.

After refactoring, two exact copies of FakeInstanceResource are in the unnamed
namespaces of fake_pepper_interface_html5_fs.cc and
fake_file_system_interface.cc.

Moving the two exact copies into one copy in, say, fake_util.h results in a
compiler error of FakeInstanceResource being ambiguous because there's another
class FakeInstanceResource with a different definition in
fake_pepper_interface_url_loader.cc:

class FakeInstanceResource : public FakeResource {
 public:
  FakeInstanceResource() : server_template(NULL) {}
  static const char* classname() { return "FakeInstanceResource"; }

  FakeURLLoaderServer* server_template;  // Weak reference.
};

To prevent the compiler error, two exact copies of FakeInstanceResource are in
the unnamed namespaces of FakePepperInterfaceHtml5Fs.cc and
FakeFileSystemInterface.cc.

[Sam Clegg, 2016-10-27 17:50:42]

On 2016/10/27 17:45:50, chanpatorikku wrote:
> Again, me nad the refactored code.  PTAL.
> 
>
https://codereview.chromium.org/2455973003/diff/1/native_client_sdk/src/tests...
> File
>
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_file_system_interface.cc
> (right):
> 
>
https://codereview.chromium.org/2455973003/diff/1/native_client_sdk/src/tests...
>
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_file_system_interface.cc:16:
> class FakeInstanceResource : public FakeResource {
> FakeInstanceResource used to be used by FakePepperInterfaceHtml5Fs and
> FakeFileSystemInterface in fake_pepper_interface_html5_fs.cc.
> 
> After refactoring, two exact copies of FakeInstanceResource are in the unnamed
> namespaces of fake_pepper_interface_html5_fs.cc and
> fake_file_system_interface.cc.
> 
> Moving the two exact copies into one copy in, say, fake_util.h results in a
> compiler error of FakeInstanceResource being ambiguous because there's another
> class FakeInstanceResource with a different definition in
> fake_pepper_interface_url_loader.cc:
> 
> class FakeInstanceResource : public FakeResource {
>  public:
>   FakeInstanceResource() : server_template(NULL) {}
>   static const char* classname() { return "FakeInstanceResource"; }
> 
>   FakeURLLoaderServer* server_template;  // Weak reference.
> };
> 
> To prevent the compiler error, two exact copies of FakeInstanceResource are in
> the unnamed namespaces of FakePepperInterfaceHtml5Fs.cc and
> FakeFileSystemInterface.cc.

I would just come up with a better name of the FakeInstanceResource that you
want to put in a header file.  No need to duplicate it.

[binji, 2016-10-27 21:26:09]

On 2016/10/27 17:50:42, Sam Clegg wrote:
> On 2016/10/27 17:45:50, chanpatorikku wrote:
> > Again, me nad the refactored code.  PTAL.
> > 
> >
>
https://codereview.chromium.org/2455973003/diff/1/native_client_sdk/src/tests...
> > File
> >
>
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_file_system_interface.cc
> > (right):
> > 
> >
>
https://codereview.chromium.org/2455973003/diff/1/native_client_sdk/src/tests...
> >
>
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_file_system_interface.cc:16:
> > class FakeInstanceResource : public FakeResource {
> > FakeInstanceResource used to be used by FakePepperInterfaceHtml5Fs and
> > FakeFileSystemInterface in fake_pepper_interface_html5_fs.cc.
> > 
> > After refactoring, two exact copies of FakeInstanceResource are in the
unnamed
> > namespaces of fake_pepper_interface_html5_fs.cc and
> > fake_file_system_interface.cc.
> > 
> > Moving the two exact copies into one copy in, say, fake_util.h results in a
> > compiler error of FakeInstanceResource being ambiguous because there's
another
> > class FakeInstanceResource with a different definition in
> > fake_pepper_interface_url_loader.cc:
> > 
> > class FakeInstanceResource : public FakeResource {
> >  public:
> >   FakeInstanceResource() : server_template(NULL) {}
> >   static const char* classname() { return "FakeInstanceResource"; }
> > 
> >   FakeURLLoaderServer* server_template;  // Weak reference.
> > };
> > 
> > To prevent the compiler error, two exact copies of FakeInstanceResource are
in
> > the unnamed namespaces of FakePepperInterfaceHtml5Fs.cc and
> > FakeFileSystemInterface.cc.
> 
> I would just come up with a better name of the FakeInstanceResource that you
> want to put in a header file.  No need to duplicate it.

Yeah, that would be ok: maybe FakeHtml5InstanceResource and
FakeURLLoaderInstanceResource?

But I'm also OK with keeping them anonymous; there's not much duplicated code.

[chanpatorikku, 2016-10-28 11:07:58]

Description was changed from

==========
[NaCl SDK] Refactor FakeFileSystemInterface.

Move FakeFileSystemInterface from fake_pepper_interface_html5_fs.{cc,h} to
fake_file_system_interface.{cc,h} to share the code with the future code.
depot_tools/clang-format was run so the formatting of code is accepted
in the code review by policy. It was run in all the {.cc,.h} files.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk
==========

to

==========
[NaCl SDK] Refactor FakeFileSystemInterface.

Move FakeFileSystemInterface from fake_pepper_interface_html5_fs.{cc,h} to
fake_file_system_interface.{cc,h} to share the code with the future code.
depot_tools/clang-format was run so the formatting of code is accepted
in the code review by policy. It was run in all the {.cc,.h} files, 
and the code formats of some files were changed.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk
==========

[chanpatorikku, 2016-10-28 12:45:29]

On 2016/10/27 21:26:09, binji wrote:
> On 2016/10/27 17:50:42, Sam Clegg wrote:
> > On 2016/10/27 17:45:50, chanpatorikku wrote:
> > > Again, me nad the refactored code.  PTAL.
> > > 
> > >
> >
>
https://codereview.chromium.org/2455973003/diff/1/native_client_sdk/src/tests...
> > > File
> > >
> >
>
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_file_system_interface.cc
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2455973003/diff/1/native_client_sdk/src/tests...
> > >
> >
>
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_file_system_interface.cc:16:
> > > class FakeInstanceResource : public FakeResource {
> > > FakeInstanceResource used to be used by FakePepperInterfaceHtml5Fs and
> > > FakeFileSystemInterface in fake_pepper_interface_html5_fs.cc.
> > > 
> > > After refactoring, two exact copies of FakeInstanceResource are in the
> unnamed
> > > namespaces of fake_pepper_interface_html5_fs.cc and
> > > fake_file_system_interface.cc.
> > > 
> > > Moving the two exact copies into one copy in, say, fake_util.h results in
a
> > > compiler error of FakeInstanceResource being ambiguous because there's
> another
> > > class FakeInstanceResource with a different definition in
> > > fake_pepper_interface_url_loader.cc:
> > > 
> > > class FakeInstanceResource : public FakeResource {
> > >  public:
> > >   FakeInstanceResource() : server_template(NULL) {}
> > >   static const char* classname() { return "FakeInstanceResource"; }
> > > 
> > >   FakeURLLoaderServer* server_template;  // Weak reference.
> > > };
> > > 
> > > To prevent the compiler error, two exact copies of FakeInstanceResource
are
> in
> > > the unnamed namespaces of FakePepperInterfaceHtml5Fs.cc and
> > > FakeFileSystemInterface.cc.
> > 
> > I would just come up with a better name of the FakeInstanceResource that you
> > want to put in a header file.  No need to duplicate it.
> 
> Yeah, that would be ok: maybe FakeHtml5InstanceResource and
> FakeURLLoaderInstanceResource?
> 
> But I'm also OK with keeping them anonymous; there's not much duplicated code.

Trouble on renaming in this review.  Here's the change in patch set 2.

An instance of FakeFileSystemResource is created by FakeFileSystemInterface.  To
follow the pattern, the InstanceResource of FakeFileSystemInterface and
FakePepperInterfaceHtml5Fs, created by FakePepperInterfaceHtml5Fs, is renamed to
PepperInterfaceFsInstanceResource.

InstanceResource is not renamed to FakePepperInterfaceHtml5InstanceResource
because InstanceResource is for any Pepper Interface Filesystem of nacl_io, not
just the Html5 type.

The future code is going to add another Pepper Interface Filesystem of nacl_io,
which can also create an instance of InstanceResource.

FakeURLLoaderInstanceResource can be done in another CL because it's out of
scope.

[chanpatorikku, 2016-10-28 16:17:09]

chanpatorikku@gmail.com changed reviewers:
+ binji@chromium.org, sbc@chromium.org

[chanpatorikku, 2016-10-31 12:33:37]

When you're not busy, ptal.

[Sam Clegg, 2016-10-31 17:26:36]

https://codereview.chromium.org/2455973003/diff/20001/native_client_sdk/src/t...
File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_util.h (right):

https://codereview.chromium.org/2455973003/diff/20001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_util.h:35: class
FakePepperInterfaceFsInstanceResource : public FakeResource {
Can we make this less of a mouthful?

How about just FakeHtml5FsResource?

[chanpatorikku, 2016-11-01 11:57:48]

My apology.  That was weird that I checked and saw InstanceResource to be
created by FakePepperInterfaceHtml5Fs and FakePepperInterfaceGoogleDriveFs,
which is for unit tests of another Fs to be exposed to nacl_io, in the future
code even though it was not.  InstanceResource was created by only
FakePepperInterfaceHtml5Fs in the future code I'm going to solicit for a review.
 The previous comment about FakePepperInterfaceFsInstanceResource created for
multiple Fs's isn't a concern right now.

ptal.

https://codereview.chromium.org/2455973003/diff/20001/native_client_sdk/src/t...
File native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_util.h (right):

https://codereview.chromium.org/2455973003/diff/20001/native_client_sdk/src/t...
native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_util.h:35: class
FakePepperInterfaceFsInstanceResource : public FakeResource {
On 2016/10/31 17:26:36, Sam Clegg wrote:
> Can we make this less of a mouthful?
> 
> How about just FakeHtml5FsResource?

Yes.  Changed to FakeHtml5FsResource.

[Sam Clegg, 2016-11-01 16:17:34]

The CQ bit was checked by sbc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 16:17:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sam Clegg, 2016-11-01 16:18:35]

lgtm

[commit-bot: I haz the power, 2016-11-01 18:05:03]

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

[commit-bot: I haz the power, 2016-11-01 18:05:03]

Dry run: This issue passed the CQ dry run.

[chanpatorikku, 2016-11-02 02:26:05]

The CQ bit was checked by chanpatorikku@gmail.com

[commit-bot: I haz the power, 2016-11-02 02:26:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 02:30:39]

Description was changed from

==========
[NaCl SDK] Refactor FakeFileSystemInterface.

Move FakeFileSystemInterface from fake_pepper_interface_html5_fs.{cc,h} to
fake_file_system_interface.{cc,h} to share the code with the future code.
depot_tools/clang-format was run so the formatting of code is accepted
in the code review by policy. It was run in all the {.cc,.h} files, 
and the code formats of some files were changed.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk
==========

to

==========
[NaCl SDK] Refactor FakeFileSystemInterface.

Move FakeFileSystemInterface from fake_pepper_interface_html5_fs.{cc,h} to
fake_file_system_interface.{cc,h} to share the code with the future code.
depot_tools/clang-format was run so the formatting of code is accepted
in the code review by policy. It was run in all the {.cc,.h} files, 
and the code formats of some files were changed.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk
==========

[commit-bot: I haz the power, 2016-11-02 02:30:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-02 02:32:42]

Description was changed from

==========
[NaCl SDK] Refactor FakeFileSystemInterface.

Move FakeFileSystemInterface from fake_pepper_interface_html5_fs.{cc,h} to
fake_file_system_interface.{cc,h} to share the code with the future code.
depot_tools/clang-format was run so the formatting of code is accepted
in the code review by policy. It was run in all the {.cc,.h} files, 
and the code formats of some files were changed.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk
==========

to

==========
[NaCl SDK] Refactor FakeFileSystemInterface.

Move FakeFileSystemInterface from fake_pepper_interface_html5_fs.{cc,h} to
fake_file_system_interface.{cc,h} to share the code with the future code.
depot_tools/clang-format was run so the formatting of code is accepted
in the code review by policy. It was run in all the {.cc,.h} files,
and the code formats of some files were changed.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_nacl_sdk;master.tryserver.chromium.mac:mac_nacl_sdk;master.tryserver.chromium.win:win_nacl_sdk

Committed: https://crrev.com/9b7e1ceacd7932dc4b1d129bf5245c9de381eb9d
Cr-Commit-Position: refs/heads/master@{#429186}
==========

[commit-bot: I haz the power, 2016-11-02 02:32:43]

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