Add API to load project from localfs into syncfs.

Add API to load project from localfs into syncfs.


This CL is on top of 13885007.

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

[miket_OOO, 2013-04-18 03:22:28]

Where are the tests? LGTM but please next CL must add tests.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:79:
std::vector<base::FilePath> FolderLs(const base::FilePath path) {
This isn't a great name for this function. Please choose something that uses
real words.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:82: |
file_util::FileEnumerator::FILES);
Is this right? You're asking for both directories and files, but putting all the
results in something called paths?

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:913:
path = path.Append(FILE_PATH_LITERAL(project_name));
Again, dangerous -- assume this name is maliciously formed.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:948:
this));
It looks like this will immediately queue up N operations on the IO thread,
which I *think* is OK and in fact the most efficient way to do it, but you might
want to ask Antony whether this makes sense (vs. waiting for each to complete
before firing off the next operation, which would be slower but might somehow
give another pending operation from another task a chance to do its work).

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:962:
true));
Wait a minute -- won't this send the response after the first file copy, not the
last one?

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:1013:
path = path.Append(project_name);
Scary

[kinuko, 2013-04-18 08:15:12]

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:79:
std::vector<base::FilePath> FolderLs(const base::FilePath path) {
ListFolder maybe?

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:956:
return;
If the # of file was 1 and we got error here we seem to fail to send response to
the app.  Should we call SetError() and SendResponse(false) (and ignore
succeeding callback responses) or just ignore all errors?

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:962:
true));
On 2013/04/18 03:22:28, miket wrote:
> Wait a minute -- won't this send the response after the first file copy, not
the
> last one?

Looks like it does, we should count up the number of paths we copied here and
send back response when it's done.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/api/developer_private/developer_private_api.h
(right):

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.h:427: 
nit: extra empty lines

https://codereview.chromium.org/14232018/diff/4001/chrome/common/extensions/a...
File chrome/common/extensions/api/developer_private.idl (right):

https://codereview.chromium.org/14232018/diff/4001/chrome/common/extensions/a...
chrome/common/extensions/api/developer_private.idl:169: optional VoidCallback
callback);
nit: indent

[Gaurav, 2013-04-18 23:08:46]

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:79:
std::vector<base::FilePath> FolderLs(const base::FilePath path) {
On 2013/04/18 08:15:12, kinuko wrote:
> ListFolder maybe?
> 

Done.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:79:
std::vector<base::FilePath> FolderLs(const base::FilePath path) {
On 2013/04/18 03:22:28, miket wrote:
> This isn't a great name for this function. Please choose something that uses
> real words.

Done.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:82: |
file_util::FileEnumerator::FILES);
On 2013/04/18 03:22:28, miket wrote:
> Is this right? You're asking for both directories and files, but putting all
the
> results in something called paths?

Thought paths represent both files and folders ?

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:913:
path = path.Append(FILE_PATH_LITERAL(project_name));
On 2013/04/18 03:22:28, miket wrote:
> Again, dangerous -- assume this name is maliciously formed.

Ensuring that the project_name contains only alpha-numeric, underscore, hyphen
and dot.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:948:
this));
On 2013/04/18 03:22:28, miket wrote:
> It looks like this will immediately queue up N operations on the IO thread,
> which I *think* is OK and in fact the most efficient way to do it, but you
might
> want to ask Antony whether this makes sense (vs. waiting for each to complete
> before firing off the next operation, which would be slower but might somehow
> give another pending operation from another task a chance to do its work).

Discussed with Antony. Looks to be an optimal approach.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:956:
return;
On 2013/04/18 08:15:12, kinuko wrote:
> If the # of file was 1 and we got error here we seem to fail to send response
to
> the app.  Should we call SetError() and SendResponse(false) (and ignore
> succeeding callback responses) or just ignore all errors?

Fixed. Send true response if all callbacks are successful, false otherwise.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:962:
true));
On 2013/04/18 08:15:12, kinuko wrote:
> On 2013/04/18 03:22:28, miket wrote:
> > Wait a minute -- won't this send the response after the first file copy, not
> the
> > last one?
> 
> Looks like it does, we should count up the number of paths we copied here and
> send back response when it's done.

Done.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:962:
true));
On 2013/04/18 03:22:28, miket wrote:
> Wait a minute -- won't this send the response after the first file copy, not
the
> last one?

Done.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:1013:
path = path.Append(project_name);
On 2013/04/18 03:22:28, miket wrote:
> Scary

As above.

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/api/developer_private/developer_private_api.h
(right):

https://codereview.chromium.org/14232018/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/api/developer_private/developer_private_api.h:427: 
On 2013/04/18 08:15:12, kinuko wrote:
> nit: extra empty lines

Done.

https://codereview.chromium.org/14232018/diff/4001/chrome/common/extensions/a...
File chrome/common/extensions/api/developer_private.idl (right):

https://codereview.chromium.org/14232018/diff/4001/chrome/common/extensions/a...
chrome/common/extensions/api/developer_private.idl:169: optional VoidCallback
callback);
On 2013/04/18 08:15:12, kinuko wrote:
> nit: indent

Done.

[commit-bot: I haz the power, 2013-04-19 17:04:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grv@chromium.org/14232018/13005

[commit-bot: I haz the power, 2013-04-19 17:43:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grv@chromium.org/14232018/13009

[commit-bot: I haz the power, 2013-04-19 17:45:33]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-04-19 18:06:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grv@chromium.org/14232018/26001

[commit-bot: I haz the power, 2013-04-19 21:18:24]

Change committed as 195287