[NaCl SDK] Implement 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
new file mode 100644
index 0000000000000000000000000000000000000000..d540274ebdccedfdba9e9b9b3559e038fa3d6a01
--- /dev/null
+++ b/native_client_sdk/src/tests/nacl_io_test/fake_ppapi/fake_pepper_interface_googledrivefs.cc
@@ -0,0 +1,312 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "fake_ppapi/fake_pepper_interface_googledrivefs.h"
+
+#include <ppapi/c/pp_completion_callback.h>
+#include <ppapi/c/pp_errors.h>
+
+#include "gtest/gtest.h"
+
+#include "fake_ppapi/fake_core_interface.h"
+#include "fake_ppapi/fake_file_ref_interface.h"
+#include "fake_ppapi/fake_pepper_interface_url_loader.h"
+#include "fake_ppapi/fake_util.h"
+#include "fake_ppapi/fake_var_interface.h"
+
+class FakeDriveInstanceResource : public FakeResource {
+ public:
+  FakeDriveInstanceResource() : server_template(NULL) {}
+  static const char* classname() { return "FakeDriveInstanceResource"; }
+
+  FakeGoogleDriveServer* server_template;
+};
+
+class FakeDriveURLLoaderResource : public FakeResource {
+ public:
+  FakeDriveURLLoaderResource() : manager(NULL), server(NULL), response(0) {}
+
+  virtual void Destroy() {
+    EXPECT_TRUE(manager != NULL);
+    if (response != 0) {
+      manager->Release(response);
+    }
+  }
+
+  static const char* classname() { return "FakeDriveURLLoaderResource"; }
+
+  FakeResourceManager* manager;
+  FakeGoogleDriveServer* server;
+  PP_Resource response;
+  std::string response_body;

[binji, 2016/12/02 22:30:38]

Why not store the response_body in the response?

[chanpatorikku, 2016/12/05 15:05:44]

If response_body is stored in the response, FinishStreamingToFile() does not
have to be called.

Open() can store response_body to the response, and the regular calls from
Open(), FinishStreamingToFile(), GetResponseInfo(), to GetBodyAsFileRef() are
not being simulated.

FinishStreamingToFile() is used to wait for the response body to be completely
downloaded, so storing response_body in the loader first and copying
response_body to the response can simulate the download of the response body.

[binji, 2016/12/08 20:35:13]

It doesn't seem as though response_body is actually used. GetBodyAsFileRef uses
the response's body directly.

The reason I mention this, is that I'd prefer if the response didn't have
references to the loader and request, it doesn't seem like it is necessary, and
it over-complicates the code.

[chanpatorikku, 2016/12/09 14:37:36]

response_body isn't actually used in the current patch, but is going to be used
in the future code. 
The future code, which changes Respond() to be serving different requests so
response_body is set from  FinishStreamingToFile(), GetBodyAsFileRef(), Open(),
and Respond() (in alphabetical order not in function call time order), includes
more code of implementing FakeGoogleDriveServer and the GoogleDriveFs nacl_io
library code.  The patch set is going to contain a few times more lines of code
if it contains code of using response_body.  

response has to have a reference to loader to implement GetBodyAsFileRef(),
although it does not have to have a reference to request

The description of GetBodyAsFileRef() reveals that:

This file is only valid if PP_URLREQUESTPROPERTY_STREAMTOFILE was set on the
URLRequestInfo used to produce this response. This file remains valid until the
URLLoader associated with this URLResponseInfo is closed or destroyed.

(https://developer.chrome.com/native-client/pepper_stable/c/struct_p_p_b___u_r...)

response has to have a reference to loader to see if 'server' is set as NULL in
loader, if the URLLoader has been closed or destroyed already, and if the file
is still valid

response does not have to have a reference to request and is able to have
another variable 'stream_to_file_set' in response, which is a copy of
'stream_to_file' in request.

I am going to remove the reference of request and add 'stream_to_file_set' in
patch set 3.  Shouldn't be a complex change.

+};
+
+class FakeDriveResponseResource : public FakeURLResponseInfoResource {
+ public:
+  FakeDriveResponseResource() : loader(0), request(0), file_ref(0) {}
+  static const char* classname() { return "FakeDriveResponseResource"; }
+
+  std::string body;
+  PP_Resource loader;

[binji, 2016/12/02 22:30:38]

Why doesn't the response own references to these resources?

[chanpatorikku, 2016/12/05 15:05:44]

Not a reason why.

In the current patch set, the response own references to these resources, and a
reference returned from GetBodyAsFileRef() is owned by the caller.

If the response, which was not owning references, gives you a not very good
experience on the review, sorry.  Some of the test codes look to be written in a
higher coding speed perhaps because the test code is for internal use.  I get
how almost all (if not all) related nacl_io codes work for googlefilesystemfs
but not the test codes.  I just read thoroughly on how reference counting in
PP_Resource works. I can incrementally update on figuring how all related test
codes work from now if the experience was not very good for you.

+  PP_Resource request;
+  PP_Resource file_ref;
+};
+
+FakeGoogleDriveServer::FakeGoogleDriveServer() {}
+
+void FakeGoogleDriveServer::Respond(
+    const std::string& url,
+    const std::string& headers,
+    const std::string& method,
+    const std::string& body,
+    FakeGoogleDriveServerResponse* out_response) {
+  out_response->status_code = STATUSCODE_NOT_IMPLEMENTED;
+}
+
+FakeDriveURLLoaderInterface::FakeDriveURLLoaderInterface(
+    FakeCoreInterface* core_interface)
+    : FakeURLLoaderInterface(core_interface) {}
+
+PP_Resource FakeDriveURLLoaderInterface::Create(PP_Instance instance) {
+  FakeDriveInstanceResource* drive_instance_resource =
+      core_interface_->resource_manager()->Get<FakeDriveInstanceResource>(
+          instance);
+  if (drive_instance_resource == NULL)
+    return 0;
+
+  FakeDriveURLLoaderResource* drive_loader_resource =
+      new FakeDriveURLLoaderResource;
+  drive_loader_resource->manager = core_interface_->resource_manager();
+  drive_loader_resource->server = drive_instance_resource->server_template;
+
+  return CREATE_RESOURCE(core_interface_->resource_manager(),
+                         FakeDriveURLLoaderResource, drive_loader_resource);
+}
+
+int32_t FakeDriveURLLoaderInterface::Open(PP_Resource loader,
+                                          PP_Resource request,

[chanpatorikku, 2016/12/02 13:23:33]

The .cc function signature names the 2ndparameter as request while the .h
function signature names the 2ndparameter as request_info.  The parameters may
be better renamed.

.cc function signature:

int32_t FakeDriveURLLoaderInterface::Open(PP_Resource loader,
                                          PP_Resource request,
　　　　　　　　　　　　                    PP_CompletionCallback callback);

.h function signature:

virtual int32_t Open(PP_Resource loader,
                     PP_Resource request_info,
                     PP_CompletionCallback callback);

[binji, 2016/12/02 22:30:38]

Feel free to fix, but it's common that the declaration parameter name doesn't
match the definition parameter name.

[chanpatorikku, 2016/12/05 15:05:44]

Not gonna fix because it's common and the code looks to be maintainable and
understandable.

+                                          PP_CompletionCallback callback) {
+  FakeDriveURLLoaderResource* drive_loader_resource =
+      core_interface_->resource_manager()->Get<FakeDriveURLLoaderResource>(
+          loader);
+  if (drive_loader_resource == NULL)
+    return PP_ERROR_BADRESOURCE;
+
+  // Close() has been called. Invalid to call Open() again.
+  if (drive_loader_resource->server == NULL)
+    return PP_ERROR_FAILED;
+
+  FakeURLRequestInfoResource* request_resource =
+      core_interface_->resource_manager()->Get<FakeURLRequestInfoResource>(
+          request);
+  if (request_resource == NULL)
+    return PP_ERROR_BADRESOURCE;
+
+  // Create a response resource.
+  FakeDriveResponseResource* drive_response_resource =
+      new FakeDriveResponseResource;
+  drive_loader_resource->response =
+      CREATE_RESOURCE(core_interface_->resource_manager(),
+                      FakeDriveResponseResource, drive_response_resource);
+
+  drive_response_resource->loader = loader;
+  drive_response_resource->request = request;
+
+  FakeGoogleDriveServerResponse server_response;
+  drive_loader_resource->server->Respond(
+      request_resource->url, request_resource->headers,
+      request_resource->method, request_resource->body, &server_response);
+
+  drive_response_resource->status_code = server_response.status_code;
+  drive_loader_resource->response_body = server_response.body;
+
+  // Call the callback.
+  return RunCompletionCallback(&callback, PP_OK);
+}
+
+PP_Resource FakeDriveURLLoaderInterface::GetResponseInfo(PP_Resource loader) {
+  FakeDriveURLLoaderResource* drive_loader_resource =
+      core_interface_->resource_manager()->Get<FakeDriveURLLoaderResource>(
+          loader);
+  if (drive_loader_resource == NULL)
+    return 0;
+
+  if (drive_loader_resource->response == 0) {

[binji, 2016/12/02 22:30:38]

nit: please be consistent with braces around single statements. Here and
elsewhere in the CL.

[chanpatorikku, 2016/12/05 15:05:44]

Done.  About being consistent with braces around single statements on the
changed codes of the previous CL's, please let me make the change in another CL.

+    return 0;
+  }
+
+  // Returned resources have an implicit AddRef.
+  core_interface_->resource_manager()->AddRef(drive_loader_resource->response);
+  return drive_loader_resource->response;
+}
+
+int32_t FakeDriveURLLoaderInterface::FinishStreamingToFile(
+    PP_Resource loader,
+    PP_CompletionCallback callback) {
+  FakeDriveURLLoaderResource* drive_loader_resource =
+      core_interface_->resource_manager()->Get<FakeDriveURLLoaderResource>(
+          loader);
+  if (drive_loader_resource == NULL)
+    return PP_ERROR_BADRESOURCE;
+
+  FakeDriveResponseResource* drive_response_resource =
+      core_interface_->resource_manager()->Get<FakeDriveResponseResource>(
+          drive_loader_resource->response);
+  if (drive_response_resource == NULL)
+    return PP_ERROR_BADRESOURCE;
+
+  FakeURLRequestInfoResource* request_resource =
+      core_interface_->resource_manager()->Get<FakeURLRequestInfoResource>(
+          drive_response_resource->request);
+  if (request_resource == NULL)
+    return PP_ERROR_BADRESOURCE;
+
+  if (!request_resource->stream_to_file) {
+    return PP_ERROR_FAILED;
+  }
+
+  drive_response_resource->body = drive_loader_resource->response_body;
+
+  // PPB_URLLoader::FinishStreamingToFile(..) returns 0 even when
+  // the response body size is > 0.
+  return 0;
+}
+
+void FakeDriveURLLoaderInterface::Close(PP_Resource loader) {
+  FakeDriveURLLoaderResource* drive_loader_resource =
+      core_interface_->resource_manager()->Get<FakeDriveURLLoaderResource>(
+          loader);
+  if (drive_loader_resource == NULL)
+    return;
+
+  core_interface_->resource_manager()->Release(drive_loader_resource->response);
+
+  drive_loader_resource->server = NULL;
+  drive_loader_resource->response = 0;
+  drive_loader_resource->response_body.clear();
+}
+
+FakeDriveURLResponseInfoInterface::FakeDriveURLResponseInfoInterface(
+    FakeCoreInterface* core_interface,
+    FakeVarInterface* var_interface,
+    FakeFileRefInterface* file_ref_interface)
+    : FakeURLResponseInfoInterface(core_interface, var_interface),
+      file_ref_interface_(file_ref_interface) {
+  FakeFileSystemResource* file_system_resource = new FakeFileSystemResource;
+  file_system_resource->filesystem = new FakeFilesystem();
+  file_system_resource->opened = true;
+
+  filesystem_resource_ =
+      CREATE_RESOURCE(core_interface_->resource_manager(),
+                      FakeFileSystemResource, file_system_resource);
+}
+
+FakeDriveURLResponseInfoInterface::~FakeDriveURLResponseInfoInterface() {
+  core_interface_->ReleaseResource(filesystem_resource_);
+}
+
+PP_Resource FakeDriveURLResponseInfoInterface::GetBodyAsFileRef(
+    PP_Resource response) {
+  FakeDriveResponseResource* drive_response_resource =
+      core_interface_->resource_manager()->Get<FakeDriveResponseResource>(
+          response);
+  if (drive_response_resource == NULL)
+    return 0;
+
+  FakeURLRequestInfoResource* request_resource =
+      core_interface_->resource_manager()->Get<FakeURLRequestInfoResource>(
+          drive_response_resource->request);
+  if (request_resource == NULL)
+    return 0;
+
+  if (!request_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)
+    return 0;
+
+  if (drive_loader_resource->server == NULL) {
+    return 0;
+  }
+
+  if (drive_response_resource->file_ref == 0) {
+    FakeFileSystemResource* file_system_resource =
+        core_interface_->resource_manager()->Get<FakeFileSystemResource>(
+            filesystem_resource_);
+    if (file_system_resource == NULL)
+      return 0;
+
+    char path_buffer[1024];

[binji, 2016/12/02 22:30:38]

nit: 1024 is way overkill here. Even if %u is 64-bit, the max value is
18446744073709551615, which is just 20 chars long.

[chanpatorikku, 2016/12/05 15:05:44]

Changed to path_buffer[32].  I was thinking the stack pointer is going to just
move 1024 and is the same as any buffer size between 21 to 1024.

[binji, 2016/12/08 20:35:12]

Yes, but the stack is not limitless, so it is best to keep it small if you can.

[chanpatorikku, 2016/12/09 14:37:36]

The stack is about a few megabytes.  It is small compared to the heap.

+    snprintf(path_buffer, sizeof(path_buffer), "%u", response);
+
+    FakeNode* fake_node;
+    if (!file_system_resource->filesystem->AddFile(
+            path_buffer, drive_response_resource->body, &fake_node)) {
+      return 0;
+    }
+
+    drive_response_resource->file_ref =
+        file_ref_interface_->Create(filesystem_resource_, path_buffer);
+  }
+
+  return drive_response_resource->file_ref;
+}
+
+FakePepperInterfaceGoogleDriveFs::FakePepperInterfaceGoogleDriveFs()
+    : core_interface_(&resource_manager_),
+      var_interface_(&var_manager_),
+      file_ref_interface_(&core_interface_, &var_interface_),
+      drive_url_loader_interface_(&core_interface_),
+      url_request_info_interface_(&core_interface_, &var_interface_),
+      drive_url_response_info_interface_(&core_interface_,
+                                         &var_interface_,
+                                         &file_ref_interface_) {
+  Init();
+}
+
+void FakePepperInterfaceGoogleDriveFs::Init() {
+  FakeDriveInstanceResource* drive_instance_resource =
+      new FakeDriveInstanceResource;
+  drive_instance_resource->server_template = &google_drive_server_template_;
+
+  instance_ =
+      CREATE_RESOURCE(core_interface_.resource_manager(),
+                      FakeDriveInstanceResource, drive_instance_resource);
+}
+
+FakePepperInterfaceGoogleDriveFs::~FakePepperInterfaceGoogleDriveFs() {
+  core_interface_.ReleaseResource(instance_);
+}
+
+nacl_io::CoreInterface* FakePepperInterfaceGoogleDriveFs::GetCoreInterface() {
+  return &core_interface_;
+}
+
+nacl_io::FileRefInterface*
+FakePepperInterfaceGoogleDriveFs::GetFileRefInterface() {
+  return &file_ref_interface_;
+}
+
+nacl_io::URLLoaderInterface*
+FakePepperInterfaceGoogleDriveFs::GetURLLoaderInterface() {
+  return &drive_url_loader_interface_;
+}
+
+nacl_io::URLRequestInfoInterface*
+FakePepperInterfaceGoogleDriveFs::GetURLRequestInfoInterface() {
+  return &url_request_info_interface_;
+}
+
+nacl_io::URLResponseInfoInterface*
+FakePepperInterfaceGoogleDriveFs::GetURLResponseInfoInterface() {
+  return &drive_url_response_info_interface_;
+}
+
+nacl_io::VarInterface* FakePepperInterfaceGoogleDriveFs::GetVarInterface() {
+  return &var_interface_;
+}