Skip to content

GitLab

T
tangled
Commit ba5771c7 authored by satorux@chromium.org's avatar satorux@chromium.org
Browse files
Options

gdata: Fix a crash when adding a new file to a new but deleted directory. 

FindDirectoryForNewEntry() was crashing when:
1) a new file is added to a new directory in the same delta feed
2) but the new directory entry is marked "deleted" (moved to trash)

BUG=127495
TEST=added a unit test for the fix

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@138368 0039d316-1c4b-4281-b951-d872f2087c98
parent 2b206813
Hide whitespace changes
Inline Side-by-side
Showing with 284 additions and 10 deletions
chrome/browser/chromeos/gdata/gdata_file_system.cc
View file @ ba5771c7
...@@ -3643,7 +3643,7 @@ base::PlatformFileError GDataFileSystem::UpdateFromFeed( ...@@ -3643,7 +3643,7 @@ base::PlatformFileError GDataFileSystem::UpdateFromFeed(
ApplyFeedFromFileUrlMap( ApplyFeedFromFileUrlMap(
is_delta_feed, is_delta_feed,
is_delta_feed ? delta_feed_changestamp : root_feed_changestamp, is_delta_feed ? delta_feed_changestamp : root_feed_changestamp,
file_map); &file_map);
if (should_notify_initial_load) if (should_notify_initial_load)
NotifyInitialLoadFinished(); NotifyInitialLoadFinished();
...@@ -3659,7 +3659,7 @@ base::PlatformFileError GDataFileSystem::UpdateFromFeed( ...@@ -3659,7 +3659,7 @@ base::PlatformFileError GDataFileSystem::UpdateFromFeed(
void GDataFileSystem::ApplyFeedFromFileUrlMap( void GDataFileSystem::ApplyFeedFromFileUrlMap(
bool is_delta_feed, bool is_delta_feed,
int feed_changestamp, int feed_changestamp,
const FileResourceIdMap& file_map) { FileResourceIdMap* file_map) {
lock_.AssertAcquired(); lock_.AssertAcquired();
// Don't send directory content change notification while performing // Don't send directory content change notification while performing
// the initial content retrieval. // the initial content retrieval.
...@@ -3677,10 +3677,15 @@ void GDataFileSystem::ApplyFeedFromFileUrlMap( ...@@ -3677,10 +3677,15 @@ void GDataFileSystem::ApplyFeedFromFileUrlMap(
new GDataRootDirectory); new GDataRootDirectory);
// Go through all entires generated by the feed and apply them to the local // Go through all entires generated by the feed and apply them to the local
// snapshot of the file system. // snapshot of the file system.
for (FileResourceIdMap::const_iterator it = file_map.begin(); for (FileResourceIdMap::iterator it = file_map->begin();
it != file_map.end(); ++it) { it != file_map->end();) {
// Ensure that the entry is deleted, unless the ownership is explicitly
// transferred by entry.release().
scoped_ptr<GDataEntry> entry(it->second); scoped_ptr<GDataEntry> entry(it->second);
DCHECK_EQ(it->first, entry->resource_id()); DCHECK_EQ(it->first, entry->resource_id());
// Erase the entry so the deleted entry won't be referenced.
file_map->erase(it++);
GDataEntry* old_entry = root_->GetEntryByResourceId(entry->resource_id()); GDataEntry* old_entry = root_->GetEntryByResourceId(entry->resource_id());
GDataDirectory* dest_dir = NULL; GDataDirectory* dest_dir = NULL;
if (entry->is_deleted()) { // Deleted file/directory. if (entry->is_deleted()) { // Deleted file/directory.
...@@ -3716,7 +3721,7 @@ void GDataFileSystem::ApplyFeedFromFileUrlMap( ...@@ -3716,7 +3721,7 @@ void GDataFileSystem::ApplyFeedFromFileUrlMap(
if (dest_dir->resource_id() != entry->parent_resource_id()) { if (dest_dir->resource_id() != entry->parent_resource_id()) {
changed_dirs.insert(dest_dir->GetFilePath()); changed_dirs.insert(dest_dir->GetFilePath());
dest_dir = FindDirectoryForNewEntry(entry.get(), dest_dir = FindDirectoryForNewEntry(entry.get(),
file_map, *file_map,
orphaned_entries_dir.get()); orphaned_entries_dir.get());
} }
DCHECK(dest_dir); DCHECK(dest_dir);
...@@ -3727,7 +3732,7 @@ void GDataFileSystem::ApplyFeedFromFileUrlMap( ...@@ -3727,7 +3732,7 @@ void GDataFileSystem::ApplyFeedFromFileUrlMap(
&changed_dirs); &changed_dirs);
} else { // Adding a new file. } else { // Adding a new file.
dest_dir = FindDirectoryForNewEntry(entry.get(), dest_dir = FindDirectoryForNewEntry(entry.get(),
file_map, *file_map,
orphaned_entries_dir.get()); orphaned_entries_dir.get());
DCHECK(dest_dir); DCHECK(dest_dir);
AddEntryToDirectoryAndCollectChangedDirectories( AddEntryToDirectoryAndCollectChangedDirectories(
...@@ -3744,6 +3749,8 @@ void GDataFileSystem::ApplyFeedFromFileUrlMap( ...@@ -3744,6 +3749,8 @@ void GDataFileSystem::ApplyFeedFromFileUrlMap(
changed_dirs.insert(dest_dir->GetFilePath()); changed_dirs.insert(dest_dir->GetFilePath());
} }
} }
// All entry must be erased from the map.
DCHECK(file_map->empty());
if (should_notify_directory_changed) { if (should_notify_directory_changed) {
for (std::set<FilePath>::iterator dir_iter = changed_dirs.begin(); for (std::set<FilePath>::iterator dir_iter = changed_dirs.begin();
...@@ -3773,9 +3780,10 @@ GDataDirectory* GDataFileSystem::FindDirectoryForNewEntry( ...@@ -3773,9 +3780,10 @@ GDataDirectory* GDataFileSystem::FindDirectoryForNewEntry(
dir = (find_iter != file_map.end() && dir = (find_iter != file_map.end() &&
find_iter->second) ? find_iter->second) ?
find_iter->second->AsGDataDirectory() : NULL; find_iter->second->AsGDataDirectory() : NULL;
DVLOG(1) << "Found parent for " << new_entry->file_name() if (dir) {
<< " in map " << parent_id; DVLOG(1) << "Found parent for " << new_entry->file_name()
if (!dir) { << " in file_map " << parent_id;
} else {
DVLOG(1) << "Adding orphan " << new_entry->GetFilePath().value(); DVLOG(1) << "Adding orphan " << new_entry->GetFilePath().value();
dir = orphaned_entries_dir; dir = orphaned_entries_dir;
} }
......
chrome/browser/chromeos/gdata/gdata_file_system.h
View file @ ba5771c7
...@@ -894,9 +894,11 @@ class GDataFileSystem : public GDataFileSystemInterface, ...@@ -894,9 +894,11 @@ class GDataFileSystem : public GDataFileSystemInterface,
int root_feed_changestamp); int root_feed_changestamp);
// Applies the pre-processed feed from |file_map| map onto the file system. // Applies the pre-processed feed from |file_map| map onto the file system.
// All entries in |file_map| will be erased (i.e. the map becomes empty),
// and values are deleted.
void ApplyFeedFromFileUrlMap(bool is_delta_feed, void ApplyFeedFromFileUrlMap(bool is_delta_feed,
int feed_changestamp, int feed_changestamp,
const FileResourceIdMap& file_map); FileResourceIdMap* file_map);
// Finds directory where new |file| should be added to during feed processing. // Finds directory where new |file| should be added to during feed processing.
// |orphaned_entries_dir| collects files/dirs that don't have a parent in // |orphaned_entries_dir| collects files/dirs that don't have a parent in
......
chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
View file @ ba5771c7
...@@ -1434,6 +1434,18 @@ TEST_F(GDataFileSystemTest, ChangeFeed_AddFileToNewDirectory) { ...@@ -1434,6 +1434,18 @@ TEST_F(GDataFileSystemTest, ChangeFeed_AddFileToNewDirectory) {
FILE_PATH_LITERAL("drive/New Directory/File in new dir.gdoc")))); FILE_PATH_LITERAL("drive/New Directory/File in new dir.gdoc"))));
} }
TEST_F(GDataFileSystemTest, ChangeFeed_AddFileToNewButDeletedDirectory) {
int latest_changelog = 0;
LoadRootFeedDocument("root_feed.json");
// This feed contains thw following updates:
// 1) A new PDF file is added to a new directory
// 2) but the new directory is marked "deleted" (i.e. moved to Trash)
// Hence, the PDF file should be just ignored.
LoadChangeFeed("delta_file_added_in_new_but_deleted_directory.json",
++latest_changelog);
}
TEST_F(GDataFileSystemTest, ChangeFeed_DirectoryMovedFromRootToDirectory) { TEST_F(GDataFileSystemTest, ChangeFeed_DirectoryMovedFromRootToDirectory) {
int latest_changelog = 0; int latest_changelog = 0;
LoadRootFeedDocument("root_feed.json"); LoadRootFeedDocument("root_feed.json");
......
chrome/test/data/chromeos/gdata/delta_file_added_in_new_but_deleted_directory.json 0 → 100644
View file @ ba5771c7
{
"encoding": "UTF-8",
"feed": {
"author": [ {
"email": {
"$t": "tester@test.com"
},
"name": {
"$t": "tester"
}
} ],
"category": [ {
"label": "change",
"scheme": "http://schemas.google.com/g/2005#kind",
"term": "http://schemas.google.com/docs/2007#change"
} ],
"docs$largestChangestamp": {
"value": "16730"
},
"docs$quotaBytesUsedInTrash": {
"$t": "402724784"
},
"entry": [ {
"app$edited": {
"$t": "2012-04-10T22:50:55.965Z",
"xmlns$app": "http://www.w3.org/2007/app"
},
"author": [ {
"email": {
"$t": "tester@test.com"
},
"name": {
"$t": "tester"
}
} ],
"category": [ {
"label": "application/pdf",
"scheme": "http://schemas.google.com/g/2005#kind",
"term": "http://schemas.google.com/docs/2007#file"
}, {
"label": "viewed",
"scheme": "http://schemas.google.com/g/2005/labels",
"term": "http://schemas.google.com/g/2005/labels#viewed"
}, {
"label": "modified-by-me",
"scheme": "http://schemas.google.com/g/2005/labels",
"term": "http://schemas.google.com/g/2005/labels#modified-by-me"
} ],
"content": {
"src": "https://content_url",
"type": "text/html"
},
"docs$changestamp": {
"value": "16683"
},
"docs$modifiedByMeDate": {
"$t": "2012-04-10T22:50:55.797Z"
},
"docs$writersCanInvite": {
"value": "true"
},
"gd$etag": "\"WVAJThBcDyt7ImBk\"",
"gd$feedLink": [ {
"href": "https://feedLink",
"rel": "http://schemas.google.com/docs/2007/revisions"
} ],
"gd$lastModifiedBy": {
"email": {
"$t": "tester@test.com"
},
"name": {
"$t": "tester"
}
},
"gd$lastViewed": {
"$t": "2012-04-10T22:50:55.797Z"
},
"gd$quotaBytesUsed": {
"$t": "0"
},
"gd$resourceId": {
// To test a crash bug crbug.com/127495, This should start with
// "pdf:", not "file:", so that the entry is processed after the
// directory "folder:new_folder_resource_id" is deleted (i.e.
// entries in delta feeds are processed in the order of resource
// IDs sorted alphabetically)
"$t": "pdf:file_added_in_deleted_dir_id"
},
"id": {
"$t": "https://document%3Afile_added_in_new_dir_id"
},
"link": [ {
"href": "https://new_dir_self_link/folder:new_folder_resource_id",
"rel": "http://schemas.google.com/docs/2007#parent",
"title": "New Directory",
"type": "application/atom+xml"
}, {
"href": "https://alternate/document%3Afile_added_in_new_dir_id/edit",
"rel": "alternate",
"type": "text/html"
}, {
"href": "https://added_in_root.png",
"rel": "http://schemas.google.com/docs/2007#icon",
"type": "image/png"
}, {
"href": "https://",
"rel": "http://schemas.google.com/g/2005#resumable-edit-media",
"type": "application/atom+xml"
}, {
"href": "https://edit_url/document%3Afile_added_in_new_dir_id",
"rel": "edit",
"type": "application/atom+xml"
}, {
"href": "https://edit-media/document%3Afile_added_in_new_dir_id",
"rel": "edit-media",
"type": "text/html"
}, {
"href": "https://changes/16683",
"rel": "self",
"type": "application/atom+xml"
} ],
"published": {
"$t": "2012-04-10T22:50:53.237Z"
},
"title": {
"$t": "new_pdf_file.pdf"
},
"updated": {
"$t": "2012-04-10T22:50:55.797Z"
}
}, {
"app$edited": {
"$t": "2011-12-14T00:41:08.287Z",
"xmlns$app": "http://www.w3.org/2007/app"
},
"author": [ {
"email": {
"$t": "entry_tester@testing.com"
},
"name": {
"$t": "entry_tester"
}
} ],
"category": [ {
"label": "viewed",
"scheme": "http://schemas.google.com/g/2005/labels",
"term": "http://schemas.google.com/g/2005/labels#viewed"
}, {
"label": "folder",
"scheme": "http://schemas.google.com/g/2005#kind",
"term": "http://schemas.google.com/docs/2007#folder"
} ],
"content": {
"src": "https://1_folder_content_url",
"type": "application/atom+xml;type=feed"
},
"docs$writersCanInvite": {
"value": "true"
},
// Mark that the directory is deleted (i.e. moved to Trash).
"gd$deleted": {
},
"gd$etag": "\"HhMOFgcNHSt7ImBr\"",
"gd$feedLink": [ {
"href": "https://1_folder_feed_linkurl",
"rel": "http://schemas.google.com/acl/2007#accessControlList"
} ],
"gd$lastModifiedBy": {
"email": {
"$t": "tester@testing.com"
},
"name": {
"$t": "tester"
}
},
"gd$lastViewed": {
"$t": "2011-11-02T04:37:38.469Z"
},
"gd$quotaBytesUsed": {
"$t": "0"
},
"gd$resourceId": {
"$t": "folder:new_folder_resource_id"
},
"id": {
"$t": "https://new_folder_id"
},
"link": [ {
"href": "https://1_folder_alternate_link",
"rel": "alternate",
"type": "text/html"
}, {
"href": "https://1_folder_resumable_create_media_link",
"rel": "http://schemas.google.com/g/2005#resumable-create-media",
"type": "application/atom+xml"
}, {
"href": "https://new_dir_self_link/folder:new_folder_resource_id",
"rel": "self",
"type": "application/atom+xml"
}, {
"href": "https://new_dir_self_link/folder:new_folder_resource_id",
"rel": "edit",
"type": "application/atom+xml"
} ],
"published": {
"$t": "2010-11-07T05:03:54.719Z"
},
"title": {
"$t": "New Directory"
},
"updated": {
"$t": "2011-04-01T18:34:08.234Z"
}
} ],
"gd$etag": "W/\"AkYGSHc8eyt7ImA9WhVXEU0.\"",
"gd$quotaBytesTotal": {
"$t": "215822106624"
},
"gd$quotaBytesUsed": {
"$t": "416375123"
},
"id": {
"$t": "https://docs.google.com/feeds/default/private/changes"
},
"link": [ {
"href": "https://docs.google.com/feeds/default/private/changes",
"rel": "http://schemas.google.com/g/2005#feed",
"type": "application/atom+xml"
}, {
"href": "https://docs.google.com/feeds/default/private/changes?alt=json&start-index=16718&max-results=1000",
"rel": "self",
"type": "application/atom+xml"
} ],
"openSearch$startIndex": {
"$t": "16718"
},
"openSearch$totalResults": {
"$t": "16730"
},
"title": {
"$t": "Changed Documents - tester@test.com"
},
"updated": {
"$t": "2012-04-11T01:35:29.973Z"
},
"xmlns": "http://www.w3.org/2005/Atom",
"xmlns$docs": "http://schemas.google.com/docs/2007",
"xmlns$gd": "http://schemas.google.com/g/2005",
"xmlns$openSearch": "http://a9.com/-/spec/opensearch/1.1/"
},
"version": "1.0"
}
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