webkit/plugins/ppapi/ppb_file_ref_impl.cc - Issue 13726024: Refactor FileSystem

Unified Diff: webkit/plugins/ppapi/ppb_file_ref_impl.cc

Issue 13726024: Refactor FileSystem (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 7 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: webkit/plugins/ppapi/ppb_file_ref_impl.cc

diff --git a/webkit/plugins/ppapi/ppb_file_ref_impl.cc b/webkit/plugins/ppapi/ppb_file_ref_impl.cc

index dfaa56919bbc8003d38a26d87a0f8b2f04b4b9da..a352647308a84361d0652ca670c666d66ba8332f 100644

--- a/webkit/plugins/ppapi/ppb_file_ref_impl.cc

+++ b/webkit/plugins/ppapi/ppb_file_ref_impl.cc

@@ -10,6 +10,7 @@

#include "googleurl/src/gurl.h"

#include "net/base/escape.h"

#include "ppapi/c/pp_errors.h"

+#include "ppapi/c/pp_file_info.h"

yzshen1 2013/04/08 21:05:16 This include is already in the .h file. You don't

victorhsieh 2013/04/08 23:44:38 Done.

#include "ppapi/shared_impl/file_type_conversion.h"

#include "ppapi/shared_impl/time_conversion.h"

#include "ppapi/shared_impl/var.h"

@@ -20,7 +21,6 @@

#include "webkit/plugins/ppapi/plugin_delegate.h"

#include "webkit/plugins/ppapi/plugin_module.h"

#include "webkit/plugins/ppapi/ppapi_plugin_instance.h"

-#include "webkit/plugins/ppapi/ppb_file_system_impl.h"

#include "webkit/plugins/ppapi/resource_helper.h"

using ppapi::HostResource;

@@ -149,7 +149,7 @@ void QueryCallback(scoped_refptr<base::TaskRunner> task_runner,

} // namespace

PPB_FileRef_Impl::PPB_FileRef_Impl(const PPB_FileRef_CreateInfo& info,

- PPB_FileSystem_Impl* file_system)

+ PP_Resource file_system)

: PPB_FileRef_Shared(::ppapi::OBJECT_IS_IMPL, info),

file_system_(file_system),

external_file_system_path_() {

@@ -166,25 +166,24 @@ PPB_FileRef_Impl::~PPB_FileRef_Impl() {

}

// static

-PPB_FileRef_Impl* PPB_FileRef_Impl::CreateInternal(PP_Resource pp_file_system,

+PPB_FileRef_Impl* PPB_FileRef_Impl::CreateInternal(PP_Instance instance,

+ PP_Resource pp_file_system,

const std::string& path) {

- EnterResourceNoLock<PPB_FileSystem_API> enter(pp_file_system, true);

- if (enter.failed())

- return 0;

- PPB_FileSystem_Impl* file_system =

- static_cast<PPB_FileSystem_Impl*>(enter.object());

- if (!file_system->pp_instance())

+ PluginInstance* plugin_instance =

+ ResourceHelper::PPInstanceToPluginInstance(instance);

+ if (!plugin_instance || !plugin_instance->delegate())

return 0;

- if (file_system->type() != PP_FILESYSTEMTYPE_LOCALPERSISTENT &&

- file_system->type() != PP_FILESYSTEMTYPE_LOCALTEMPORARY &&

- file_system->type() != PP_FILESYSTEMTYPE_EXTERNAL)

+ PP_FileSystemType type =

+ plugin_instance->delegate()->GetFileSystemType(instance, pp_file_system);

+ if (type != PP_FILESYSTEMTYPE_LOCALPERSISTENT &&

+ type != PP_FILESYSTEMTYPE_LOCALTEMPORARY &&

+ type != PP_FILESYSTEMTYPE_EXTERNAL)

return 0;

PPB_FileRef_CreateInfo info;

- info.resource = HostResource::MakeInstanceOnly(file_system->pp_instance());

- info.file_system_type = file_system->type();

+ info.resource = HostResource::MakeInstanceOnly(instance);

+ info.file_system_type = type;

// Validate the path.

info.path = path;

@@ -194,7 +193,7 @@ PPB_FileRef_Impl* PPB_FileRef_Impl::CreateInternal(PP_Resource pp_file_system,

info.name = GetNameForVirtualFilePath(info.path);

- return new PPB_FileRef_Impl(info, file_system);

+ return new PPB_FileRef_Impl(info, pp_file_system);

}

// static

@@ -229,7 +228,7 @@ PP_Resource PPB_FileRef_Impl::GetParent() {

std::string parent_path = virtual_path.substr(0, pos);

scoped_refptr<PPB_FileRef_Impl> parent_ref(

- CreateInternal(file_system_->pp_resource(), parent_path));

+ CreateInternal(pp_instance(), file_system_, parent_path));

if (!parent_ref.get())

return 0;

return parent_ref->GetReference();

@@ -246,7 +245,7 @@ int32_t PPB_FileRef_Impl::MakeDirectory(

return PP_ERROR_FAILED;

if (!plugin_instance->delegate()->MakeDirectory(

GetFileSystemURL(), PP_ToBool(make_ancestors),

- new FileCallbacks(this, callback, NULL, NULL)))

+ new FileCallbacks(this, callback, NULL)))

return PP_ERROR_FAILED;

return PP_OK_COMPLETIONPENDING;

}

@@ -264,7 +263,7 @@ int32_t PPB_FileRef_Impl::Touch(PP_Time last_access_time,

GetFileSystemURL(),

PPTimeToTime(last_access_time),

PPTimeToTime(last_modified_time),

- new FileCallbacks(this, callback, NULL, NULL)))

