New API to copy syncfs folder to localfs.

New API to copy syncfs folder to localfs.

Changed the API to take a HTML5 DirectoryEntry.

loadDirectory(DirectoryEntry entry).

BUG=335299
TBR=thakis@chromiun.org

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

[Gaurav, 2014-01-16 02:07:07]

https://codereview.chromium.org/132313009/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/132313009/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:971:
bool syncfs = true;
Will update with syncfs folder check.

[kinuko, 2014-01-16 04:42:23]

https://chromiumcodereview.appspot.com/132313009/diff/50001/chrome/browser/ex...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://chromiumcodereview.appspot.com/132313009/diff/50001/chrome/browser/ex...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:966: if
(is_syncfs_url) {
A preferred way to check this is get FileSystemURL by CrackURL() and check its
type, like:

fileapi::FileSystemURL directory_url =
context_->CrackURL(GURL(directory_url_str));

if (directory_url.is_valid() && directory_url.type() ==
fileapi::kFileSystemTypeSyncable) {
  ...
}

(You'll need to move line 984-986 before this code snippet to set context_)

[asargent_no_longer_on_chrome, 2014-01-16 22:57:33]

lgtm assuming you fix nits and address kinuko's feedback

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:964:
size_t pos = 0;
nit: it might be good to move this down to line 984, so it's closer to where
it's initialized and used

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:1032:
SetResult(new base::StringValue("-1"));
nit: instead of passing a meaningless piece of data for success, we should just
have an empty function signature for the callback (eg switch from PathCallback
to VoidCallback in the .idl for this method)

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:1033:
SendResponse(true);
nit: indentation looks wrong here

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/developer_private_api.h
(right):

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.h:410:
base::FilePath project_path_;
nit: it's not obvious what the difference between these 2 is ; please add a
comment for each one (or a single comment describing the difference between the
two)

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.h:419:
base::FilePath path_;
Also, how does |path_| here relate to |project_base_url_| and |project_path_|
above? Please add a comment, and consider renaming it to be more clear

[commit-bot: I haz the power, 2014-01-16 23:34:19]

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

[commit-bot: I haz the power, 2014-01-17 00:03:20]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Gaurav, 2014-01-17 00:21:14]

On 2014/01/17 00:03:20, I haz the power (commit-bot) wrote:
> Retried try job too often on chromium_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

TBR thakis for chrome/renderer/resources/renderer_resources.grd

[Gaurav, 2014-01-17 00:21:27]

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:964:
size_t pos = 0;
On 2014/01/16 22:57:33, Antony Sargent wrote:
> nit: it might be good to move this down to line 984, so it's closer to where
> it's initialized and used

Done.

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:1032:
SetResult(new base::StringValue("-1"));
On 2014/01/16 22:57:33, Antony Sargent wrote:
> nit: instead of passing a meaningless piece of data for success, we should
just
> have an empty function signature for the callback (eg switch from PathCallback
> to VoidCallback in the .idl for this method)

This API should actually return the extension_id of the loaded extension.
However, the current LoadUnpackedInstaller does not fire an event on completion.
Once it is in place i will return the extension_id. 

I do not want the api to change, thus returning a dummy value here.

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:1033:
SendResponse(true);
On 2014/01/16 22:57:33, Antony Sargent wrote:
> nit: indentation looks wrong here

Done.

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/developer_private_api.h
(right):

https://codereview.chromium.org/132313009/diff/120008/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.h:410:
base::FilePath project_path_;
On 2014/01/16 22:57:33, Antony Sargent wrote:
> nit: it's not obvious what the difference between these 2 is ; please add a
> comment for each one (or a single comment describing the difference between
the
> two)

Done.

[commit-bot: I haz the power, 2014-01-17 00:21:46]

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

[commit-bot: I haz the power, 2014-01-17 02:41:20]

Retried try job too often on win_x64_rel for step(s) base_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[commit-bot: I haz the power, 2014-01-17 03:27:45]

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

[commit-bot: I haz the power, 2014-01-17 04:02:42]

Retried try job too often on win_rel for step(s) app_list_unittests,
ash_unittests, aura_unittests, cacheinvalidation_unittests, cc_unittests,
check_deps, check_deps2git, chrome_elf_unittests, chromedriver_unittests,
components_unittests, compositor_unittests, content_browsertests,
content_unittests, crypto_unittests, events_unittests, google_apis_unittests,
gpu_unittests, installer_util_unittests, ipc_tests, jingle_unittests,
media_unittests, mini_installer_test, nacl_integration, ppapi_unittests,
printing_unittests, remoting_unittests, sql_unittests, sync_integration_tests,
sync_unit_tests, telemetry_unittests, views_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-17 04:07:39]

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

[commit-bot: I haz the power, 2014-01-17 04:46:14]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-17 05:14:18]

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

[commit-bot: I haz the power, 2014-01-17 05:49:05]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-17 07:02:17]

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

[commit-bot: I haz the power, 2014-01-17 10:02:18]

Change committed as 245474