Fix filesystem.retainEntry API to handle non-native directory.

Fix filesystem.retainEntry API to handle non-native directory.

The CL changes the function to obtain is_directory flag internally with File System API's one, so that the retainEntry can handle non-native entry.

BUG=417376
TEST=newly added for the case

Committed: https://crrev.com/0e2c264f05c6b7271ad04154c0222612e10541a6
Cr-Commit-Position: refs/heads/master@{#299653}

[hirono, 2014-10-14 05:55:55]

hirono@chromium.org changed reviewers:
+ mtomasz@chromium.org

[hirono, 2014-10-14 05:55:56]

PTAL the CL? Thank you!

[mtomasz, 2014-10-14 06:08:50]

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:923: void
FileSystemRetainEntryFunction::SetIsDirectoryOnIOThread(
Calling SetIsDirectoryOnIOThread should only set "is_directory_, but it also
retains the file entry. Can we add a callback argument, and call it instead of
RetainFileEntry?

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:932:
content::BrowserThread::PostTask(
IO -> UI?

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.h (right):

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.h:188: void
SetIsDirectoryOnIOThread(const std::string& entry_id,
nit: Please add a comment.

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
File
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js
(right):

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js:11:

nit: This \n looks strange here.

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js:14:
return new Promise(function(fulfill) {
nit: AFAIK we require two arguments here for the closure compiler.
See:
https://codereview.chromium.org/631143002/diff/1/ui/file_manager/image_loader...

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js:61:
}).then(function(obj) {
nit: obj -> values?

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js:68:
window.onerror();
Is window.onerror() necessary?

[hirono, 2014-10-14 09:22:10]

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.cc (right):

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:923: void
FileSystemRetainEntryFunction::SetIsDirectoryOnIOThread(
On 2014/10/14 06:08:49, mtomasz wrote:
> Calling SetIsDirectoryOnIOThread should only set "is_directory_, but it also
> retains the file entry. Can we add a callback argument, and call it instead of
> RetainFileEntry?

I removed the method and aggregated the procedures in RetailFileEntry.

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.cc:932:
content::BrowserThread::PostTask(
On 2014/10/14 06:08:50, mtomasz wrote:
> IO -> UI?

Thank you for catching it. Done.

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
File chrome/browser/extensions/api/file_system/file_system_api.h (right):

https://codereview.chromium.org/656733002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/api/file_system/file_system_api.h:188: void
SetIsDirectoryOnIOThread(const std::string& entry_id,
On 2014/10/14 06:08:50, mtomasz wrote:
> nit: Please add a comment.

I removed the method.

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
File
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js
(right):

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js:11:

On 2014/10/14 06:08:50, mtomasz wrote:
> nit: This \n looks strange here.

Yes, mixed it in unintentionally.

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js:14:
return new Promise(function(fulfill) {
On 2014/10/14 06:08:50, mtomasz wrote:
> nit: AFAIK we require two arguments here for the closure compiler.
> See:
>
https://codereview.chromium.org/631143002/diff/1/ui/file_manager/image_loader...

Done.

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js:61:
}).then(function(obj) {
On 2014/10/14 06:08:50, mtomasz wrote:
> nit: obj -> values?

Done.

https://codereview.chromium.org/656733002/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/file_browser/retain_entry/background.js:68:
window.onerror();
On 2014/10/14 06:08:50, mtomasz wrote:
> Is window.onerror() necessary?

Removed.

[mtomasz, 2014-10-14 09:29:39]

nice, lgtm!

[hirono, 2014-10-15 04:03:41]

The CQ bit was checked by hirono@chromium.org

[commit-bot: I haz the power, 2014-10-15 04:04:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/656733002/130001

[commit-bot: I haz the power, 2014-10-15 04:57:47]

Committed patchset #8 (id:130001)

[commit-bot: I haz the power, 2014-10-15 05:29:36]

Patchset 8 (id:??) landed as
https://crrev.com/0e2c264f05c6b7271ad04154c0222612e10541a6
Cr-Commit-Position: refs/heads/master@{#299653}