+ new FileCallbacks(this, callback, NULL)))

return PP_ERROR_FAILED;

return PP_OK_COMPLETIONPENDING;

}

@@ -278,7 +277,7 @@ int32_t PPB_FileRef_Impl::Delete(scoped_refptr<TrackedCallback> callback) {

return PP_ERROR_FAILED;

if (!plugin_instance->delegate()->Delete(

GetFileSystemURL(),

- new FileCallbacks(this, callback, NULL, NULL)))

+ new FileCallbacks(this, callback, NULL)))

return PP_ERROR_FAILED;

return PP_OK_COMPLETIONPENDING;

}

@@ -292,7 +291,7 @@ int32_t PPB_FileRef_Impl::Rename(PP_Resource new_pp_file_ref,

static_cast<PPB_FileRef_Impl*>(enter.object());

if (!IsValidNonExternalFileSystem() ||

- file_system_.get() != new_file_ref->file_system_.get())

+ file_system_ != new_file_ref->file_system_)

return PP_ERROR_NOACCESS;

// TODO(viettrungluu): Also cancel when the new file ref is destroyed?

@@ -302,7 +301,7 @@ int32_t PPB_FileRef_Impl::Rename(PP_Resource new_pp_file_ref,

return PP_ERROR_FAILED;

if (!plugin_instance->delegate()->Rename(

GetFileSystemURL(), new_file_ref->GetFileSystemURL(),

- new FileCallbacks(this, callback, NULL, NULL)))

+ new FileCallbacks(this, callback, NULL)))

return PP_ERROR_FAILED;

return PP_OK_COMPLETIONPENDING;

}

@@ -340,17 +339,30 @@ GURL PPB_FileRef_Impl::GetFileSystemURL() const {

// We need to trim off the '/' before calling Resolve, as FileSystem URLs

// start with a storage type identifier that looks like a path segment.

- return file_system_->root_url().Resolve(

- net::EscapePath(virtual_path.substr(1)));

+ PluginInstance* plugin_instance = ResourceHelper::GetPluginInstance(this);

+ PluginDelegate* delegate =

+ plugin_instance ? plugin_instance->delegate() : NULL;

+ if (!delegate)

+ return GURL();

+ return GURL(delegate->GetFileSystemRootUrl(pp_instance(), file_system_))

+ .Resolve(net::EscapePath(virtual_path.substr(1)));

}

-bool PPB_FileRef_Impl::HasValidFileSystem() const {

- return file_system_ && file_system_->opened();

+bool PPB_FileRef_Impl::IsValidNonExternalFileSystem() const {

+ PluginInstance* plugin_instance = ResourceHelper::GetPluginInstance(this);

+ PluginDelegate* delegate =

+ plugin_instance ? plugin_instance->delegate() : NULL;

+ return delegate &&

+ delegate->IsFileSystemOpened(pp_instance(), file_system_) &&

+ delegate->GetFileSystemType(pp_instance(), file_system_) !=

+ PP_FILESYSTEMTYPE_EXTERNAL;

}

-bool PPB_FileRef_Impl::IsValidNonExternalFileSystem() const {

- return file_system_ && file_system_->opened() &&

- file_system_->type() != PP_FILESYSTEMTYPE_EXTERNAL;

+bool PPB_FileRef_Impl::HasValidFileSystem() const {

+ PluginInstance* plugin_instance = ResourceHelper::GetPluginInstance(this);

+ PluginDelegate* delegate =

+ plugin_instance ? plugin_instance->delegate() : NULL;

+ return delegate && delegate->IsFileSystemOpened(pp_instance(), file_system_);

}

int32_t PPB_FileRef_Impl::Query(PP_FileInfo* info,

@@ -377,9 +389,16 @@ int32_t PPB_FileRef_Impl::Query(PP_FileInfo* info,

if (!HasValidFileSystem())

return PP_ERROR_NOACCESS;

+ PluginInstance* plugin_instance = ResourceHelper::GetPluginInstance(this);

+ PluginDelegate* delegate =

+ plugin_instance ? plugin_instance->delegate() : NULL;

+ DCHECK(delegate);

yzshen1 2013/04/08 21:05:16 Why we DCHECK here while testing NULL elsewhere?

victorhsieh 2013/04/08 23:44:38 I don't know. Fixed :P

if (!plugin_instance->delegate()->Query(

GetFileSystemURL(),

- new FileCallbacks(this, callback, info, file_system_)))

+ new FileCallbacks(this, callback, info,

+ delegate->GetFileSystemType(pp_instance(),

+ file_system_))))

return PP_ERROR_FAILED;

}

« webkit/plugins/ppapi/ppb_file_ref_impl.h ('K') | « webkit/plugins/ppapi/ppb_file_ref_impl.h ('k') | webkit/plugins/ppapi/ppb_file_system_impl.h » ('j') | no next file with comments »