Commit 3d298014 authored by satorux@chromium.org's avatar satorux@chromium.org

gdata: Fix a crash at deletion of GDataFileSystem on debug build.

GDataFileSystem was a ref counted object, but deleted by
ProfileKeyedServiceFactory with 'delete'. This was horribly
wrong, and resulted in a crash at deletion of GDataFileSystem
that happened at browser shutdown time.

The fix is to make GDataFileSystem non-ref counted.
WeakPtr is used to create callbacks.

Along the way, replaced kGDataAPICallThread with BrowserThread::UI
in gdata.cc. TokenFetcher requires callers to be on UI thread.

BUG=chromium-os:27121
TEST=Run chrome on debug build, open the file manager, show gdata files, Ctrl-C to exit. No crash from GDataFileSystem

Review URL: https://chromiumcodereview.appspot.com/9564002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@124554 0039d316-1c4b-4281-b951-d872f2087c98
parent b006bdf2
...@@ -42,10 +42,6 @@ namespace gdata { ...@@ -42,10 +42,6 @@ namespace gdata {
namespace { namespace {
// All gdata api calls will be initated and processed from this thread.
// TODO(zelidrag): We might want to change this to its own thread
const BrowserThread::ID kGDataAPICallThread = BrowserThread::UI;
// Template for optional OAuth2 authorization HTTP header. // Template for optional OAuth2 authorization HTTP header.
const char kAuthorizationHeaderFormat[] = const char kAuthorizationHeaderFormat[] =
"Authorization: Bearer %s"; "Authorization: Bearer %s";
...@@ -742,7 +738,7 @@ GDataAuthService::~GDataAuthService() { ...@@ -742,7 +738,7 @@ GDataAuthService::~GDataAuthService() {
} }
void GDataAuthService::StartAuthentication(AuthStatusCallback callback) { void GDataAuthService::StartAuthentication(AuthStatusCallback callback) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
(new AuthOperation(profile_, GetOAuth2RefreshToken()))->Start( (new AuthOperation(profile_, GetOAuth2RefreshToken()))->Start(
base::Bind(&gdata::GDataAuthService::OnAuthCompleted, base::Bind(&gdata::GDataAuthService::OnAuthCompleted,
...@@ -753,7 +749,7 @@ void GDataAuthService::StartAuthentication(AuthStatusCallback callback) { ...@@ -753,7 +749,7 @@ void GDataAuthService::StartAuthentication(AuthStatusCallback callback) {
void GDataAuthService::OnAuthCompleted(AuthStatusCallback callback, void GDataAuthService::OnAuthCompleted(AuthStatusCallback callback,
GDataErrorCode error, GDataErrorCode error,
const std::string& auth_token) { const std::string& auth_token) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (error == HTTP_SUCCESS) if (error == HTTP_SUCCESS)
auth_token_ = auth_token; auth_token_ = auth_token;
...@@ -818,7 +814,13 @@ void DocumentsService::Initialize(Profile* profile) { ...@@ -818,7 +814,13 @@ void DocumentsService::Initialize(Profile* profile) {
gdata_auth_service_->Initialize(profile); gdata_auth_service_->Initialize(profile);
} }
base::WeakPtr<DocumentsService> DocumentsService::AsWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
}
void DocumentsService::Authenticate(const AuthStatusCallback& callback) { void DocumentsService::Authenticate(const AuthStatusCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (gdata_auth_service_->IsFullyAuthenticated()) { if (gdata_auth_service_->IsFullyAuthenticated()) {
callback.Run(gdata::HTTP_SUCCESS, gdata_auth_service_->oauth2_auth_token()); callback.Run(gdata::HTTP_SUCCESS, gdata_auth_service_->oauth2_auth_token());
} else if (gdata_auth_service_->IsPartiallyAuthenticated()) { } else if (gdata_auth_service_->IsPartiallyAuthenticated()) {
...@@ -831,7 +833,8 @@ void DocumentsService::Authenticate(const AuthStatusCallback& callback) { ...@@ -831,7 +833,8 @@ void DocumentsService::Authenticate(const AuthStatusCallback& callback) {
void DocumentsService::GetDocuments(const GURL& url, void DocumentsService::GetDocuments(const GURL& url,
const GetDataCallback& callback) { const GetDataCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (!gdata_auth_service_->IsFullyAuthenticated()) { if (!gdata_auth_service_->IsFullyAuthenticated()) {
// Fetch OAuth2 authetication token from the refresh token first. // Fetch OAuth2 authetication token from the refresh token first.
gdata_auth_service_->StartAuthentication( gdata_auth_service_->StartAuthentication(
...@@ -860,7 +863,7 @@ void DocumentsService::GetDocumentsOnAuthRefresh(const GURL& url, ...@@ -860,7 +863,7 @@ void DocumentsService::GetDocumentsOnAuthRefresh(const GURL& url,
const GetDataCallback& callback, const GetDataCallback& callback,
GDataErrorCode error, GDataErrorCode error,
const std::string& token) { const std::string& token) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (error != HTTP_SUCCESS) { if (error != HTTP_SUCCESS) {
if (!callback.is_null()) if (!callback.is_null())
...@@ -875,7 +878,7 @@ void DocumentsService::OnGetDocumentsCompleted(const GURL& url, ...@@ -875,7 +878,7 @@ void DocumentsService::OnGetDocumentsCompleted(const GURL& url,
const GetDataCallback& callback, const GetDataCallback& callback,
GDataErrorCode error, GDataErrorCode error,
base::Value* value) { base::Value* value) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
switch (error) { switch (error) {
case HTTP_UNAUTHORIZED: case HTTP_UNAUTHORIZED:
...@@ -896,7 +899,7 @@ void DocumentsService::OnGetDocumentsCompleted(const GURL& url, ...@@ -896,7 +899,7 @@ void DocumentsService::OnGetDocumentsCompleted(const GURL& url,
void DocumentsService::DownloadDocument(const GURL& document_url, void DocumentsService::DownloadDocument(const GURL& document_url,
DocumentExportFormat format, DocumentExportFormat format,
DownloadActionCallback callback) { DownloadActionCallback callback) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DownloadFile( DownloadFile(
chrome_browser_net::AppendQueryParameter(document_url, chrome_browser_net::AppendQueryParameter(document_url,
...@@ -907,7 +910,7 @@ void DocumentsService::DownloadDocument(const GURL& document_url, ...@@ -907,7 +910,7 @@ void DocumentsService::DownloadDocument(const GURL& document_url,
void DocumentsService::DownloadFile(const GURL& document_url, void DocumentsService::DownloadFile(const GURL& document_url,
DownloadActionCallback callback) { DownloadActionCallback callback) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (!gdata_auth_service_->IsFullyAuthenticated()) { if (!gdata_auth_service_->IsFullyAuthenticated()) {
// Fetch OAuth2 authetication token from the refresh token first. // Fetch OAuth2 authetication token from the refresh token first.
gdata_auth_service_->StartAuthentication( gdata_auth_service_->StartAuthentication(
...@@ -931,7 +934,7 @@ void DocumentsService::DownloadDocumentOnAuthRefresh( ...@@ -931,7 +934,7 @@ void DocumentsService::DownloadDocumentOnAuthRefresh(
const GURL& document_url, const GURL& document_url,
GDataErrorCode error, GDataErrorCode error,
const std::string& token) { const std::string& token) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (error != HTTP_SUCCESS) { if (error != HTTP_SUCCESS) {
if (!callback.is_null()) if (!callback.is_null())
...@@ -947,7 +950,7 @@ void DocumentsService::OnDownloadDocumentCompleted( ...@@ -947,7 +950,7 @@ void DocumentsService::OnDownloadDocumentCompleted(
GDataErrorCode error, GDataErrorCode error,
const GURL& document_url, const GURL& document_url,
const FilePath& file_path) { const FilePath& file_path) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
switch (error) { switch (error) {
case HTTP_UNAUTHORIZED: case HTTP_UNAUTHORIZED:
...@@ -965,7 +968,8 @@ void DocumentsService::OnDownloadDocumentCompleted( ...@@ -965,7 +968,8 @@ void DocumentsService::OnDownloadDocumentCompleted(
void DocumentsService::DeleteDocument(const GURL& document_url, void DocumentsService::DeleteDocument(const GURL& document_url,
EntryActionCallback callback) { EntryActionCallback callback) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (!gdata_auth_service_->IsFullyAuthenticated()) { if (!gdata_auth_service_->IsFullyAuthenticated()) {
// Fetch OAuth2 authetication token from the refresh token first. // Fetch OAuth2 authetication token from the refresh token first.
gdata_auth_service_->StartAuthentication( gdata_auth_service_->StartAuthentication(
...@@ -989,7 +993,7 @@ void DocumentsService::DeleteDocumentOnAuthRefresh( ...@@ -989,7 +993,7 @@ void DocumentsService::DeleteDocumentOnAuthRefresh(
const GURL& document_url, const GURL& document_url,
GDataErrorCode error, GDataErrorCode error,
const std::string& token) { const std::string& token) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (error != HTTP_SUCCESS) { if (error != HTTP_SUCCESS) {
if (!callback.is_null()) if (!callback.is_null())
...@@ -1004,7 +1008,7 @@ void DocumentsService::OnDeleteDocumentCompleted( ...@@ -1004,7 +1008,7 @@ void DocumentsService::OnDeleteDocumentCompleted(
EntryActionCallback callback, EntryActionCallback callback,
GDataErrorCode error, GDataErrorCode error,
const GURL& document_url) { const GURL& document_url) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
switch (error) { switch (error) {
case HTTP_UNAUTHORIZED: case HTTP_UNAUTHORIZED:
...@@ -1022,7 +1026,7 @@ void DocumentsService::OnDeleteDocumentCompleted( ...@@ -1022,7 +1026,7 @@ void DocumentsService::OnDeleteDocumentCompleted(
void DocumentsService::InitiateUpload(const UploadFileInfo& upload_file_info, void DocumentsService::InitiateUpload(const UploadFileInfo& upload_file_info,
InitiateUploadCallback callback) { InitiateUploadCallback callback) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
// If we don't have doucment feed, queue caller of InitiateUpload. // If we don't have doucment feed, queue caller of InitiateUpload.
if (!feed_value_.get()) { if (!feed_value_.get()) {
...@@ -1105,7 +1109,7 @@ void DocumentsService::InitiateUploadOnAuthRefresh( ...@@ -1105,7 +1109,7 @@ void DocumentsService::InitiateUploadOnAuthRefresh(
const UploadFileInfo& upload_file_info, const UploadFileInfo& upload_file_info,
GDataErrorCode error, GDataErrorCode error,
const std::string& token) { const std::string& token) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (error != HTTP_SUCCESS) { if (error != HTTP_SUCCESS) {
if (!callback.is_null()) if (!callback.is_null())
...@@ -1121,7 +1125,7 @@ void DocumentsService::OnInitiateUploadCompleted( ...@@ -1121,7 +1125,7 @@ void DocumentsService::OnInitiateUploadCompleted(
GDataErrorCode error, GDataErrorCode error,
const UploadFileInfo& upload_file_info, const UploadFileInfo& upload_file_info,
const GURL& upload_location) { const GURL& upload_location) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
switch (error) { switch (error) {
case HTTP_UNAUTHORIZED: case HTTP_UNAUTHORIZED:
...@@ -1139,7 +1143,7 @@ void DocumentsService::OnInitiateUploadCompleted( ...@@ -1139,7 +1143,7 @@ void DocumentsService::OnInitiateUploadCompleted(
void DocumentsService::ResumeUpload(const UploadFileInfo& upload_file_info, void DocumentsService::ResumeUpload(const UploadFileInfo& upload_file_info,
ResumeUploadCallback callback) { ResumeUploadCallback callback) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (!gdata_auth_service_->IsFullyAuthenticated()) { if (!gdata_auth_service_->IsFullyAuthenticated()) {
// Fetch OAuth2 authetication token from the refresh token first. // Fetch OAuth2 authetication token from the refresh token first.
...@@ -1165,7 +1169,7 @@ void DocumentsService::ResumeUploadOnAuthRefresh( ...@@ -1165,7 +1169,7 @@ void DocumentsService::ResumeUploadOnAuthRefresh(
const UploadFileInfo& upload_file_info, const UploadFileInfo& upload_file_info,
GDataErrorCode error, GDataErrorCode error,
const std::string& token) { const std::string& token) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (error != HTTP_SUCCESS) { if (error != HTTP_SUCCESS) {
if (!callback.is_null()) if (!callback.is_null())
...@@ -1182,7 +1186,7 @@ void DocumentsService::OnResumeUploadCompleted( ...@@ -1182,7 +1186,7 @@ void DocumentsService::OnResumeUploadCompleted(
const UploadFileInfo& upload_file_info, const UploadFileInfo& upload_file_info,
int64 start_range_received, int64 start_range_received,
int64 end_range_received) { int64 end_range_received) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
switch (error) { switch (error) {
case HTTP_UNAUTHORIZED: case HTTP_UNAUTHORIZED:
...@@ -1200,7 +1204,7 @@ void DocumentsService::OnResumeUploadCompleted( ...@@ -1200,7 +1204,7 @@ void DocumentsService::OnResumeUploadCompleted(
} }
void DocumentsService::OnOAuth2RefreshTokenChanged() { void DocumentsService::OnOAuth2RefreshTokenChanged() {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
// TODO(zelidrag): Remove this block once we properly wire these API calls // TODO(zelidrag): Remove this block once we properly wire these API calls
// through extension API. // through extension API.
...@@ -1218,7 +1222,7 @@ void DocumentsService::OnOAuth2RefreshTokenChanged() { ...@@ -1218,7 +1222,7 @@ void DocumentsService::OnOAuth2RefreshTokenChanged() {
void DocumentsService::UpdateFilelist(GDataErrorCode status, void DocumentsService::UpdateFilelist(GDataErrorCode status,
base::Value* data) { base::Value* data) {
DCHECK(BrowserThread::CurrentlyOn(kGDataAPICallThread)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
get_documents_started_ = false; get_documents_started_ = false;
......
...@@ -76,6 +76,7 @@ typedef base::Callback<void(GDataErrorCode error, ...@@ -76,6 +76,7 @@ typedef base::Callback<void(GDataErrorCode error,
// This class provides authentication for GData based services. // This class provides authentication for GData based services.
// It integrates specific service integration with OAuth2 stack // It integrates specific service integration with OAuth2 stack
// (TokenService) and provides OAuth2 token refresh infrastructure. // (TokenService) and provides OAuth2 token refresh infrastructure.
// All public functions must be called on UI thread.
class GDataAuthService : public content::NotificationObserver { class GDataAuthService : public content::NotificationObserver {
public: public:
class Observer { class Observer {
...@@ -141,15 +142,19 @@ class GDataAuthService : public content::NotificationObserver { ...@@ -141,15 +142,19 @@ class GDataAuthService : public content::NotificationObserver {
}; };
// This calls provides documents feed service calls. // This calls provides documents feed service calls.
// All public functions must be called on UI thread.
class DocumentsService : public GDataAuthService::Observer { class DocumentsService : public GDataAuthService::Observer {
public: public:
// DocumentsService is usually owned and created by GDataFileSystem. // DocumentsService is usually owned and created by GDataFileSystem.
DocumentsService(); DocumentsService();
virtual ~DocumentsService(); virtual ~DocumentsService();
// Initializes the documents service. // Initializes the documents service tied with |profile|.
void Initialize(Profile* profile); void Initialize(Profile* profile);
// Returns a weak pointer of this documents service.
base::WeakPtr<DocumentsService> AsWeakPtr();
// Authenticates the user by fetching the auth token as // Authenticates the user by fetching the auth token as
// needed. |callback| will be run with the error code and the auth // needed. |callback| will be run with the error code and the auth
// token, on the thread this function is run. // token, on the thread this function is run.
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/json/json_writer.h" #include "base/json/json_writer.h"
#include "base/message_loop.h" #include "base/message_loop.h"
#include "base/message_loop_proxy.h"
#include "base/utf_string_conversions.h" #include "base/utf_string_conversions.h"
#include "base/platform_file.h" #include "base/platform_file.h"
#include "base/stringprintf.h" #include "base/stringprintf.h"
...@@ -307,21 +308,37 @@ GDataFileSystem::FindFileParams::~FindFileParams() { ...@@ -307,21 +308,37 @@ GDataFileSystem::FindFileParams::~FindFileParams() {
GDataFileSystem::GDataFileSystem(Profile* profile) GDataFileSystem::GDataFileSystem(Profile* profile)
: profile_(profile), : profile_(profile),
documents_service_(new DocumentsService) { documents_service_(new DocumentsService),
weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
documents_service_->Initialize(profile_); documents_service_->Initialize(profile_);
root_.reset(new GDataDirectory(NULL)); root_.reset(new GDataDirectory(NULL));
root_->set_file_name(kGDataRootDirectory); root_->set_file_name(kGDataRootDirectory);
// Should be created from the file browser extension API on UI thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
// These weak pointers are used to post tasks to UI thread.
file_system_bound_to_ui_thread_ = weak_ptr_factory_.GetWeakPtr();
documents_service_bound_to_ui_thread_ = documents_service_->AsWeakPtr();
} }
GDataFileSystem::~GDataFileSystem() { GDataFileSystem::~GDataFileSystem() {
// Should be deleted as part of Profile on UI thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
} }
void GDataFileSystem::Shutdown() { void GDataFileSystem::Shutdown() {
// TODO(satorux): We should probably cancel or wait for the in-flight // TODO(satorux): We should probably cancel or wait for the in-flight
// operation here. // operation here.
// Should be called on UI thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
} }
void GDataFileSystem::Authenticate(const AuthStatusCallback& callback) { void GDataFileSystem::Authenticate(const AuthStatusCallback& callback) {
// TokenFetcher, used in DocumentsService, must be run on UI thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
documents_service_->Authenticate(callback); documents_service_->Authenticate(callback);
} }
...@@ -338,29 +355,39 @@ void GDataFileSystem::StartDirectoryRefresh( ...@@ -338,29 +355,39 @@ void GDataFileSystem::StartDirectoryRefresh(
BrowserThread::PostTask( BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE, BrowserThread::UI, FROM_HERE,
base::Bind( base::Bind(
&GDataFileSystem::RefreshFeedOnUIThread, &DocumentsService::GetDocuments,
this, documents_service_bound_to_ui_thread_,
params.feed_url, params.feed_url,
base::Bind(&GDataFileSystem::OnGetDocuments, base::Bind(&GDataFileSystem::OnGetDocuments,
this, file_system_bound_to_ui_thread_,
params))); params)));
} }
void GDataFileSystem::Remove(const FilePath& file_path, void GDataFileSystem::Remove(const FilePath& file_path,
bool is_recursive, bool is_recursive,
const FileOperationCallback& callback) { const FileOperationCallback& callback) {
// Kick off document deletion. GURL document_url = GetDocumentUrlFromPath(file_path);
if (document_url.is_empty()) {
if (!callback.is_null()) {
MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND));
}
}
BrowserThread::PostTask( BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE, BrowserThread::UI, FROM_HERE,
base::Bind( base::Bind(
&GDataFileSystem::RemoveOnUIThread, &DocumentsService::DeleteDocument,
this, documents_service_bound_to_ui_thread_,
file_path, document_url,
is_recursive,
base::Bind(&GDataFileSystem::OnRemovedDocument, base::Bind(&GDataFileSystem::OnRemovedDocument,
this, file_system_bound_to_ui_thread_,
callback, callback,
file_path))); file_path,
// MessageLoopProxy is used to run |callback| on the
// thread where Remove() was called.
base::MessageLoopProxy::current())));
} }
void GDataFileSystem::UnsafeFindFileByPath( void GDataFileSystem::UnsafeFindFileByPath(
...@@ -414,27 +441,6 @@ void GDataFileSystem::UnsafeFindFileByPath( ...@@ -414,27 +441,6 @@ void GDataFileSystem::UnsafeFindFileByPath(
delegate->OnError(base::PLATFORM_FILE_ERROR_NOT_FOUND); delegate->OnError(base::PLATFORM_FILE_ERROR_NOT_FOUND);
} }
void GDataFileSystem::RefreshFeedOnUIThread(const GURL& feed_url,
const GetDataCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
documents_service_->GetDocuments(feed_url, callback);
}
void GDataFileSystem::RemoveOnUIThread(
const FilePath& file_path, bool is_recursive,
const EntryActionCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
GURL document_url = GetDocumentUrlFromPath(file_path);
if (document_url.is_empty()) {
if (!callback.is_null())
callback.Run(HTTP_NOT_FOUND, GURL());
return;
}
documents_service_->DeleteDocument(document_url, callback);
}
GURL GDataFileSystem::GetDocumentUrlFromPath(const FilePath& file_path) { GURL GDataFileSystem::GetDocumentUrlFromPath(const FilePath& file_path) {
base::AutoLock lock(lock_); base::AutoLock lock(lock_);
// Find directory element within the cached file system snapshot. // Find directory element within the cached file system snapshot.
...@@ -451,6 +457,8 @@ void GDataFileSystem::OnGetDocuments( ...@@ -451,6 +457,8 @@ void GDataFileSystem::OnGetDocuments(
const FindFileParams& params, const FindFileParams& params,
GDataErrorCode status, GDataErrorCode status,
base::Value* data) { base::Value* data) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
base::PlatformFileError error = GDataToPlatformError(status); base::PlatformFileError error = GDataToPlatformError(status);
if (error == base::PLATFORM_FILE_OK && if (error == base::PLATFORM_FILE_OK &&
...@@ -493,14 +501,19 @@ void GDataFileSystem::OnGetDocuments( ...@@ -493,14 +501,19 @@ void GDataFileSystem::OnGetDocuments(
void GDataFileSystem::OnRemovedDocument( void GDataFileSystem::OnRemovedDocument(
const FileOperationCallback& callback, const FileOperationCallback& callback,
const FilePath& file_path, const FilePath& file_path,
GDataErrorCode status, const GURL& document_url) { scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
GDataErrorCode status,
const GURL& document_url) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
base::PlatformFileError error = GDataToPlatformError(status); base::PlatformFileError error = GDataToPlatformError(status);
if (error == base::PLATFORM_FILE_OK) if (error == base::PLATFORM_FILE_OK)
error = RemoveFileFromFileSystem(file_path); error = RemoveFileFromFileSystem(file_path);
if (!callback.is_null()) if (!callback.is_null()) {
callback.Run(error); message_loop_proxy->PostTask(FROM_HERE, base::Bind(callback, error));
}
} }
base::PlatformFileError GDataFileSystem::RemoveFileFromFileSystem( base::PlatformFileError GDataFileSystem::RemoveFileFromFileSystem(
......
...@@ -9,6 +9,8 @@ ...@@ -9,6 +9,8 @@
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/memory/singleton.h" #include "base/memory/singleton.h"
#include "base/memory/weak_ptr.h"
#include "base/message_loop_proxy.h"
#include "base/platform_file.h" #include "base/platform_file.h"
#include "base/synchronization/lock.h" #include "base/synchronization/lock.h"
#include "chrome/browser/chromeos/gdata/gdata.h" #include "chrome/browser/chromeos/gdata/gdata.h"
...@@ -198,11 +200,9 @@ class ReadOnlyFindFileDelegate : public FindFileDelegate { ...@@ -198,11 +200,9 @@ class ReadOnlyFindFileDelegate : public FindFileDelegate {
}; };
// GData file system abstraction layer. This class is refcounted since we // GData file system abstraction layer.
// access it from different threads and aggregate into number of other objects.
// GDataFileSystem is per-profie, hence inheriting ProfileKeyedService. // GDataFileSystem is per-profie, hence inheriting ProfileKeyedService.
class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>, class GDataFileSystem : public ProfileKeyedService {
public ProfileKeyedService {
public: public:
struct FindFileParams { struct FindFileParams {
FindFileParams(const FilePath& in_file_path, FindFileParams(const FilePath& in_file_path,
...@@ -230,12 +230,16 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>, ...@@ -230,12 +230,16 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>,
// Authenticates the user by fetching the auth token as // Authenticates the user by fetching the auth token as
// needed. |callback| will be run with the error code and the auth // needed. |callback| will be run with the error code and the auth
// token, on the thread this function is run. // token, on the thread this function is run.
//
// Must be called on UI thread.
void Authenticate(const AuthStatusCallback& callback); void Authenticate(const AuthStatusCallback& callback);
// Finds file info by using virtual |file_path|. If |require_content| is set, // Finds file info by using virtual |file_path|. If |require_content| is set,
// the found directory will be pre-populated before passed back to the // the found directory will be pre-populated before passed back to the
// |delegate|. If |allow_refresh| is not set, directories' content // |delegate|. If |allow_refresh| is not set, directories' content
// won't be performed. // won't be performed.
//
// Can be called from any thread.
void FindFileByPath(const FilePath& file_path, void FindFileByPath(const FilePath& file_path,
scoped_refptr<FindFileDelegate> delegate); scoped_refptr<FindFileDelegate> delegate);
...@@ -244,21 +248,27 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>, ...@@ -244,21 +248,27 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>,
// contained children elements. The file entry represented by |file_path| // contained children elements. The file entry represented by |file_path|
// needs to be present in in-memory representation of the file system that // needs to be present in in-memory representation of the file system that
// in order to be removed. // in order to be removed.
//
// |callback| is run on UI thread, when |file_path| is removed on the
// thread where Remove() is called.
//
// TODO(zelidrag): Wire |is_recursive| through gdata api // TODO(zelidrag): Wire |is_recursive| through gdata api
// (find appropriate call for it). // (find appropriate call for it).
//
// Can be called from any thread.
void Remove(const FilePath& file_path, void Remove(const FilePath& file_path,
bool is_recursive, bool is_recursive,
const FileOperationCallback& callback); const FileOperationCallback& callback);
// Initiates directory feed fetching operation and continues previously
// initiated FindFileByPath() attempt upon its completion. Safe to be called // initiated FindFileByPath() attempt upon its completion. Safe to be called
// from any thread. Internally, it will route content refresh request to // from any thread. Internally, it will route content refresh request to
// RefreshFeedOnUIThread() method which will initiated content fetching from // DocumentsService::GetDocuments() which will initiated content
// UI thread as required by gdata library (UrlFetcher). // fetching from UI thread as required by gdata library (UrlFetcher).
//
// Can be called from any thread.
void StartDirectoryRefresh(const FindFileParams& params); void StartDirectoryRefresh(const FindFileParams& params);
private: private:
friend class base::RefCountedThreadSafe<GDataFileSystem>;
friend class GDataFileSystemFactory; friend class GDataFileSystemFactory;
friend class GDataFileSystemTest; friend class GDataFileSystemTest;
...@@ -269,15 +279,6 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>, ...@@ -269,15 +279,6 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>,
void UnsafeFindFileByPath(const FilePath& file_path, void UnsafeFindFileByPath(const FilePath& file_path,
scoped_refptr<FindFileDelegate> delegate); scoped_refptr<FindFileDelegate> delegate);
// Initiates document feed fetching from UI thread.
void RefreshFeedOnUIThread(const GURL& feed_url,
const GetDataCallback& callback);
// Initiates |file_path| entry deletion from UI thread.
void RemoveOnUIThread(const FilePath& file_path,
bool is_recursive,
const EntryActionCallback& callback);
// Finds file object by |file_path| and returns its gdata self-url. // Finds file object by |file_path| and returns its gdata self-url.
// Returns empty GURL if it does not find the file. // Returns empty GURL if it does not find the file.
GURL GetDocumentUrlFromPath(const FilePath& file_path); GURL GetDocumentUrlFromPath(const FilePath& file_path);
...@@ -297,11 +298,14 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>, ...@@ -297,11 +298,14 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>,
GDataErrorCode status, GDataErrorCode status,
base::Value* data); base::Value* data);
// Callback for handling document remove attempt. // Callback for handling document remove attempt. |message_loop_proxy| is
// used to post a task to the thread where Remove() was called.
void OnRemovedDocument( void OnRemovedDocument(
const FileOperationCallback& callback, const FileOperationCallback& callback,
const FilePath& file_path, const FilePath& file_path,
GDataErrorCode status, const GURL& document_url); scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
GDataErrorCode status,
const GURL& document_url);
// Removes file under |file_path| from in-memory snapshot of the file system. // Removes file under |file_path| from in-memory snapshot of the file system.
// Return PLATFORM_FILE_OK if successful. // Return PLATFORM_FILE_OK if successful.
...@@ -325,6 +329,10 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>, ...@@ -325,6 +329,10 @@ class GDataFileSystem : public base::RefCountedThreadSafe<GDataFileSystem>,
// The document service for the GDataFileSystem. // The document service for the GDataFileSystem.
scoped_ptr<DocumentsService> documents_service_; scoped_ptr<DocumentsService> documents_service_;
base::WeakPtrFactory<GDataFileSystem> weak_ptr_factory_;
base::WeakPtr<GDataFileSystem> file_system_bound_to_ui_thread_;
base::WeakPtr<DocumentsService> documents_service_bound_to_ui_thread_;
}; };
// Singleton that owns all GDataFileSystems and associates them with // Singleton that owns all GDataFileSystems and associates them with
......
...@@ -44,7 +44,7 @@ base::FileUtilProxy::Entry GDataFileToFileUtilProxyEntry( ...@@ -44,7 +44,7 @@ base::FileUtilProxy::Entry GDataFileToFileUtilProxyEntry(
class FindFileDelegateReplyBase : public FindFileDelegate { class FindFileDelegateReplyBase : public FindFileDelegate {
public: public:
FindFileDelegateReplyBase( FindFileDelegateReplyBase(
scoped_refptr<GDataFileSystem> file_system, GDataFileSystem* file_system,
const FilePath& search_file_path, const FilePath& search_file_path,
bool require_content) bool require_content)
: file_system_(file_system), : file_system_(file_system),
...@@ -93,7 +93,7 @@ class FindFileDelegateReplyBase : public FindFileDelegate { ...@@ -93,7 +93,7 @@ class FindFileDelegateReplyBase : public FindFileDelegate {
} }
protected: protected:
scoped_refptr<GDataFileSystem> file_system_; GDataFileSystem* file_system_;
// Search file path. // Search file path.
FilePath search_file_path_; FilePath search_file_path_;
// True if the final directory content is required. // True if the final directory content is required.
...@@ -106,7 +106,7 @@ class FindFileDelegateReplyBase : public FindFileDelegate { ...@@ -106,7 +106,7 @@ class FindFileDelegateReplyBase : public FindFileDelegate {
class GetFileInfoDelegate : public FindFileDelegateReplyBase { class GetFileInfoDelegate : public FindFileDelegateReplyBase {
public: public:
GetFileInfoDelegate( GetFileInfoDelegate(
scoped_refptr<GDataFileSystem> file_system, GDataFileSystem* file_system,
const FilePath& search_file_path, const FilePath& search_file_path,
const FileSystemOperationInterface::GetMetadataCallback& callback) const FileSystemOperationInterface::GetMetadataCallback& callback)
: FindFileDelegateReplyBase(file_system, : FindFileDelegateReplyBase(file_system,
...@@ -166,7 +166,7 @@ class GetFileInfoDelegate : public FindFileDelegateReplyBase { ...@@ -166,7 +166,7 @@ class GetFileInfoDelegate : public FindFileDelegateReplyBase {
class ReadDirectoryDelegate : public FindFileDelegateReplyBase { class ReadDirectoryDelegate : public FindFileDelegateReplyBase {
public: public:
ReadDirectoryDelegate( ReadDirectoryDelegate(
scoped_refptr<GDataFileSystem> file_system, GDataFileSystem* file_system,
const FilePath& search_file_path, const FilePath& search_file_path,
const FileSystemOperationInterface::ReadDirectoryCallback& callback) const FileSystemOperationInterface::ReadDirectoryCallback& callback)
: FindFileDelegateReplyBase(file_system, : FindFileDelegateReplyBase(file_system,
...@@ -243,9 +243,14 @@ class ReadDirectoryDelegate : public FindFileDelegateReplyBase { ...@@ -243,9 +243,14 @@ class ReadDirectoryDelegate : public FindFileDelegateReplyBase {
GDataFileSystemProxy::GDataFileSystemProxy(Profile* profile) GDataFileSystemProxy::GDataFileSystemProxy(Profile* profile)
: file_system_(GDataFileSystemFactory::GetForProfile(profile)) { : file_system_(GDataFileSystemFactory::GetForProfile(profile)) {
// Should be created from the file browser extension API (AddMountFunction)
// on UI thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
} }
GDataFileSystemProxy::~GDataFileSystemProxy() { GDataFileSystemProxy::~GDataFileSystemProxy() {
// Should be deleted from the CrosMountPointProvider on IO thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
} }
void GDataFileSystemProxy::GetFileInfo(const GURL& file_url, void GDataFileSystemProxy::GetFileInfo(const GURL& file_url,
......
...@@ -40,12 +40,16 @@ class GDataFileSystemProxy : public fileapi::RemoteFileSystemProxyInterface { ...@@ -40,12 +40,16 @@ class GDataFileSystemProxy : public fileapi::RemoteFileSystemProxyInterface {
const fileapi::FileSystemOperationInterface::StatusCallback& callback, const fileapi::FileSystemOperationInterface::StatusCallback& callback,
base::PlatformFileError result); base::PlatformFileError result);
// Checks if a given |url| belongs to this file system. If it does, // Checks if a given |url| belongs to this file system. If it does,
// the call will return true and fill in |file_path| with a file path of // the call will return true and fill in |file_path| with a file path of
// a corresponding element within this file system. // a corresponding element within this file system.
static bool ValidateUrl(const GURL& url, FilePath* file_path); static bool ValidateUrl(const GURL& url, FilePath* file_path);
scoped_refptr<GDataFileSystem> file_system_; // GDataFileSystemProxy is owned by Profile, which outlives
// GDataFileSystemProxy, which is owned by CrosMountPointProvider (i.e. by
// the time Profile is removed, the file manager is already gone). Hence
// it's safe to use this as a raw pointer.
GDataFileSystem* file_system_;
}; };
} // namespace chromeos } // namespace chromeos
......
...@@ -38,7 +38,7 @@ class GDataFileSystemTest : public testing::Test { ...@@ -38,7 +38,7 @@ class GDataFileSystemTest : public testing::Test {
virtual void SetUp() { virtual void SetUp() {
profile_.reset(new TestingProfile); profile_.reset(new TestingProfile);
file_system_ = new GDataFileSystem(profile_.get()); file_system_ = GDataFileSystemFactory::GetForProfile(profile_.get());
} }
// Loads test json file as root ("/gdata") element. // Loads test json file as root ("/gdata") element.
...@@ -129,7 +129,7 @@ class GDataFileSystemTest : public testing::Test { ...@@ -129,7 +129,7 @@ class GDataFileSystemTest : public testing::Test {
MessageLoopForUI message_loop_; MessageLoopForUI message_loop_;
content::TestBrowserThread ui_thread_; content::TestBrowserThread ui_thread_;
scoped_ptr<TestingProfile> profile_; scoped_ptr<TestingProfile> profile_;
scoped_refptr<GDataFileSystem> file_system_; GDataFileSystem* file_system_;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment