[NaCl SDK] Add more support and fix FakePepperInterfaceGoogleDriveFs

native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc

Index: native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc
diff --git a/native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc b/native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc
index d0a14a39d132c4b01b6640acf791ab5398c62586..a280312bf6a819066dc8f9257324463c5f80704a 100644
--- a/native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc
+++ b/native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc
@@ -44,13 +44,13 @@ class FakeDriveURLLoaderResource : public FakeResource {
 class FakeDriveResponseResource : public FakeURLResponseInfoResource {
  public:
   FakeDriveResponseResource()
-      : manager(NULL), loader(0), file_ref(0), stream_to_file(false) {}
+      : manager(NULL),
+        loader_resource(NULL),
+        file_ref(0),
+        stream_to_file(false) {}
 
   virtual void Destroy() {
     EXPECT_TRUE(manager != NULL);
-    if (loader != 0)
-      manager->Release(loader);
-
     if (file_ref != 0)
       manager->Release(file_ref);
   }
@@ -58,7 +58,7 @@ class FakeDriveResponseResource : public FakeURLResponseInfoResource {
   static const char* classname() { return "FakeDriveResponseResource"; }
 
   FakeResourceManager* manager;
-  PP_Resource loader;
+  FakeDriveURLLoaderResource* loader_resource;
   PP_Resource file_ref;

[chanpatorikku, 2016/12/13 16:03:18]

The previous code "PP_Resource loader" will make the destructor of
FakeResourceManager showing a message because loader and response reference each
other.

In the previous review, I wrote that I read thoroughly on how reference counting
in PP_Resource works. I read Create() from FakeResourceManager, AddRef() &
Release() from FakeResourceManager & FakeCoreInterface, and Release() from
ScopedResource.  I didn't know about the destructor of FakeResourceManager
showing a message when the reference counts of some resources are > 0.  If you
had a not very good experience on reviewing the previous code "PP_Resource
loader", sorry.

The fix changes "PP_Resource loader" to "FakeDriveURLLoaderResource*
loader_resource".  loader_resource may store a dangling pointer when the
FakeDriveURLLoaderResource instance has been freed.  The code is test code and
it can be coded so a dangling pointer is not going to arise.

[chanpatorikku, 2016/12/14 15:56:47]

Other than "FakeDriveURLLoaderResource* loader_resource", there're 2 other ways
of handling how to check to see if the corresponding loader_resource has been
closed or destroyed.

The 1st way is to convert back to "PP_Resource loader" but do not call AddRef()
on the "loader" in Open().  The code may be less understandable when the
response has "PP_Resource file_ref" that owns a reference but "PP_Resource
loader" that does not own a reference.

The 2nd way is to create 2 booleans "bool loader_destroyed; bool loader_closed"
in the response.  Both booleans are set as true in the constructor of the
response.  In Open(), both booleans are set to false after the response resource
is created.  In Close(), "loader_closed" is set to true.  In the Destroy() of
loader, the response is gotten from Get(), and "loader_destroyed" is set to
true.  In GetBodyAsFileRef(), either "loader_destroyed == true" or
"loader_closed == true" is going to let 0 to be returned.

   bool stream_to_file;
   std::string body;
@@ -122,9 +122,7 @@ int32_t FakeDriveURLLoaderInterface::Open(PP_Resource loader,
       CREATE_RESOURCE(core_interface_->resource_manager(),
                       FakeDriveResponseResource, drive_response_resource);
 
-  drive_response_resource->loader = loader;
-  core_interface_->resource_manager()->AddRef(drive_response_resource->loader);
-
+  drive_response_resource->loader_resource = drive_loader_resource;
   drive_response_resource->stream_to_file = request_resource->stream_to_file;
 
   FakeGoogleDriveServerResponse server_response;
@@ -193,6 +191,23 @@ void FakeDriveURLLoaderInterface::Close(PP_Resource loader) {
   drive_loader_resource->response_body.clear();
 }
 
+FakeDriveURLRequestInfoInterface::FakeDriveURLRequestInfoInterface(
+    FakeCoreInterface* core_interface,
+    FakeVarInterface* var_interface)
+    : FakeURLRequestInfoInterface(core_interface, var_interface) {}
+
+PP_Resource FakeDriveURLRequestInfoInterface::Create(PP_Instance instance) {
+  FakeDriveInstanceResource* drive_instance_resource =
+      core_interface_->resource_manager()->Get<FakeDriveInstanceResource>(
+          instance);
+  if (drive_instance_resource == NULL)
+    return PP_ERROR_BADRESOURCE;
+
+  return CREATE_RESOURCE(core_interface_->resource_manager(),
+                         FakeURLRequestInfoResource,
+                         new FakeURLRequestInfoResource);
+}
+
 FakeDriveURLResponseInfoInterface::FakeDriveURLResponseInfoInterface(
     FakeCoreInterface* core_interface,
     FakeVarInterface* var_interface,
@@ -212,6 +227,35 @@ FakeDriveURLResponseInfoInterface::~FakeDriveURLResponseInfoInterface() {
   core_interface_->ReleaseResource(filesystem_resource_);
 }
 
+PP_Var FakeDriveURLResponseInfoInterface::GetProperty(
+    PP_Resource response,
+    PP_URLResponseProperty property) {
+  FakeDriveResponseResource* drive_response_resource =
+      core_interface_->resource_manager()->Get<FakeDriveResponseResource>(
+          response);
+  if (drive_response_resource == NULL)
+    return PP_Var();
+
+  switch (property) {
+    case PP_URLRESPONSEPROPERTY_URL:
+      return var_interface_->VarFromUtf8(drive_response_resource->url.data(),
+                                         drive_response_resource->url.size());
+
+    case PP_URLRESPONSEPROPERTY_STATUSCODE:
+      return PP_MakeInt32(drive_response_resource->status_code);
+
+    case PP_URLRESPONSEPROPERTY_HEADERS:
+      return var_interface_->VarFromUtf8(
+          drive_response_resource->headers.data(),
+          drive_response_resource->headers.size());
+    default:
+      EXPECT_TRUE(false) << "Unimplemented property " << property
+                         << " in "
+                            "FakeDriveURLResponseInfoInterface::GetProperty";
+      return PP_Var();
+  }
+}
+
 PP_Resource FakeDriveURLResponseInfoInterface::GetBodyAsFileRef(
     PP_Resource response) {
   FakeDriveResponseResource* drive_response_resource =
@@ -223,13 +267,10 @@ PP_Resource FakeDriveURLResponseInfoInterface::GetBodyAsFileRef(
   if (!drive_response_resource->stream_to_file)
     return 0;
 
-  FakeDriveURLLoaderResource* drive_loader_resource =
-      core_interface_->resource_manager()->Get<FakeDriveURLLoaderResource>(
-          drive_response_resource->loader);
-  if (drive_loader_resource == NULL)
+  if (drive_response_resource->loader_resource == NULL)

[chanpatorikku, 2016/12/14 15:56:47]

"drive_response_resource->loader_resource == NULL" doesn't have to be checked
because drive_response_resource->loader_resource is set to a non-NULL in Open()
and GetBodyAsFileRef() is called after Open().

It was weird about changing from 

FakeDriveURLLoaderResource* drive_loader_resource = 
 
core_interface_->resource_manager()->Get<FakeDriveURLLoaderResource>(drive_response_resource->loader);
if (drive_loader_resource == NULL)
  return 0;

to

drive_response_resource->loader_resource == NULL;

, and while changing the code, i did not do a verygood work on noticing the
difference because i did not understand line by line how Get() worked and
thought both codes just checked to see if loader_resource was able to be
dereferenced.

i just read line by line the related codes of all the related methods modified
in this patch set so it won't happen again.

     return 0;
 
-  if (drive_loader_resource->server == NULL)
+  if (drive_response_resource->loader_resource->server == NULL)
     return 0;
 
   if (drive_response_resource->file_ref == 0) {
@@ -260,9 +301,10 @@ PP_Resource FakeDriveURLResponseInfoInterface::GetBodyAsFileRef(
 FakePepperInterfaceGoogleDriveFs::FakePepperInterfaceGoogleDriveFs()
     : core_interface_(&resource_manager_),
       var_interface_(&var_manager_),
+      file_io_interface_(&core_interface_),
       file_ref_interface_(&core_interface_, &var_interface_),
       drive_url_loader_interface_(&core_interface_),
-      url_request_info_interface_(&core_interface_, &var_interface_),
+      drive_url_request_info_interface_(&core_interface_, &var_interface_),
       drive_url_response_info_interface_(&core_interface_,
                                          &var_interface_,
                                          &file_ref_interface_) {
@@ -287,6 +329,11 @@ nacl_io::CoreInterface* FakePepperInterfaceGoogleDriveFs::GetCoreInterface() {
   return &core_interface_;
 }
 
+nacl_io::FileIoInterface*
+FakePepperInterfaceGoogleDriveFs::GetFileIoInterface() {
+  return &file_io_interface_;
+}
+
 nacl_io::FileRefInterface*
 FakePepperInterfaceGoogleDriveFs::GetFileRefInterface() {
   return &file_ref_interface_;
@@ -299,7 +346,7 @@ FakePepperInterfaceGoogleDriveFs::GetURLLoaderInterface() {
 
 nacl_io::URLRequestInfoInterface*
 FakePepperInterfaceGoogleDriveFs::GetURLRequestInfoInterface() {
-  return &url_request_info_interface_;
+  return &drive_url_request_info_interface_;
 }
 
 nacl_io::URLResponseInfoInterface*