gdata: Get rid of GDataFileSystem::GetCacheState()
Instead, add GDataCache::GetCacheEntryOnUIThread().
BUG=133552
TEST=the existing testis covered this functionality
Review URL: https://chromiumcodereview.appspot.com/10690028
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@144878 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/chromeos/extensions/file_browser_private_api.cc b/chrome/browser/chromeos/extensions/file_browser_private_api.cc
index 7af2884..828050d 100644
--- a/chrome/browser/chromeos/extensions/file_browser_private_api.cc
+++ b/chrome/browser/chromeos/extensions/file_browser_private_api.cc
@@ -1819,7 +1819,7 @@
}
}
- system_service->file_system()->GetCacheState(
+ system_service->cache()->GetCacheEntryOnUIThread(
file_proto->gdata_entry().resource_id(),
file_proto->file_md5(),
base::Bind(
@@ -1829,8 +1829,10 @@
void GetGDataFilePropertiesFunction::CacheStateReceived(
base::DictionaryValue* property_dict,
- base::PlatformFileError error,
- int cache_state) {
+ bool success,
+ const gdata::GDataCache::CacheEntry& cache_entry) {
+ const int cache_state = (success ? cache_entry.cache_state :
+ gdata::GDataCache::CACHE_STATE_NONE);
property_dict->SetBoolean(
"isPinned",
gdata::GDataCache::IsCachePinned(cache_state));
diff --git a/chrome/browser/chromeos/extensions/file_browser_private_api.h b/chrome/browser/chromeos/extensions/file_browser_private_api.h
index 170a62be..8f1f74a 100644
--- a/chrome/browser/chromeos/extensions/file_browser_private_api.h
+++ b/chrome/browser/chromeos/extensions/file_browser_private_api.h
@@ -14,6 +14,7 @@
#include "base/memory/scoped_ptr.h"
#include "base/platform_file.h"
#include "chrome/browser/chromeos/extensions/file_browser_event_router.h"
+#include "chrome/browser/chromeos/gdata/gdata_cache.h"
#include "chrome/browser/extensions/extension_function.h"
#include "chrome/browser/prefs/pref_service.h"
#include "googleurl/src/url_util.h"
@@ -450,8 +451,8 @@
scoped_ptr<gdata::GDataFileProto> file_proto);
void CacheStateReceived(base::DictionaryValue* property_dict,
- base::PlatformFileError error,
- int cache_state);
+ bool success,
+ const gdata::GDataCache::CacheEntry& cache_entry);
size_t current_index_;
base::ListValue* path_list_;
diff --git a/chrome/browser/chromeos/gdata/gdata_cache.cc b/chrome/browser/chromeos/gdata/gdata_cache.cc
index 24f4b9b..2737fe2 100644
--- a/chrome/browser/chromeos/gdata/gdata_cache.cc
+++ b/chrome/browser/chromeos/gdata/gdata_cache.cc
@@ -346,7 +346,7 @@
}
// Runs callback with pointers dereferenced.
-// Used to implement GetResourceIdsOfBacklog().
+// Used to implement GetResourceIdsOfBacklogOnUIThread().
void RunGetResourceIdsCallback(const GetResourceIdsCallback& callback,
std::vector<std::string>* to_fetch,
std::vector<std::string>* to_upload) {
@@ -358,6 +358,19 @@
callback.Run(*to_fetch, *to_upload);
}
+// Runs callback with pointers dereferenced.
+// Used to implement GetCacheEntryOnUIThread().
+void RunGetCacheEntryCallback(
+ const GDataCache::GetCacheEntryCallback& callback,
+ bool* success,
+ GDataCache::CacheEntry* cache_entry) {
+ DCHECK(success);
+ DCHECK(cache_entry);
+
+ if (!callback.is_null())
+ callback.Run(*success, *cache_entry);
+}
+
} // namespace
std::string GDataCache::CacheEntry::ToString() const {
@@ -445,6 +458,28 @@
observers_.RemoveObserver(observer);
}
+void GDataCache::GetCacheEntryOnUIThread(
+ const std::string& resource_id,
+ const std::string& md5,
+ const GetCacheEntryCallback& callback) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+ bool* success = new bool(false);
+ GDataCache::CacheEntry* cache_entry = new GDataCache::CacheEntry;
+ pool_->GetSequencedTaskRunner(sequence_token_)->PostTaskAndReply(
+ FROM_HERE,
+ base::Bind(&GDataCache::GetCacheEntryHelper,
+ base::Unretained(this),
+ resource_id,
+ md5,
+ success,
+ cache_entry),
+ base::Bind(&RunGetCacheEntryCallback,
+ callback,
+ base::Owned(success),
+ base::Owned(cache_entry)));
+}
+
void GDataCache::GetResourceIdsOfBacklogOnUIThread(
const GetResourceIdsCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -1463,6 +1498,20 @@
FOR_EACH_OBSERVER(Observer, observers_, OnCacheCommitted(resource_id));
}
+void GDataCache::GetCacheEntryHelper(const std::string& resource_id,
+ const std::string& md5,
+ bool* success,
+ GDataCache::CacheEntry* cache_entry) {
+ AssertOnSequencedWorkerPool();
+ DCHECK(success);
+ DCHECK(cache_entry);
+
+ scoped_ptr<GDataCache::CacheEntry> value(GetCacheEntry(resource_id, md5));
+ *success = value.get();
+ if (*success)
+ *cache_entry = *value;
+}
+
// static
FilePath GDataCache::GetCacheRootPath(Profile* profile) {
FilePath cache_base_path;
diff --git a/chrome/browser/chromeos/gdata/gdata_cache.h b/chrome/browser/chromeos/gdata/gdata_cache.h
index 4e615df..a8995b0 100644
--- a/chrome/browser/chromeos/gdata/gdata_cache.h
+++ b/chrome/browser/chromeos/gdata/gdata_cache.h
@@ -122,9 +122,9 @@
CacheEntry(const std::string& md5,
CacheSubDirectoryType sub_dir_type,
int cache_state)
- : md5(md5),
- sub_dir_type(sub_dir_type),
- cache_state(cache_state) {
+ : md5(md5),
+ sub_dir_type(sub_dir_type),
+ cache_state(cache_state) {
}
bool IsPresent() const { return IsCachePresent(cache_state); }
@@ -140,6 +140,16 @@
int cache_state;
};
+ // Callback for GetCacheEntryOnUIThread.
+ // |success| indicates if the operation was successful.
+ // |cache_entry| is the obtained cache entry.
+ //
+ // TODO(satorux): Unlike other callback types, this has to be defined
+ // inside GDataCache as CacheEntry is inside GDataCache. We should get them
+ // outside of GDataCache.
+ typedef base::Callback<void(bool success, const CacheEntry& cache_entry)>
+ GetCacheEntryCallback;
+
static bool IsCachePresent(int cache_state) {
return cache_state & CACHE_STATE_PRESENT;
}
@@ -205,6 +215,15 @@
// Must be called on UI thread.
void RemoveObserver(Observer* observer);
+ // Gets the cache entry by the given resource ID and MD5.
+ // See also GetCacheEntry().
+ //
+ // Must be called on UI thread. |callback| is run on UI thread.
+ void GetCacheEntryOnUIThread(
+ const std::string& resource_id,
+ const std::string& md5,
+ const GetCacheEntryCallback& callback);
+
// Gets the resource IDs of pinned-but-not-fetched files and
// dirty-but-not-uploaded files.
//
@@ -418,6 +437,12 @@
const std::string& md5,
const CacheOperationCallback& callback);
+ // Helper function to implement GetCacheEntryOnUIThread().
+ void GetCacheEntryHelper(const std::string& resource_id,
+ const std::string& md5,
+ bool* success,
+ GDataCache::CacheEntry* cache_entry);
+
// The root directory of the cache (i.e. <user_profile_dir>/GCache/v1).
const FilePath cache_root_path_;
// Paths for all subdirectories of GCache, one for each
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system.cc b/chrome/browser/chromeos/gdata/gdata_file_system.cc
index c14befb..60749abc 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system.cc
@@ -190,21 +190,6 @@
relay_proxy->PostTask(FROM_HERE, base::Bind(callback, error));
}
-// Gets a cache entry from a GDataCache, must be called on the blocking pool.
-// The result value is copied to cache_entry on success.
-void GetCacheEntryOnBlockingPool(
- GDataCache* cache,
- const std::string& resource_id,
- const std::string& md5,
- GDataCache::CacheEntry* cache_entry,
- bool* success) {
- scoped_ptr<GDataCache::CacheEntry> value(
- cache->GetCacheEntry(resource_id, md5));
- *success = value.get();
- if (*success)
- *cache_entry = *value;
-}
-
// Runs GetFileCallback with pointers dereferenced.
// Used for PostTaskAndReply().
void RunGetFileCallbackHelper(const GetFileCallback& callback,
@@ -253,21 +238,6 @@
callback.Run(error);
}
-// Used to implement GetCacheState.
-void RunGetCacheStateCallbackHelper(
- const GetCacheStateCallback& callback,
- GDataCache::CacheEntry* cache_entry,
- bool* success) {
- DCHECK(cache_entry);
- DCHECK(success);
- if (callback.is_null())
- return;
-
- callback.Run(
- base::PLATFORM_FILE_OK,
- *success ? cache_entry->cache_state : GDataCache::CACHE_STATE_NONE);
-}
-
// The class to wait for the initial load of root feed and runs the callback
// after the initialization.
class InitialLoadObserver : public GDataFileSystemInterface::Observer {
@@ -2291,62 +2261,6 @@
base::Passed(&upload_file_info)));
}
-void GDataFileSystem::GetCacheState(const std::string& resource_id,
- const std::string& md5,
- const GetCacheStateCallback& callback) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
- BrowserThread::CurrentlyOn(BrowserThread::IO));
-
- // Always post a task to the UI thread to call GetCacheStateOnUIThread even if
- // GetCacheState is called on the UI thread. This ensures that, regardless of
- // whether GDataFileSystem is locked or not, GDataFileSystem is unlocked when
- // GetCacheStateOnUIThread is called.
- const bool posted = BrowserThread::PostTask(
- BrowserThread::UI,
- FROM_HERE,
- base::Bind(&GDataFileSystem::GetCacheStateOnUIThread,
- ui_weak_ptr_,
- resource_id,
- md5,
- CreateRelayCallback(callback)));
- DCHECK(posted);
-}
-
-void GDataFileSystem::GetCacheStateOnUIThread(
- const std::string& resource_id,
- const std::string& md5,
- const GetCacheStateCallback& callback) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- GDataEntry* entry = root_->GetEntryByResourceId(resource_id);
- if (!entry || !entry->AsGDataFile()) {
- const bool posted = BrowserThread::PostTask(
- BrowserThread::UI,
- FROM_HERE,
- base::Bind(callback,
- base::PLATFORM_FILE_ERROR_NOT_FOUND,
- GDataCache::CACHE_STATE_NONE));
- DCHECK(posted);
- return;
- }
-
- GDataCache::CacheEntry* cache_entry = new GDataCache::CacheEntry;
- bool* success = new bool(false);
- PostBlockingPoolSequencedTaskAndReply(
- FROM_HERE,
- sequence_token_,
- base::Bind(&GetCacheEntryOnBlockingPool,
- cache_,
- resource_id,
- md5,
- cache_entry,
- success),
- base::Bind(&RunGetCacheStateCallbackHelper,
- callback,
- base::Owned(cache_entry),
- base::Owned(success)));
-}
-
void GDataFileSystem::GetAvailableSpace(
const GetAvailableSpaceCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
@@ -2856,23 +2770,13 @@
// If user cancels download of a pinned-but-not-fetched file, mark file as
// unpinned so that we do not sync the file again.
if (status == GDATA_CANCELLED) {
- GDataCache::CacheEntry* cache_entry = new GDataCache::CacheEntry;
- bool* success = new bool(false);
- PostBlockingPoolSequencedTaskAndReply(
- FROM_HERE,
- sequence_token_,
- base::Bind(&GetCacheEntryOnBlockingPool,
- cache_,
- params.resource_id,
- params.md5,
- cache_entry,
- success),
+ cache_->GetCacheEntryOnUIThread(
+ params.resource_id,
+ params.md5,
base::Bind(&GDataFileSystem::UnpinIfPinned,
ui_weak_ptr_,
params.resource_id,
- params.md5,
- base::Owned(cache_entry),
- base::Owned(success)));
+ params.md5));
}
// At this point, the disk can be full or nearly full for several reasons:
@@ -2900,14 +2804,15 @@
base::Owned(has_enough_space)));
}
-void GDataFileSystem::UnpinIfPinned(const std::string& resource_id,
- const std::string& md5,
- GDataCache::CacheEntry* cache_entry,
- bool* cache_entry_is_valid) {
+void GDataFileSystem::UnpinIfPinned(
+ const std::string& resource_id,
+ const std::string& md5,
+ bool success,
+ const GDataCache::CacheEntry& cache_entry) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
// TODO(hshi): http://crbug.com/127138 notify when file properties change.
// This allows file manager to clear the "Available offline" checkbox.
- if (*cache_entry_is_valid && cache_entry->IsPinned())
+ if (success && cache_entry.IsPinned())
cache_->UnpinOnUIThread(resource_id, md5, CacheOperationCallback());
}
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system.h b/chrome/browser/chromeos/gdata/gdata_file_system.h
index 356c3ed..add434f 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system.h
+++ b/chrome/browser/chromeos/gdata/gdata_file_system.h
@@ -103,10 +103,6 @@
typedef base::Callback<void(const std::string& resource_id)>
GetDocumentResourceIdCallback;
-// Callback for GetCacheState operation.
-typedef base::Callback<void(base::PlatformFileError error,
- int cache_state)> GetCacheStateCallback;
-
// GData file system abstraction layer.
// The interface is defined to make GDataFileSystem mockable.
class GDataFileSystemInterface {
@@ -297,16 +293,6 @@
const std::string& resource_id,
const FileOperationCallback& callback) = 0;
- // Gets the cache state of file corresponding to |resource_id| and |md5| if it
- // exists on disk.
- // Initializes cache if it has not been initialized.
- // Upon completion, |callback| is invoked on the thread where this method was
- // called with the cache state if file exists in cache or CACHE_STATE_NONE
- // otherwise.
- virtual void GetCacheState(const std::string& resource_id,
- const std::string& md5,
- const GetCacheStateCallback& callback) = 0;
-
// Finds an entry (a file or a directory) by |file_path|. This call will also
// retrieve and refresh file system content from server and disk cache.
//
@@ -434,9 +420,6 @@
virtual void UpdateFileByResourceId(
const std::string& resource_id,
const FileOperationCallback& callback) OVERRIDE;
- virtual void GetCacheState(const std::string& resource_id,
- const std::string& md5,
- const GetCacheStateCallback& callback) OVERRIDE;
virtual void GetEntryInfoByPath(
const FilePath& file_path,
const GetEntryInfoCallback& callback) OVERRIDE;
@@ -846,8 +829,8 @@
// Unpins file if cache entry is pinned.
void UnpinIfPinned(const std::string& resource_id,
const std::string& md5,
- GDataCache::CacheEntry* cache_entry,
- bool* cache_entry_is_valid);
+ bool success,
+ const GDataCache::CacheEntry& cache_entry);
// Similar to OnFileDownloaded() but takes |has_enough_space| so we report
// an error in case we don't have enough disk space.
@@ -1159,9 +1142,6 @@
const FilePath& file_path);
void OnRequestDirectoryRefresh(GetDocumentsParams* params,
base::PlatformFileError error);
- void GetCacheStateOnUIThread(const std::string& resource_id,
- const std::string& md5,
- const GetCacheStateCallback& callback);
void GetAvailableSpaceOnUIThread(const GetAvailableSpaceCallback& callback);
void AddUploadedFileOnUIThread(UploadMode upload_mode,
const FilePath& virtual_dir_path,
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc b/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
index 3ec3759..6d6854d 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
@@ -211,6 +211,7 @@
expected_error_(base::PLATFORM_FILE_OK),
expected_cache_state_(0),
expected_sub_dir_type_(GDataCache::CACHE_TYPE_META),
+ expected_success_(true),
expect_outgoing_symlink_(false),
root_feed_changestamp_(0) {
}
@@ -731,26 +732,29 @@
RunAllPendingForIO(); // Post FreeDiskSpaceIfNeededFor to blocking pool.
}
- void TestGetCacheState(const std::string& resource_id, const std::string& md5,
- base::PlatformFileError expected_error,
- int expected_cache_state, GDataFile* expected_file) {
- expected_error_ = expected_error;
+ void TestGetCacheState(const std::string& resource_id,
+ const std::string& md5,
+ bool expected_success,
+ int expected_cache_state,
+ GDataFile* expected_file) {
+ expected_success_ = expected_success;
expected_cache_state_ = expected_cache_state;
- file_system_->GetCacheState(resource_id, md5,
+ cache_->GetCacheEntryOnUIThread(resource_id, md5,
base::Bind(&GDataFileSystemTest::VerifyGetCacheState,
base::Unretained(this)));
RunAllPendingForIO();
}
- void VerifyGetCacheState(base::PlatformFileError error, int cache_state) {
+ void VerifyGetCacheState(bool success,
+ const GDataCache::CacheEntry& cache_entry) {
++num_callback_invocations_;
- EXPECT_EQ(expected_error_, error);
+ EXPECT_EQ(expected_success_, success);
- if (error == base::PLATFORM_FILE_OK) {
- EXPECT_EQ(expected_cache_state_, cache_state);
+ if (success) {
+ EXPECT_EQ(expected_cache_state_, cache_entry.cache_state);
}
}
@@ -1327,6 +1331,7 @@
base::PlatformFileError expected_error_;
int expected_cache_state_;
GDataCache::CacheSubDirectoryType expected_sub_dir_type_;
+ bool expected_success_;
bool expect_outgoing_symlink_;
std::string expected_file_extension_;
int root_feed_changestamp_;
@@ -2970,7 +2975,7 @@
// Get its cache state.
num_callback_invocations_ = 0;
- TestGetCacheState(resource_id, md5, base::PLATFORM_FILE_OK,
+ TestGetCacheState(resource_id, md5, true,
GDataCache::CACHE_STATE_PRESENT, file);
EXPECT_EQ(1, num_callback_invocations_);
}
@@ -3000,14 +3005,14 @@
// Get its cache state.
num_callback_invocations_ = 0;
- TestGetCacheState(resource_id, md5, base::PLATFORM_FILE_OK,
+ TestGetCacheState(resource_id, md5, true,
expected_cache_state, file);
EXPECT_EQ(1, num_callback_invocations_);
}
{ // Test cache state of a non-existent file.
num_callback_invocations_ = 0;
- TestGetCacheState("pdf:12345", "abcd", base::PLATFORM_FILE_ERROR_NOT_FOUND,
+ TestGetCacheState("pdf:12345", "abcd", false,
0, NULL);
EXPECT_EQ(1, num_callback_invocations_);
}
diff --git a/chrome/browser/chromeos/gdata/mock_gdata_file_system.h b/chrome/browser/chromeos/gdata/mock_gdata_file_system.h
index a94bb54..ec515700 100644
--- a/chrome/browser/chromeos/gdata/mock_gdata_file_system.h
+++ b/chrome/browser/chromeos/gdata/mock_gdata_file_system.h
@@ -68,9 +68,6 @@
MOCK_METHOD2(UpdateFileByResourceId,
void(const std::string& resource_id,
const FileOperationCallback& callback));
- MOCK_METHOD3(GetCacheState, void(const std::string& resource_id,
- const std::string& md5,
- const GetCacheStateCallback& callback));
MOCK_METHOD2(GetFileInfoByPath, void(const FilePath& file_path,
const GetFileInfoCallback& callback));
MOCK_METHOD2(GetEntryInfoByPath, void(const FilePath& file_path,
