diff options
author | kinaba@chromium.org <kinaba@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-26 10:00:36 +0000 |
---|---|---|
committer | kinaba@chromium.org <kinaba@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-26 10:00:36 +0000 |
commit | fa600b39f944c9359b95492b84991197e3393d4c (patch) | |
tree | d081656ecb82ae07ae3615082c85a8139f2245bc | |
parent | 9437e9a9575193fa442c12dc8a7b4ec5a6efe38a (diff) | |
download | chromium_src-fa600b39f944c9359b95492b84991197e3393d4c.zip chromium_src-fa600b39f944c9359b95492b84991197e3393d4c.tar.gz chromium_src-fa600b39f944c9359b95492b84991197e3393d4c.tar.bz2 |
drive: Fix local metadata going out of sync for offline boot.
This CL merges FROM_CACHE and FROM_SERVER to a single LOADED state.
FROM_CACHE is now indicating the state that "the feed was loaded from
cache, but the subsequent initial feed refresh somehow failed".
FROM_SERVER is for "the initial (delta-)feed was successfully fetched".
The only difference is that in FROM_SERVER state we do refreshing by delta
feed, while we don't in FROM_CACHE. However, what it means is, if the
initial feed refresh fails (=>FROM_CACHE), our metadata is not refreshed
anymore until until the user logs in again. This is bad, see the TEST scenario.
There shouldn't be any problem to request delta feed upon cached metadata.
BUG=157098, 157857
TEST=
1. Boot and login to ChromeOS on an offline machine.
2. Open Files.app and open Drive folder.
3. Connect to network.
4. Go drive.google.com and create a new folder.
5. Wait a while, and verify the new folder appears in Files.app.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=164296
Review URL: https://codereview.chromium.org/11288005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@164310 0039d316-1c4b-4281-b951-d872f2087c98
6 files changed, 64 insertions, 28 deletions
diff --git a/chrome/browser/chromeos/drive/drive_feed_loader.cc b/chrome/browser/chromeos/drive/drive_feed_loader.cc index cab8142..24f56b2 100644 --- a/chrome/browser/chromeos/drive/drive_feed_loader.cc +++ b/chrome/browser/chromeos/drive/drive_feed_loader.cc @@ -329,10 +329,7 @@ void DriveFeedLoader::OnGetAccountMetadata( << ", server = " << account_metadata->largest_changestamp(); } - // If our cache holds the latest state from the server, change the - // state to FROM_SERVER. - resource_metadata_->set_origin( - initial_origin == FROM_CACHE ? FROM_SERVER : initial_origin); + resource_metadata_->set_origin(initial_origin); changes_detected = false; } @@ -390,10 +387,7 @@ void DriveFeedLoader::OnGetAboutResource( << ", server = " << largest_changestamp; } - // If our cache holds the latest state from the server, change the - // state to FROM_SERVER. - resource_metadata_->set_origin( - initial_origin == FROM_CACHE ? FROM_SERVER : initial_origin); + resource_metadata_->set_origin(initial_origin); changes_detected = false; } @@ -793,7 +787,7 @@ void DriveFeedLoader::OnProtoLoaded(LoadRootFeedParams* params) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); // If we have already received updates from the server, bail out. - if (resource_metadata_->origin() == FROM_SERVER) + if (resource_metadata_->origin() == INITIALIZED) return; // Update directory structure only if everything is OK and we haven't yet @@ -843,8 +837,8 @@ void DriveFeedLoader::ContinueWithInitializedResourceMetadata( ContentOrigin initial_origin = UNINITIALIZED; if (resource_metadata_->origin() != INITIALIZING) { // If directory content is already initialized, restore content origin - // to FROM_CACHE in case of failure. - initial_origin = FROM_CACHE; + // to INITIALIZED in case of failure. + initial_origin = INITIALIZED; resource_metadata_->set_origin(REFRESHING); } diff --git a/chrome/browser/chromeos/drive/drive_feed_processor.cc b/chrome/browser/chromeos/drive/drive_feed_processor.cc index fc36a44..3394f58 100644 --- a/chrome/browser/chromeos/drive/drive_feed_processor.cc +++ b/chrome/browser/chromeos/drive/drive_feed_processor.cc @@ -56,7 +56,7 @@ void DriveFeedProcessor::ApplyFeeds( std::set<FilePath>* changed_dirs) { bool is_delta_feed = start_changestamp != 0; - resource_metadata_->set_origin(FROM_SERVER); + resource_metadata_->set_origin(INITIALIZED); int64 delta_feed_changestamp = 0; FeedToEntryProtoMapUMAStats uma_stats; diff --git a/chrome/browser/chromeos/drive/drive_file_system.cc b/chrome/browser/chromeos/drive/drive_file_system.cc index e4d19e4..9ae0c86 100644 --- a/chrome/browser/chromeos/drive/drive_file_system.cc +++ b/chrome/browser/chromeos/drive/drive_file_system.cc @@ -439,7 +439,7 @@ void DriveFileSystem::CheckForUpdates() { DVLOG(1) << "CheckForUpdates"; ContentOrigin initial_origin = resource_metadata_->origin(); - if (initial_origin == FROM_SERVER) { + if (initial_origin == INITIALIZED) { resource_metadata_->set_origin(REFRESHING); feed_loader_->ReloadFromServerIfNeeded( initial_origin, diff --git a/chrome/browser/chromeos/drive/drive_file_system_unittest.cc b/chrome/browser/chromeos/drive/drive_file_system_unittest.cc index fc6565c..ac2eeff 100644 --- a/chrome/browser/chromeos/drive/drive_file_system_unittest.cc +++ b/chrome/browser/chromeos/drive/drive_file_system_unittest.cc @@ -130,6 +130,13 @@ ACTION_P2(MockCopyDocument, status, value) { base::Bind(arg2, status, base::Passed(value))); } +ACTION(MockFailingGetDocuments) { + base::MessageLoopProxy::current()->PostTask( + FROM_HERE, + base::Bind(arg4, google_apis::GDATA_NO_CONNECTION, + base::Passed(scoped_ptr<base::Value>()))); +} + // Counts the number of files (not directories) in |entries|. int CountFiles(const DriveEntryProtoVector& entries) { int num_files = 0; @@ -593,16 +600,23 @@ class DriveFileSystemTest : public testing::Test { google_apis::test_util::RunBlockingPoolTask(); } + // Flag for specifying the timestamp of the test filesystem cache. + enum SaveTestFileSystemParam { + USE_OLD_TIMESTAMP, + USE_SERVER_TIMESTAMP, + }; + // Creates a proto file representing a filesystem with directories: // drive, drive/Dir1, drive/Dir1/SubDir2 // and files // drive/File1, drive/Dir1/File2, drive/Dir1/SubDir2/File3. - // Sets the changestamp to 654321, equal to that of "account_metadata.json" - // test data, indicating the cache is holding the latest file system info. - void SaveTestFileSystem() { + // If |use_up_to_date_timestamp| is true, sets the changestamp to 654321, + // equal to that of "account_metadata.json" test data, indicating the cache is + // holding the latest file system info. + void SaveTestFileSystem(SaveTestFileSystemParam param) { DriveRootDirectoryProto root; root.set_version(kProtoVersion); - root.set_largest_changestamp(654321); + root.set_largest_changestamp(param == USE_SERVER_TIMESTAMP ? 654321 : 0); DriveDirectoryProto* root_dir = root.mutable_drive_directory(); DriveEntryProto* dir_base = root_dir->mutable_drive_entry(); PlatformFileInfoProto* platform_info = dir_base->mutable_file_info(); @@ -1159,7 +1173,7 @@ TEST_F(DriveFileSystemTest, ChangeFeed_FileRenamedInDirectory) { } TEST_F(DriveFileSystemTest, CachedFeedLoading) { - SaveTestFileSystem(); + SaveTestFileSystem(USE_OLD_TIMESTAMP); TestLoadMetadataFromCache(); EXPECT_TRUE(EntryExists(FilePath(FILE_PATH_LITERAL("drive/File1")))); @@ -1171,7 +1185,7 @@ TEST_F(DriveFileSystemTest, CachedFeedLoading) { } TEST_F(DriveFileSystemTest, CachedFeadLoadingThenServerFeedLoading) { - SaveTestFileSystem(); + SaveTestFileSystem(USE_SERVER_TIMESTAMP); // SaveTestFileSystem and "account_metadata.json" have the same changestamp, // so no request for new feeds (i.e., call to GetDocuments) should happen. @@ -1186,13 +1200,44 @@ TEST_F(DriveFileSystemTest, CachedFeadLoadingThenServerFeedLoading) { EXPECT_TRUE(EntryExists(FilePath(FILE_PATH_LITERAL("drive/File1")))); // Since the file system has verified that it holds the latest snapshot, - // it should change its state to FROM_SERVER, which admits periodic refresh. + // it should change its state to INITIALIZED, which admits periodic refresh. + // To test it, call CheckForUpdates and verify it does try to check updates. + mock_drive_service_->set_account_metadata( + google_apis::test_util::LoadJSONFile( + "gdata/account_metadata.json").release()); + EXPECT_CALL(*mock_drive_service_, GetAccountMetadata(_)).Times(1); + EXPECT_CALL(*mock_webapps_registry_, UpdateFromFeed(_)).Times(1); + + file_system_->CheckForUpdates(); + google_apis::test_util::RunBlockingPoolTask(); +} + +TEST_F(DriveFileSystemTest, OfflineCachedFeedLoading) { + SaveTestFileSystem(USE_OLD_TIMESTAMP); + + mock_drive_service_->set_account_metadata( + google_apis::test_util::LoadJSONFile( + "gdata/account_metadata.json").release()); + EXPECT_CALL(*mock_drive_service_, GetAccountMetadata(_)).Times(1); + EXPECT_CALL(*mock_webapps_registry_, UpdateFromFeed(_)).Times(1); + + // Make GetDocuments fail for simulating offline situation. This will leave + // the file system "loaded from cache, but not synced with server" state. + EXPECT_CALL(*mock_drive_service_, GetDocuments(_, _, _, _, _)) + .WillOnce(MockFailingGetDocuments()); + + // Kicks loading of cached file system and query for server update. + EXPECT_TRUE(EntryExists(FilePath(FILE_PATH_LITERAL("drive/File1")))); + + // Since the file system has at least succeeded to load cached snapshot, + // the file system should be able to start periodic refresh. // To test it, call CheckForUpdates and verify it does try to check updates. mock_drive_service_->set_account_metadata( google_apis::test_util::LoadJSONFile( "gdata/account_metadata.json").release()); EXPECT_CALL(*mock_drive_service_, GetAccountMetadata(_)).Times(1); EXPECT_CALL(*mock_webapps_registry_, UpdateFromFeed(_)).Times(1); + EXPECT_CALL(*mock_drive_service_, GetDocuments(_, _, _, _, _)).Times(1); file_system_->CheckForUpdates(); google_apis::test_util::RunBlockingPoolTask(); diff --git a/chrome/browser/chromeos/drive/drive_resource_metadata.cc b/chrome/browser/chromeos/drive/drive_resource_metadata.cc index a6f780c..f8a70ce 100644 --- a/chrome/browser/chromeos/drive/drive_resource_metadata.cc +++ b/chrome/browser/chromeos/drive/drive_resource_metadata.cc @@ -53,8 +53,7 @@ std::string ContentOriginToString(ContentOrigin origin) { case UNINITIALIZED: return "UNINITIALIZED"; case INITIALIZING: return "INITIALIZING"; case REFRESHING: return "REFRESHING"; - case FROM_CACHE: return "FROM_CACHE"; - case FROM_SERVER: return "FROM_SERVER"; + case INITIALIZED: return "INITIALIZED"; }; NOTREACHED(); return "(unknown content origin)"; @@ -740,7 +739,7 @@ void DriveResourceMetadata::InitResourceMap( DCHECK_EQ(resource_map.size(), resource_map_.size()); DCHECK_EQ(resource_map.size(), serialized_resources->size()); - origin_ = FROM_CACHE; + origin_ = INITIALIZED; callback.Run(DRIVE_FILE_OK); } @@ -809,7 +808,7 @@ bool DriveResourceMetadata::ParseFromString( root_->FromProto(proto.drive_directory()); - origin_ = FROM_CACHE; + origin_ = INITIALIZED; largest_changestamp_ = proto.largest_changestamp(); return true; diff --git a/chrome/browser/chromeos/drive/drive_resource_metadata.h b/chrome/browser/chromeos/drive/drive_resource_metadata.h index 2ab0f76..2a7ae53 100644 --- a/chrome/browser/chromeos/drive/drive_resource_metadata.h +++ b/chrome/browser/chromeos/drive/drive_resource_metadata.h @@ -51,10 +51,8 @@ enum ContentOrigin { INITIALIZING, // Content is initialized, but during refreshing. REFRESHING, - // Content is initialized from disk cache. - FROM_CACHE, - // Content is initialized from the direct server response. - FROM_SERVER, + // Content is initialized. + INITIALIZED, }; // Converts a ContentOrigin constant to a string of its name. |