gdata: Make GDataFileSystem::ReadDirectoryByPath() much more efficient

gdata: Make GDataFileSystem::ReadDirectoryByPath() much more efficient

Previously, ReadDirectoryByPath() was returning DataDirectoryProto,
which contains all child files and directories, recursively.
This means, calling ReadDirectoryByPath() on the root directory is
super expensive as the returned GDataDirectoryProto contains the
entire tree as proto. This was super expensive.

With this change, ReadDirectoryByPath() will only return contents
of the target directory as a flat vector.

BUG=137871
TEST=out/Release/unit_tests --gtest_filter=GData*; also confirmed that the file manager worked as before, including the "save as file" to Drive

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=147806

[satorux1, 2012-07-19 15:11:00]

please put this on hold. I found a bug...

http://codereview.chromium.org/10815008/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc (right):

http://codereview.chromium.org/10815008/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc:695: if
(proto.file_info().is_directory() && hide_hosted_documents)
noooo, this is wrong. we should check if the file is a hosted document, but we
cannot from GDataEntryProto...

[satorux1, 2012-07-21 06:28:31]

Here you go. It's ready!

[achuithb, 2012-07-21 21:32:00]

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_file_system_interface.h (right):

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_interface.h:65: // If |error| is
not PLATFORM_FILE_OK, |entries| is set to NULL.
Could you add a comment about what is |entries|? It's the children of the
directory, right? Maybe it should be called child_entries?

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_interface.h:68:
scoped_ptr<std::vector<GDataEntryProto> > entries)>
Might be nicer to add a typedef for this vector?

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc (right):

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc:159: // Counts the
numbre of files (not directories) in |entries|.
number

[achuithb, 2012-07-21 21:32:31]

Since you're modifying gdata_download_observer, could you manually test uploads
to make sure they work with this CL?

[satorux1, 2012-07-21 21:54:08]

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_file_system_interface.h (right):

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_interface.h:65: // If |error| is
not PLATFORM_FILE_OK, |entries| is set to NULL.
On 2012/07/21 21:32:00, achuith.bhandarkar wrote:
> Could you add a comment about what is |entries|? It's the children of the
> directory, right? Maybe it should be called child_entries?

Done.

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_interface.h:68:
scoped_ptr<std::vector<GDataEntryProto> > entries)>
On 2012/07/21 21:32:00, achuith.bhandarkar wrote:
> Might be nicer to add a typedef for this vector?

Done.

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc (right):

http://codereview.chromium.org/10815008/diff/3002/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc:159: // Counts the
numbre of files (not directories) in |entries|.
On 2012/07/21 21:32:00, achuith.bhandarkar wrote:
> number

Done.

[satorux1, 2012-07-21 21:54:56]

On 2012/07/21 21:32:31, achuith.bhandarkar wrote:
> Since you're modifying gdata_download_observer, could you manually test
uploads
> to make sure they work with this CL?

Sure. I did. "save as file" to Drive works as before.

[achuithb, 2012-07-21 22:34:18]

lgtm

http://codereview.chromium.org/10815008/diff/4012/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc (right):

http://codereview.chromium.org/10815008/diff/4012/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc:680:
scoped_ptr<gdata::GDataEntryProtoVector> proto_entries) {
same

http://codereview.chromium.org/10815008/diff/4012/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_file_system_proxy.h (right):

http://codereview.chromium.org/10815008/diff/4012/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_proxy.h:125:
scoped_ptr<gdata::GDataEntryProtoVector> proto_entries);
nit: Don't think you need gdata::

[satorux1, 2012-07-21 23:23:36]

http://codereview.chromium.org/10815008/diff/4012/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_file_system_proxy.h (right):

http://codereview.chromium.org/10815008/diff/4012/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_file_system_proxy.h:125:
scoped_ptr<gdata::GDataEntryProtoVector> proto_entries);
On 2012/07/21 22:34:19, achuith.bhandarkar wrote:
> nit: Don't think you need gdata::

There are many places gdata:: prefix is used in this and the .cc file. I'll
revemo them in a separate patch.
http://code.google.com/p/chromium/issues/detail?id=138418