drive: Remove FileCacheObserver
FileSystem::Pin()/Unpin() calls SyncClient methods directly
BUG=247274
TEST=unit_tests
Review URL: https://chromiumcodereview.appspot.com/16318004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@205803 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/chromeos/drive/file_cache.cc b/chrome/browser/chromeos/drive/file_cache.cc
index 0499765..36e09b99 100644
--- a/chrome/browser/chromeos/drive/file_cache.cc
+++ b/chrome/browser/chromeos/drive/file_cache.cc
@@ -15,7 +15,6 @@
#include "base/task_runner_util.h"
#include "chrome/browser/chromeos/drive/drive.pb.h"
#include "chrome/browser/chromeos/drive/file_cache_metadata.h"
-#include "chrome/browser/chromeos/drive/file_cache_observer.h"
#include "chrome/browser/chromeos/drive/file_system_util.h"
#include "chrome/browser/google_apis/task_util.h"
#include "chromeos/chromeos_constants.h"
@@ -345,16 +344,6 @@
return cache_root_path_ == path || cache_root_path_.IsParent(path);
}
-void FileCache::AddObserver(FileCacheObserver* observer) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- observers_.AddObserver(observer);
-}
-
-void FileCache::RemoveObserver(FileCacheObserver* observer) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- observers_.RemoveObserver(observer);
-}
-
void FileCache::GetCacheEntryOnUIThread(const std::string& resource_id,
const std::string& md5,
const GetCacheEntryCallback& callback) {
@@ -522,10 +511,8 @@
base::PostTaskAndReplyWithResult(
blocking_task_runner_,
FROM_HERE,
- base::Bind(&FileCache::Pin,
- base::Unretained(this), resource_id, md5),
- base::Bind(&FileCache::OnPinned,
- weak_ptr_factory_.GetWeakPtr(), resource_id, md5, callback));
+ base::Bind(&FileCache::Pin, base::Unretained(this), resource_id, md5),
+ callback);
}
FileError FileCache::Pin(const std::string& resource_id,
@@ -581,10 +568,8 @@
base::PostTaskAndReplyWithResult(
blocking_task_runner_,
FROM_HERE,
- base::Bind(&FileCache::Unpin,
- base::Unretained(this), resource_id, md5),
- base::Bind(&FileCache::OnUnpinned,
- weak_ptr_factory_.GetWeakPtr(), resource_id, md5, callback));
+ base::Bind(&FileCache::Unpin, base::Unretained(this), resource_id, md5),
+ callback);
}
FileError FileCache::Unpin(const std::string& resource_id,
@@ -637,6 +622,11 @@
// Remove the existing entry if we are unpinning a non-present file.
metadata_->RemoveCacheEntry(resource_id);
}
+
+ // Now the file is moved from "persistent" to "tmp" directory.
+ // It's a chance to free up space if needed.
+ FreeDiskSpaceIfNeededFor(0);
+
return FILE_ERROR_OK;
}
@@ -1112,44 +1102,6 @@
return true;
}
-void FileCache::OnPinned(const std::string& resource_id,
- const std::string& md5,
- const FileOperationCallback& callback,
- FileError error) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- DCHECK(!callback.is_null());
-
- callback.Run(error);
-
- if (error == FILE_ERROR_OK)
- FOR_EACH_OBSERVER(FileCacheObserver,
- observers_,
- OnCachePinned(resource_id, md5));
-}
-
-void FileCache::OnUnpinned(const std::string& resource_id,
- const std::string& md5,
- const FileOperationCallback& callback,
- FileError error) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- DCHECK(!callback.is_null());
-
- callback.Run(error);
-
- if (error == FILE_ERROR_OK)
- FOR_EACH_OBSERVER(FileCacheObserver,
- observers_,
- OnCacheUnpinned(resource_id, md5));
-
- // Now the file is moved from "persistent" to "tmp" directory.
- // It's a chance to free up space if needed.
- blocking_task_runner_->PostTask(
- FROM_HERE,
- base::Bind(
- base::IgnoreResult(&FileCache::FreeDiskSpaceIfNeededFor),
- base::Unretained(this), 0));
-}
-
bool FileCache::HasEnoughSpaceFor(int64 num_bytes,
const base::FilePath& path) {
int64 free_space = 0;
diff --git a/chrome/browser/chromeos/drive/file_cache.h b/chrome/browser/chromeos/drive/file_cache.h
index 0c6d5e9..24b876a 100644
--- a/chrome/browser/chromeos/drive/file_cache.h
+++ b/chrome/browser/chromeos/drive/file_cache.h
@@ -12,7 +12,6 @@
#include "base/files/file_path.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
-#include "base/observer_list.h"
#include "chrome/browser/chromeos/drive/file_errors.h"
class Profile;
@@ -42,7 +41,6 @@
namespace internal {
class FileCacheMetadata;
-class FileCacheObserver;
// Callback for GetFileFromCache.
typedef base::Callback<void(FileError error,
@@ -115,14 +113,6 @@
// Can be called on any thread.
bool IsUnderFileCacheDirectory(const base::FilePath& path) const;
- // Adds observer.
- // Must be called on the UI thread.
- void AddObserver(FileCacheObserver* observer);
-
- // Removes observer.
- // Must be called on the UI thread.
- void RemoveObserver(FileCacheObserver* observer);
-
// Gets the cache entry for file corresponding to |resource_id| and |md5|
// and runs |callback| with true and the entry found if entry exists in cache
// map. Otherwise, runs |callback| with false.
@@ -343,18 +333,6 @@
// Used to implement ClearAllOnUIThread.
bool ClearAll();
- // Runs callback and notifies the observers when file is pinned.
- void OnPinned(const std::string& resource_id,
- const std::string& md5,
- const FileOperationCallback& callback,
- FileError error);
-
- // Runs callback and notifies the observers when file is unpinned.
- void OnUnpinned(const std::string& resource_id,
- const std::string& md5,
- const FileOperationCallback& callback,
- FileError error);
-
// Returns true if we have sufficient space to store the given number of
// bytes, while keeping kMinFreeSpace bytes on the disk.
bool HasEnoughSpaceFor(int64 num_bytes, const base::FilePath& path);
@@ -369,9 +347,6 @@
// The cache state data. This member must be access only on the blocking pool.
scoped_ptr<FileCacheMetadata> metadata_;
- // List of observers, this member must be accessed on UI thread.
- ObserverList<FileCacheObserver> observers_;
-
FreeDiskSpaceGetterInterface* free_disk_space_getter_; // Not owned.
// Note: This should remain the last member so it'll be destroyed and
diff --git a/chrome/browser/chromeos/drive/file_cache_observer.h b/chrome/browser/chromeos/drive/file_cache_observer.h
deleted file mode 100644
index d7792999..0000000
--- a/chrome/browser/chromeos/drive/file_cache_observer.h
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright (c) 2012 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_DRIVE_FILE_CACHE_OBSERVER_H_
-#define CHROME_BROWSER_CHROMEOS_DRIVE_FILE_CACHE_OBSERVER_H_
-
-#include <string>
-
-namespace drive {
-namespace internal {
-
-// Interface for classes that need to observe events from FileCache.
-// All events are notified on UI thread.
-class FileCacheObserver {
- public:
- // Triggered when a file has been pinned successfully.
- virtual void OnCachePinned(const std::string& resource_id,
- const std::string& md5) {}
-
- // Triggered when a file has been unpinned successfully.
- virtual void OnCacheUnpinned(const std::string& resource_id,
- const std::string& md5) {}
-
- protected:
- virtual ~FileCacheObserver() {}
-};
-
-} // namespace internal
-} // namespace drive
-
-#endif // CHROME_BROWSER_CHROMEOS_DRIVE_FILE_CACHE_OBSERVER_H_
diff --git a/chrome/browser/chromeos/drive/file_cache_unittest.cc b/chrome/browser/chromeos/drive/file_cache_unittest.cc
index daa1f68e..63b4701a 100644
--- a/chrome/browser/chromeos/drive/file_cache_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_cache_unittest.cc
@@ -15,16 +15,12 @@
#include "chrome/browser/chromeos/drive/drive.pb.h"
#include "chrome/browser/chromeos/drive/fake_free_disk_space_getter.h"
#include "chrome/browser/chromeos/drive/file_system_util.h"
-#include "chrome/browser/chromeos/drive/mock_file_cache_observer.h"
#include "chrome/browser/chromeos/drive/test_util.h"
#include "chrome/browser/google_apis/test_util.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/test/test_browser_thread_bundle.h"
-#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
-using ::testing::StrictMock;
-
namespace drive {
namespace internal {
namespace {
@@ -71,9 +67,6 @@
blocking_task_runner_,
fake_free_disk_space_getter_.get()));
- mock_cache_observer_.reset(new StrictMock<MockCacheObserver>);
- cache_->AddObserver(mock_cache_observer_.get());
-
bool success = false;
cache_->RequestInitialize(
google_apis::test_util::CreateCopyResultCallback(&success));
@@ -473,7 +466,6 @@
scoped_ptr<FileCache, test_util::DestroyHelperForTests> cache_;
scoped_ptr<FakeFreeDiskSpaceGetter> fake_free_disk_space_getter_;
- scoped_ptr<StrictMock<MockCacheObserver> > mock_cache_observer_;
FileError expected_error_;
int expected_cache_state_;
@@ -578,9 +570,6 @@
TEST_F(FileCacheTestOnUIThread, PinAndUnpin) {
std::string resource_id("pdf:1a2b");
std::string md5("abcdef0123456789");
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(2);
- EXPECT_CALL(*mock_cache_observer_, OnCacheUnpinned(resource_id, md5))
- .Times(1);
// First store a file to cache.
TestStoreToCache(resource_id, md5, dummy_file_path_,
@@ -608,9 +597,6 @@
// Pin a non-existent file in cache.
resource_id = "document:1a2b";
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
- EXPECT_CALL(*mock_cache_observer_, OnCacheUnpinned(resource_id, md5))
- .Times(1);
TestPin(resource_id, md5, FILE_ERROR_OK,
test_util::TEST_CACHE_STATE_PINNED,
@@ -624,9 +610,6 @@
// Unpin a file that doesn't exist in cache and is not pinned, i.e. cache
// has zero knowledge of the file.
resource_id = "not-in-cache:1a2b";
- // Because unpinning will fail, OnCacheUnpinned() won't be run.
- EXPECT_CALL(*mock_cache_observer_, OnCacheUnpinned(resource_id, md5))
- .Times(0);
TestUnpin(resource_id, md5, FILE_ERROR_NOT_FOUND,
test_util::TEST_CACHE_STATE_NONE,
@@ -636,7 +619,6 @@
TEST_F(FileCacheTestOnUIThread, StoreToCachePinned) {
std::string resource_id("pdf:1a2b");
std::string md5("abcdef0123456789");
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
// Pin a non-existent file.
TestPin(resource_id, md5, FILE_ERROR_OK,
@@ -664,7 +646,6 @@
TEST_F(FileCacheTestOnUIThread, GetFromCachePinned) {
std::string resource_id("pdf:1a2b");
std::string md5("abcdef0123456789");
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
// Pin a non-existent file.
TestPin(resource_id, md5, FILE_ERROR_OK,
@@ -692,7 +673,6 @@
// Use alphanumeric characters for resource_id.
std::string resource_id("pdf:1a2b");
std::string md5("abcdef0123456789");
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
// Store a file to cache, and pin it.
TestStoreToCache(resource_id, md5, dummy_file_path_,
@@ -710,7 +690,6 @@
// Repeat using non-alphanumeric characters for resource id, including '.'
// which is an extension separator.
resource_id = "pdf:`~!@#$%^&*()-_=+[{|]}\\;',<.>/?";
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
TestStoreToCache(resource_id, md5, dummy_file_path_,
FILE_ERROR_OK, test_util::TEST_CACHE_STATE_PRESENT,
@@ -749,7 +728,6 @@
TEST_F(FileCacheTestOnUIThread, DirtyCachePinned) {
std::string resource_id("pdf:1a2b");
std::string md5("abcdef0123456789");
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
// First store a file to cache and pin it.
TestStoreToCache(resource_id, md5, dummy_file_path_,
@@ -780,9 +758,6 @@
TEST_F(FileCacheTestOnUIThread, PinAndUnpinDirtyCache) {
std::string resource_id("pdf:1a2b");
std::string md5("abcdef0123456789");
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
- EXPECT_CALL(*mock_cache_observer_, OnCacheUnpinned(resource_id, md5))
- .Times(1);
// First store a file to cache and mark it as dirty.
TestStoreToCache(resource_id, md5, dummy_file_path_,
@@ -903,7 +878,6 @@
TEST_F(FileCacheTestOnUIThread, RemoveFromDirtyCache) {
std::string resource_id("pdf:1a2b");
std::string md5("abcdef0123456789");
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
// Store a file to cache, pin it, mark it dirty and commit it.
TestStoreToCache(resource_id, md5, dummy_file_path_,
@@ -965,17 +939,8 @@
TEST_F(FileCacheTestOnUIThread, Iterate) {
const std::vector<test_util::TestCacheResource> cache_resources(
test_util::GetDefaultTestCacheResources());
- // Set mock expectations.
- for (size_t i = 0; i < cache_resources.size(); ++i) {
- if (cache_resources[i].is_pinned) {
- EXPECT_CALL(*mock_cache_observer_,
- OnCachePinned(cache_resources[i].resource_id,
- cache_resources[i].md5)).Times(1);
- }
- }
- ASSERT_TRUE(test_util::PrepareTestCacheResources(
- cache_.get(),
- cache_resources));
+ ASSERT_TRUE(test_util::PrepareTestCacheResources(cache_.get(),
+ cache_resources));
std::vector<std::string> resource_ids;
std::vector<FileCacheEntry> cache_entries;
@@ -1069,7 +1034,6 @@
FileCache::CACHE_TYPE_TMP);
// Pin the file.
- EXPECT_CALL(*mock_cache_observer_, OnCachePinned(resource_id, md5)).Times(1);
TestPin(resource_id, md5,
FILE_ERROR_OK,
test_util::TEST_CACHE_STATE_PRESENT |
diff --git a/chrome/browser/chromeos/drive/file_system.cc b/chrome/browser/chromeos/drive/file_system.cc
index 2aaaa8b4..82e036b3 100644
--- a/chrome/browser/chromeos/drive/file_system.cc
+++ b/chrome/browser/chromeos/drive/file_system.cc
@@ -344,7 +344,22 @@
DCHECK(entry);
cache_->PinOnUIThread(entry->resource_id(),
- entry->file_specific_info().md5(), callback);
+ entry->file_specific_info().md5(),
+ base::Bind(&FileSystem::FinishPin,
+ weak_ptr_factory_.GetWeakPtr(),
+ callback,
+ entry->resource_id()));
+}
+
+void FileSystem::FinishPin(const FileOperationCallback& callback,
+ const std::string& resource_id,
+ FileError error) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(!callback.is_null());
+
+ if (error == FILE_ERROR_OK)
+ sync_client_->AddFetchTask(resource_id);
+ callback.Run(error);
}
void FileSystem::Unpin(const base::FilePath& file_path,
@@ -377,7 +392,22 @@
DCHECK(entry);
cache_->UnpinOnUIThread(entry->resource_id(),
- entry->file_specific_info().md5(), callback);
+ entry->file_specific_info().md5(),
+ base::Bind(&FileSystem::FinishUnpin,
+ weak_ptr_factory_.GetWeakPtr(),
+ callback,
+ entry->resource_id()));
+}
+
+void FileSystem::FinishUnpin(const FileOperationCallback& callback,
+ const std::string& resource_id,
+ FileError error) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(!callback.is_null());
+
+ if (error == FILE_ERROR_OK)
+ sync_client_->RemoveFetchTask(resource_id);
+ callback.Run(error);
}
void FileSystem::GetFileByPath(const base::FilePath& file_path,
diff --git a/chrome/browser/chromeos/drive/file_system.h b/chrome/browser/chromeos/drive/file_system.h
index 40bc7a6..a9f422e 100644
--- a/chrome/browser/chromeos/drive/file_system.h
+++ b/chrome/browser/chromeos/drive/file_system.h
@@ -203,11 +203,17 @@
void PinAfterGetResourceEntryByPath(const FileOperationCallback& callback,
FileError error,
scoped_ptr<ResourceEntry> entry);
+ void FinishPin(const FileOperationCallback& callback,
+ const std::string& resource_id,
+ FileError error);
// Used to implement Unpin().
void UnpinAfterGetResourceEntryByPath(const FileOperationCallback& callback,
FileError error,
scoped_ptr<ResourceEntry> entry);
+ void FinishUnpin(const FileOperationCallback& callback,
+ const std::string& resource_id,
+ FileError error);
// Part of OpenFile(). Called after the file downloading is completed.
void OpenFileAfterFileDownloaded(
diff --git a/chrome/browser/chromeos/drive/file_system/copy_operation_unittest.cc b/chrome/browser/chromeos/drive/file_system/copy_operation_unittest.cc
index 6a43fac..970815c 100644
--- a/chrome/browser/chromeos/drive/file_system/copy_operation_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_system/copy_operation_unittest.cc
@@ -5,7 +5,6 @@
#include "chrome/browser/chromeos/drive/file_system/copy_operation.h"
#include "base/file_util.h"
-#include "chrome/browser/chromeos/drive/file_cache_observer.h"
#include "chrome/browser/chromeos/drive/file_system/operation_test_base.h"
#include "chrome/browser/chromeos/drive/file_system_util.h"
#include "chrome/browser/google_apis/fake_drive_service.h"
diff --git a/chrome/browser/chromeos/drive/file_system_unittest.cc b/chrome/browser/chromeos/drive/file_system_unittest.cc
index 16cb257..9cecbd5 100644
--- a/chrome/browser/chromeos/drive/file_system_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_system_unittest.cc
@@ -20,7 +20,6 @@
#include "chrome/browser/chromeos/drive/file_system_util.h"
#include "chrome/browser/chromeos/drive/job_scheduler.h"
#include "chrome/browser/chromeos/drive/mock_directory_change_observer.h"
-#include "chrome/browser/chromeos/drive/mock_file_cache_observer.h"
#include "chrome/browser/chromeos/drive/sync_client.h"
#include "chrome/browser/chromeos/drive/test_util.h"
#include "chrome/browser/google_apis/drive_api_parser.h"
@@ -92,9 +91,6 @@
blocking_task_runner_,
fake_free_disk_space_getter_.get()));
- mock_cache_observer_.reset(new StrictMock<MockCacheObserver>);
- cache_->AddObserver(mock_cache_observer_.get());
-
mock_directory_observer_.reset(new StrictMock<MockDirectoryChangeObserver>);
bool success = false;
@@ -341,7 +337,6 @@
scoped_ptr<internal::ResourceMetadata, test_util::DestroyHelperForTests>
resource_metadata_;
scoped_ptr<FakeFreeDiskSpaceGetter> fake_free_disk_space_getter_;
- scoped_ptr<StrictMock<MockCacheObserver> > mock_cache_observer_;
scoped_ptr<StrictMock<MockDirectoryChangeObserver> > mock_directory_observer_;
};
@@ -682,23 +677,58 @@
// Pin the file.
FileError error = FILE_ERROR_FAILED;
- EXPECT_CALL(*mock_cache_observer_,
- OnCachePinned(entry->resource_id(),
- entry->file_specific_info().md5())).Times(1);
file_system_->Pin(file_path,
google_apis::test_util::CreateCopyResultCallback(&error));
google_apis::test_util::RunBlockingPoolTask();
EXPECT_EQ(FILE_ERROR_OK, error);
+ FileCacheEntry cache_entry;
+ EXPECT_TRUE(GetCacheEntryFromOriginThread(
+ entry->resource_id(), std::string(), &cache_entry));
+ EXPECT_TRUE(cache_entry.is_pinned());
+ EXPECT_TRUE(cache_entry.is_present());
+ EXPECT_TRUE(cache_entry.is_persistent());
+
// Unpin the file.
error = FILE_ERROR_FAILED;
- EXPECT_CALL(*mock_cache_observer_,
- OnCacheUnpinned(entry->resource_id(),
- entry->file_specific_info().md5())).Times(1);
file_system_->Unpin(file_path,
google_apis::test_util::CreateCopyResultCallback(&error));
google_apis::test_util::RunBlockingPoolTask();
EXPECT_EQ(FILE_ERROR_OK, error);
+
+ EXPECT_TRUE(GetCacheEntryFromOriginThread(
+ entry->resource_id(), std::string(), &cache_entry));
+ EXPECT_FALSE(cache_entry.is_pinned());
+}
+
+TEST_F(FileSystemTest, PinAndUnpin_NotSynced) {
+ ASSERT_TRUE(LoadFullResourceList());
+
+ base::FilePath file_path(FILE_PATH_LITERAL("drive/root/File 1.txt"));
+
+ // Get the file info.
+ scoped_ptr<ResourceEntry> entry(GetResourceEntryByPathSync(file_path));
+ ASSERT_TRUE(entry);
+
+ // Unpin the file just after pinning. File fetch should be cancelled.
+ FileError error_pin = FILE_ERROR_FAILED;
+ file_system_->Pin(
+ file_path,
+ google_apis::test_util::CreateCopyResultCallback(&error_pin));
+
+ FileError error_unpin = FILE_ERROR_FAILED;
+ file_system_->Unpin(
+ file_path,
+ google_apis::test_util::CreateCopyResultCallback(&error_unpin));
+
+ google_apis::test_util::RunBlockingPoolTask();
+ EXPECT_EQ(FILE_ERROR_OK, error_pin);
+ EXPECT_EQ(FILE_ERROR_OK, error_unpin);
+
+ // No cache file available because the sync was cancelled by Unpin().
+ FileCacheEntry cache_entry;
+ EXPECT_FALSE(GetCacheEntryFromOriginThread(
+ entry->resource_id(), std::string(), &cache_entry));
}
TEST_F(FileSystemTest, GetAvailableSpace) {
diff --git a/chrome/browser/chromeos/drive/mock_file_cache_observer.cc b/chrome/browser/chromeos/drive/mock_file_cache_observer.cc
deleted file mode 100644
index 0f7868be..0000000
--- a/chrome/browser/chromeos/drive/mock_file_cache_observer.cc
+++ /dev/null
@@ -1,15 +0,0 @@
-// Copyright (c) 2012 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 "chrome/browser/chromeos/drive/mock_file_cache_observer.h"
-
-namespace drive {
-
-MockCacheObserver::MockCacheObserver() {
-}
-
-MockCacheObserver::~MockCacheObserver() {
-}
-
-} // namespace drive
diff --git a/chrome/browser/chromeos/drive/mock_file_cache_observer.h b/chrome/browser/chromeos/drive/mock_file_cache_observer.h
deleted file mode 100644
index 0232ccf..0000000
--- a/chrome/browser/chromeos/drive/mock_file_cache_observer.h
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright (c) 2012 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_DRIVE_MOCK_FILE_CACHE_OBSERVER_H_
-#define CHROME_BROWSER_CHROMEOS_DRIVE_MOCK_FILE_CACHE_OBSERVER_H_
-
-#include <string>
-
-#include "chrome/browser/chromeos/drive/file_cache_observer.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-namespace drive {
-
-// Mock for FileCacheObserver.
-class MockCacheObserver : public internal::FileCacheObserver {
- public:
- MockCacheObserver();
- virtual ~MockCacheObserver();
-
- MOCK_METHOD2(OnCachePinned, void(const std::string& resource_id,
- const std::string& md5));
- MOCK_METHOD2(OnCacheUnpinned, void(const std::string& resource_id,
- const std::string& md5));
-};
-
-} // namespace drive
-
-#endif // CHROME_BROWSER_CHROMEOS_DRIVE_MOCK_FILE_CACHE_OBSERVER_H_
diff --git a/chrome/browser/chromeos/drive/sync_client.cc b/chrome/browser/chromeos/drive/sync_client.cc
index 42064d8..aa29172 100644
--- a/chrome/browser/chromeos/drive/sync_client.cc
+++ b/chrome/browser/chromeos/drive/sync_client.cc
@@ -78,15 +78,10 @@
delay_(base::TimeDelta::FromSeconds(kDelaySeconds)),
weak_ptr_factory_(this) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- DCHECK(cache);
-
- cache_->AddObserver(this);
}
SyncClient::~SyncClient() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- cache_->RemoveObserver(this);
}
void SyncClient::StartProcessingBacklog() {
@@ -110,40 +105,19 @@
base::Bind(&base::DoNothing));
}
-std::vector<std::string> SyncClient::GetResourceIdsForTesting(
- SyncType sync_type) const {
- switch (sync_type) {
- case FETCH:
- return std::vector<std::string>(fetch_list_.begin(), fetch_list_.end());
- case UPLOAD:
- return std::vector<std::string>(upload_list_.begin(), upload_list_.end());
- }
- NOTREACHED();
- return std::vector<std::string>();
-}
-
-void SyncClient::OnCachePinned(const std::string& resource_id,
- const std::string& md5) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- AddTaskToQueue(FETCH, resource_id);
-}
-
-void SyncClient::OnCacheUnpinned(const std::string& resource_id,
- const std::string& md5) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- // Remove the resource_id if it's in the queue. This can happen if the
- // user cancels pinning before the file is fetched.
- pending_fetch_list_.erase(resource_id);
-}
-
void SyncClient::AddFetchTask(const std::string& resource_id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
AddTaskToQueue(FETCH, resource_id);
}
+void SyncClient::RemoveFetchTask(const std::string& resource_id) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+ // TODO(kinaba): Cancel tasks in JobScheduler as well. crbug.com/248856
+ pending_fetch_list_.erase(resource_id);
+}
+
void SyncClient::AddUploadTask(const std::string& resource_id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
diff --git a/chrome/browser/chromeos/drive/sync_client.h b/chrome/browser/chromeos/drive/sync_client.h
index 449458e1..bbc53af 100644
--- a/chrome/browser/chromeos/drive/sync_client.h
+++ b/chrome/browser/chromeos/drive/sync_client.h
@@ -12,7 +12,6 @@
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/time.h"
-#include "chrome/browser/chromeos/drive/file_cache_observer.h"
#include "chrome/browser/chromeos/drive/file_errors.h"
namespace base {
@@ -37,19 +36,12 @@
class ResourceMetadata;
// The SyncClient is used to synchronize pinned files on Drive and the
-// cache on the local drive. The sync client works as follows.
-//
-// When the user pins files on Drive, this client is notified about the files
-// that get pinned, and queues tasks and starts fetching these files in the
-// background.
-//
-// When the user unpins files on Drive, this client is notified about the
-// files that get unpinned, cancels tasks if these are still in the queue.
+// cache on the local drive.
//
// If the user logs out before fetching of the pinned files is complete, this
// client resumes fetching operations next time the user logs in, based on
// the states left in the cache.
-class SyncClient : public FileCacheObserver {
+class SyncClient {
public:
// Types of sync tasks.
enum SyncType {
@@ -64,15 +56,12 @@
FileCache* cache);
virtual ~SyncClient();
- // FileCache::Observer overrides.
- virtual void OnCachePinned(const std::string& resource_id,
- const std::string& md5) OVERRIDE;
- virtual void OnCacheUnpinned(const std::string& resource_id,
- const std::string& md5) OVERRIDE;
-
// Adds a fetch task to the queue.
void AddFetchTask(const std::string& resource_id);
+ // Removes a fetch task from the queue.
+ void RemoveFetchTask(const std::string& resource_id);
+
// Adds an upload task to the queue.
void AddUploadTask(const std::string& resource_id);
@@ -86,10 +75,6 @@
// are added to the queue and the sync loop is started.
void StartCheckingExistingPinnedFiles();
- // Returns the resource IDs in |queue_| for the given sync type. Used only
- // for testing.
- std::vector<std::string> GetResourceIdsForTesting(SyncType sync_type) const;
-
// Sets a delay for testing.
void set_delay_for_testing(const base::TimeDelta& delay) {
delay_ = delay;
diff --git a/chrome/browser/chromeos/drive/sync_client_unittest.cc b/chrome/browser/chromeos/drive/sync_client_unittest.cc
index 1c5b88c..933cd9c5 100644
--- a/chrome/browser/chromeos/drive/sync_client_unittest.cc
+++ b/chrome/browser/chromeos/drive/sync_client_unittest.cc
@@ -226,10 +226,8 @@
EXPECT_FALSE(cache_entry.is_dirty());
}
-TEST_F(SyncClientTest, OnCachePinned) {
- // This file will be fetched by GetFileByResourceId() as OnCachePinned()
- // will kick off the sync loop.
- sync_client_->OnCachePinned(resource_ids_["foo"], std::string());
+TEST_F(SyncClientTest, AddFetchTask) {
+ sync_client_->AddFetchTask(resource_ids_["foo"]);
base::RunLoop().RunUntilIdle();
FileCacheEntry cache_entry;
@@ -238,15 +236,11 @@
EXPECT_TRUE(cache_entry.is_present());
}
-TEST_F(SyncClientTest, OnCachePinnedAndCancelled) {
- drive_service_->set_resource_id_to_be_cancelled(resource_ids_["foo"]);
+TEST_F(SyncClientTest, AddFetchTaskAndCancelled) {
// Trigger fetching of a file which results in cancellation.
- FileError error = FILE_ERROR_FAILED;
- cache_->PinOnUIThread(
- resource_ids_["foo"], std::string(),
- google_apis::test_util::CreateCopyResultCallback(&error));
+ drive_service_->set_resource_id_to_be_cancelled(resource_ids_["foo"]);
+ sync_client_->AddFetchTask(resource_ids_["foo"]);
base::RunLoop().RunUntilIdle();
- EXPECT_EQ(FILE_ERROR_OK, error);
// The file should be unpinned if the user wants the download to be cancelled.
FileCacheEntry cache_entry;
@@ -254,15 +248,13 @@
&cache_entry));
}
-TEST_F(SyncClientTest, OnCacheUnpinned) {
+TEST_F(SyncClientTest, RemoveFetchTask) {
sync_client_->AddFetchTask(resource_ids_["foo"]);
sync_client_->AddFetchTask(resource_ids_["bar"]);
sync_client_->AddFetchTask(resource_ids_["baz"]);
- ASSERT_EQ(3U,
- sync_client_->GetResourceIdsForTesting(SyncClient::FETCH).size());
- sync_client_->OnCacheUnpinned(resource_ids_["foo"], std::string());
- sync_client_->OnCacheUnpinned(resource_ids_["baz"], std::string());
+ sync_client_->RemoveFetchTask(resource_ids_["foo"]);
+ sync_client_->RemoveFetchTask(resource_ids_["baz"]);
base::RunLoop().RunUntilIdle();
// Only "bar" should be fetched.
@@ -281,19 +273,6 @@
}
-TEST_F(SyncClientTest, Deduplication) {
- sync_client_->AddFetchTask(resource_ids_["foo"]);
-
- // Set the delay so that DoSyncLoop() is delayed.
- sync_client_->set_delay_for_testing(TestTimeouts::action_max_timeout());
- // Raise OnCachePinned() event. This shouldn't result in adding the second
- // task, as tasks are de-duplicated.
- sync_client_->OnCachePinned(resource_ids_["foo"], std::string());
-
- ASSERT_EQ(1U,
- sync_client_->GetResourceIdsForTesting(SyncClient::FETCH).size());
-}
-
TEST_F(SyncClientTest, ExistingPinnedFiles) {
// Start checking the existing pinned files. This will collect the resource
// IDs of pinned files, with stale local cache files.
diff --git a/chrome/chrome_browser_chromeos.gypi b/chrome/chrome_browser_chromeos.gypi
index 942230b..335be82 100644
--- a/chrome/chrome_browser_chromeos.gypi
+++ b/chrome/chrome_browser_chromeos.gypi
@@ -234,7 +234,6 @@
'browser/chromeos/drive/file_cache.h',
'browser/chromeos/drive/file_cache_metadata.cc',
'browser/chromeos/drive/file_cache_metadata.h',
- 'browser/chromeos/drive/file_cache_observer.h',
'browser/chromeos/drive/file_change.cc',
'browser/chromeos/drive/file_change.h',
'browser/chromeos/drive/file_errors.cc',
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index c3ae1c6..dd37225 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -598,8 +598,6 @@
'browser/chromeos/drive/local_file_reader_unittest.cc',
'browser/chromeos/drive/mock_directory_change_observer.cc',
'browser/chromeos/drive/mock_directory_change_observer.h',
- 'browser/chromeos/drive/mock_file_cache_observer.cc',
- 'browser/chromeos/drive/mock_file_cache_observer.h',
'browser/chromeos/drive/remove_stale_cache_files_unittest.cc',
'browser/chromeos/drive/resource_entry_conversion_unittest.cc',
'browser/chromeos/drive/resource_metadata_storage_unittest.cc',
