Replace EntryArraySync type by an EntrySync[]

Replace EntryArraySync type by an EntrySync[]

Replace EntryArraySync type by an EntrySync[] to match the specification:
http://www.w3.org/TR/file-system-api/#widl-DirectoryReaderSync-readEntries-EntrySync

As a consequence, regular JavaScript Array methods can now be used.
There is no longer an item() method exposed but developers usually
access items using [] operator.

BUG=268356
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155869

[kinuko, 2013-08-08 11:39:19]

Looks good, a few nits and one question.

https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
File Source/modules/filesystem/SyncCallbackHelper.h (right):

https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
Source/modules/filesystem/SyncCallbackHelper.h:54: class EntryArraySync;
nit: obsolete

https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
Source/modules/filesystem/SyncCallbackHelper.h:56: class FileEntrySync;
I guess actually we can remove all these forward decls?

https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
Source/modules/filesystem/SyncCallbackHelper.h:158: template <typename
SuccessCallback, typename ObserverType, typename CallbackArg, typename
ResultType>
Could this change become simpler if we define traits classes for ResultTypes and
keep the remaining logic as is?

The traits can define types like ReturnType, StorageType and static methods like
getValue, setValue.

(If you think it'd make the code rather complicated current code is fine too)

[do-not-use, 2013-08-08 14:21:03]

On 2013/08/08 11:39:19, kinuko wrote:
> Looks good, a few nits and one question.
> 
>
https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
> File Source/modules/filesystem/SyncCallbackHelper.h (right):
> 
>
https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
> Source/modules/filesystem/SyncCallbackHelper.h:54: class EntryArraySync;
> nit: obsolete
> 
>
https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
> Source/modules/filesystem/SyncCallbackHelper.h:56: class FileEntrySync;
> I guess actually we can remove all these forward decls?
> 
>
https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
> Source/modules/filesystem/SyncCallbackHelper.h:158: template <typename
> SuccessCallback, typename ObserverType, typename CallbackArg, typename
> ResultType>
> Could this change become simpler if we define traits classes for ResultTypes
and
> keep the remaining logic as is?
> 
> The traits can define types like ReturnType, StorageType and static methods
like
> getValue, setValue.
> 
> (If you think it'd make the code rather complicated current code is fine too)

I will give it a try and report back. Thanks for the suggestion.

[do-not-use, 2013-08-08 18:48:43]

On 2013/08/08 11:39:19, kinuko wrote:
> Looks good, a few nits and one question.
> 
>
https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
> File Source/modules/filesystem/SyncCallbackHelper.h (right):
> 
>
https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
> Source/modules/filesystem/SyncCallbackHelper.h:54: class EntryArraySync;
> nit: obsolete
> 
>
https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
> Source/modules/filesystem/SyncCallbackHelper.h:56: class FileEntrySync;
> I guess actually we can remove all these forward decls?
> 
>
https://codereview.chromium.org/22673003/diff/4001/Source/modules/filesystem/...
> Source/modules/filesystem/SyncCallbackHelper.h:158: template <typename
> SuccessCallback, typename ObserverType, typename CallbackArg, typename
> ResultType>
> Could this change become simpler if we define traits classes for ResultTypes
and
> keep the remaining logic as is?
> 
> The traits can define types like ReturnType, StorageType and static methods
like
> getValue, setValue.
> 
> (If you think it'd make the code rather complicated current code is fine too)

I used traits classes in Patch set 4 and the code looks simpler indeed. What do
you think?

[kinuko, 2013-08-09 02:15:54]

Looks nicer, thanks! lgtm

https://codereview.chromium.org/22673003/diff/13001/Source/modules/filesystem...
File Source/modules/filesystem/SyncCallbackHelper.h (right):

https://codereview.chromium.org/22673003/diff/13001/Source/modules/filesystem...
Source/modules/filesystem/SyncCallbackHelper.h:53: struct HelperResultType {
nit: these helper classes can be in an anonymous namespace.

[do-not-use, 2013-08-09 06:39:21]

https://codereview.chromium.org/22673003/diff/13001/Source/modules/filesystem...
File Source/modules/filesystem/SyncCallbackHelper.h (right):

https://codereview.chromium.org/22673003/diff/13001/Source/modules/filesystem...
Source/modules/filesystem/SyncCallbackHelper.h:53: struct HelperResultType {
On 2013/08/09 02:15:54, kinuko wrote:
> nit: these helper classes can be in an anonymous namespace.

This would apparently be against coding style:
"Source/modules/filesystem/SyncCallbackHelper.h:52:  Do not use unnamed
namespaces in header files.  See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces for
more information."

[kinuko, 2013-08-09 06:47:31]

On 2013/08/09 06:39:21, Christophe Dumez wrote:
>
https://codereview.chromium.org/22673003/diff/13001/Source/modules/filesystem...
> File Source/modules/filesystem/SyncCallbackHelper.h (right):
> 
>
https://codereview.chromium.org/22673003/diff/13001/Source/modules/filesystem...
> Source/modules/filesystem/SyncCallbackHelper.h:53: struct HelperResultType {
> On 2013/08/09 02:15:54, kinuko wrote:
> > nit: these helper classes can be in an anonymous namespace.
> 
> This would apparently be against coding style:
> "Source/modules/filesystem/SyncCallbackHelper.h:52:  Do not use unnamed
> namespaces in header files.  See
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces for
> more information."

Ah, you're right, we're in .h file. Using namespace name 'internal' or something
might be more common then?

[tkent, 2013-08-09 06:48:47]

lgtm

[kinuko, 2013-08-09 06:50:56]

On 2013/08/09 06:47:31, kinuko wrote:
> On 2013/08/09 06:39:21, Christophe Dumez wrote:
> >
>
https://codereview.chromium.org/22673003/diff/13001/Source/modules/filesystem...
> > File Source/modules/filesystem/SyncCallbackHelper.h (right):
> > 
> >
>
https://codereview.chromium.org/22673003/diff/13001/Source/modules/filesystem...
> > Source/modules/filesystem/SyncCallbackHelper.h:53: struct HelperResultType {
> > On 2013/08/09 02:15:54, kinuko wrote:
> > > nit: these helper classes can be in an anonymous namespace.
> > 
> > This would apparently be against coding style:
> > "Source/modules/filesystem/SyncCallbackHelper.h:52:  Do not use unnamed
> > namespaces in header files.  See
> > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces
for
> > more information."
> 
> Ah, you're right, we're in .h file. Using namespace name 'internal' or
something
> might be more common then?

Well, in Blink it doesn't look very common either. Ok never mind, lgtm.

[commit-bot: I haz the power, 2013-08-09 18:07:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/22673003/13001

[commit-bot: I haz the power, 2013-08-09 20:52:11]

Change committed as 155869