You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Jesse <pu...@gmail.com> on 2013/09/06 02:45:07 UTC

FileTransfer tests

I am working through some of the failing tests for wp8 filetransfer and
have some issues with the following tests.  Can anyone add any input on
where some of the following requirements come from?

1. filetransfer.spec.7 should be able to download a file using file://
(when hosted from file://)


{code}
if (!/^file/.exec(remoteFile)) {
    expect(remoteFile).toMatch(/^file:/);
    return;
}
{/code}


This will fail on wp7, wp8, and windows8, all of which are NOT loaded from
the file:// protocol, but have their own specific file-like protocols.
( windows phone uses: x-wmapp0:, and windows 8 uses ms-appx:// )
This should not be a failure, from what I can tell.  I plan to expand the
test to include the current protocol, + specifically load something using
the file:// protocol

2. filetransfer.spec.10 should be stopped by abort() right away

{code}
expect(new Date() - startTime).toBeLessThan(300);
{/code}

Where does the 300 ms rule come from? Shouldn't this just test that aborted
downloads do not complete? and that the target file is not partially
written when aborted?

3. filetransfer.spec.27 should be able to set custom headers
{code}
options.headers = {
                    "CustomHeader1": "CustomValue1",
                    "CustomHeader2": ["CustomValue2", "CustomValue3"],
                };
{/code}

Is it required that we set headers to deeply nested object literals?
Is it required that we support non-well-formed object literals? hence the
extra ','?

4. Android test / dependency on device
{code}
var getMalformedUrl = function() {
        if (device.platform.match(/Android/i)) {
            // bad protocol causes a MalformedUrlException on Android
            return "httpssss://example.com";
        }...
{/code}

This test is dependent on cordova-plugin-device, which may not be present.
I will switch this to use a userAgent check for Android.




@purplecabbage
risingj.com

Re: FileTransfer tests

Posted by Andrew Grieve <ag...@chromium.org>.
On Fri, Sep 6, 2013 at 1:40 PM, Jesse MacFadyen <pu...@gmail.com>wrote:

> Inline.
>
>
>
> >> On Sep 5, 2013, at 7:59 PM, Ian Clelland <ic...@chromium.org>
> wrote:
> >>
> >> On Thu, Sep 5, 2013 at 8:45 PM, Jesse <pu...@gmail.com> wrote:
> >>
> >> I am working through some of the failing tests for wp8 filetransfer and
> >> have some issues with the following tests.  Can anyone add any input on
> >> where some of the following requirements come from?
> >>
> >> 1. filetransfer.spec.7 should be able to download a file using file://
> >> (when hosted from file://)
> >>
> >>
> >> {code}
> >> if (!/^file/.exec(remoteFile)) {
> >>   expect(remoteFile).toMatch(/^file:/);
> >>   return;
> >> }
> >> {/code}
> >>
> >>
> >> This will fail on wp7, wp8, and windows8, all of which are NOT loaded
> from
> >> the file:// protocol, but have their own specific file-like protocols.
> >> ( windows phone uses: x-wmapp0:, and windows 8 uses ms-appx:// )
> >> This should not be a failure, from what I can tell.  I plan to expand
> the
> >> test to include the current protocol, + specifically load something
> using
> >> the file:// protocol
> >
> > I suspect that if WP and Win apps cannot be hosted on file:/// URLs, then
> > this test shoudn't be run on them. We should probably change it, then, to
> > test that file downloads work from same-scheme / same-host origins.
> >
> > (I figure that this test exists for platforms like Android, where you
> have
> > to explicitly grant the webview access to file:/// URLs from native code,
> > or else it will not allow you to access them at all, even if your app is
> > also hosted at that origin.)
> >
> > I'm not familiar with the WP architecture -- are file:/// resources
> allowed
> > at all? Or are all device-local-resources accessed from the custom
> schemes?
> >
>
> File resources are not used. Assets are loaded from x-wmapp0:
> I of course resolve file:// for the file API and filetransfer as well
> as local xhr.
>
> <img src='file://kitty.jpg'> will not work
>
>
>
> >
> >>
> >> 2. filetransfer.spec.10 should be stopped by abort() right away
> >>
> >> {code}
> >> expect(new Date() - startTime).toBeLessThan(300);
> >> {/code}
> >>
> >> Where does the 300 ms rule come from? Shouldn't this just test that
> aborted
> >> downloads do not complete?
> >
> >
> > The test itself is verifying that file transfers are asynchronous. If it
> > fails, it means that the ft.abort() call did not get through in 300ms,
> > which is a good indication that the system was blocking on the transfer.
> >
> > File transfers should not leave the webview unresponsive, and should
> handle
> > abort() calls immediately.
>
> I currently wait until I have interrupted the downloading thread
> before dispatching the abort error. This in no way blocks the UI
> thread. Based on network conditions and buffer size, this could take
> longer than 300 ms.
> I could change native code to dispatch aborterror immediately to meet
> the 300 ms deadline, but the number is still arbitrary and does not
> mean the webview is not blocked. We might as well just have the js
> code for abort() dispatch the error, then it will pass everywhere.
>
> Agree it's a bit silly. I'm fine if you want to delete this test.

>
> >> and that the target file is not partially
> >> written when aborted?
> >
> > There's another test for that: filetransfer.spec.9 checks that partial
> (or
> > complete) files aren't left around after an abort()ed transfer.
> >
> >
> >> 3. filetransfer.spec.27 should be able to set custom headers
> >> {code}
> >> options.headers = {
> >>                   "CustomHeader1": "CustomValue1",
> >>                   "CustomHeader2": ["CustomValue2", "CustomValue3"],
> >>               };
> >> {/code}
> >>
> >> Is it required that we set headers to deeply nested object literals?
> >
> > I believe this syntax is used to set two values for the same header. This
> > would appear in the request header as
> >
> > CustomHeader1: CustomValue1
> > CustomHeader2: CustomValue2
> > CustomHeader2: CustomValue3
> >
> > It should also be allowed to output them like this:
> >
> > CustomHeader1: CustomValue1
> > CustomHeader2: CustomValue2, CustomValue3
> >
>
> I'll need to compare the outcome on other platforms then.
>
>
>
> > Is it required that we support non-well-formed object literals? hence the
> >> extra ','?
> >
> > No, that's an error.
> >
> >
> >>
> >> 4. Android test / dependency on device
> >> {code}
> >> var getMalformedUrl = function() {
> >>       if (device.platform.match(/Android/i)) {
> >>           // bad protocol causes a MalformedUrlException on Android
> >>           return "httpssss://example.com";
> >>       }...
> >> {/code}
> >>
> >> This test is dependent on cordova-plugin-device, which may not be
> present.
> >> I will switch this to use a userAgent check for Android.
> >
> > If you can wait until tomorrow, I am planning on fixing this :)
> Apparently
> > there's a long-standing feature request somewhere to provide that info as
> > 'cordova.platform', and now that Device is an optional plugin, it makes a
> > lot more sense. I was lamenting earlier today that I had to do the same
> > userAgent check in one of our plugins, so I'm just going to take care of
> it.
> >
> > Ian
>
> Done yet?
>

Re: FileTransfer tests

Posted by Ian Clelland <ic...@google.com>.
On Fri, Sep 6, 2013 at 1:40 PM, Jesse MacFadyen <pu...@gmail.com>wrote:

>
> > If you can wait until tomorrow, I am planning on fixing this :)
> Apparently
> > there's a long-standing feature request somewhere to provide that info as
> > 'cordova.platform', and now that Device is an optional plugin, it makes a
> > lot more sense. I was lamenting earlier today that I had to do the same
> > userAgent check in one of our plugins, so I'm just going to take care of
> it.
> >
> > Ian
>
> Done yet?
>

No. :(
/me hangs head in shame

Still hoping to knock that off today; there is a workaround right now:
cordova.require('cordova/platform').id will return the platform that your
app is running on.

I just need to map that to cordova.platform, or something similar, I think.

Ian

Re: FileTransfer tests

Posted by Jesse MacFadyen <pu...@gmail.com>.
Inline.



>> On Sep 5, 2013, at 7:59 PM, Ian Clelland <ic...@chromium.org> wrote:
>>
>> On Thu, Sep 5, 2013 at 8:45 PM, Jesse <pu...@gmail.com> wrote:
>>
>> I am working through some of the failing tests for wp8 filetransfer and
>> have some issues with the following tests.  Can anyone add any input on
>> where some of the following requirements come from?
>>
>> 1. filetransfer.spec.7 should be able to download a file using file://
>> (when hosted from file://)
>>
>>
>> {code}
>> if (!/^file/.exec(remoteFile)) {
>>   expect(remoteFile).toMatch(/^file:/);
>>   return;
>> }
>> {/code}
>>
>>
>> This will fail on wp7, wp8, and windows8, all of which are NOT loaded from
>> the file:// protocol, but have their own specific file-like protocols.
>> ( windows phone uses: x-wmapp0:, and windows 8 uses ms-appx:// )
>> This should not be a failure, from what I can tell.  I plan to expand the
>> test to include the current protocol, + specifically load something using
>> the file:// protocol
>
> I suspect that if WP and Win apps cannot be hosted on file:/// URLs, then
> this test shoudn't be run on them. We should probably change it, then, to
> test that file downloads work from same-scheme / same-host origins.
>
> (I figure that this test exists for platforms like Android, where you have
> to explicitly grant the webview access to file:/// URLs from native code,
> or else it will not allow you to access them at all, even if your app is
> also hosted at that origin.)
>
> I'm not familiar with the WP architecture -- are file:/// resources allowed
> at all? Or are all device-local-resources accessed from the custom schemes?
>

File resources are not used. Assets are loaded from x-wmapp0:
I of course resolve file:// for the file API and filetransfer as well
as local xhr.

<img src='file://kitty.jpg'> will not work



>
>>
>> 2. filetransfer.spec.10 should be stopped by abort() right away
>>
>> {code}
>> expect(new Date() - startTime).toBeLessThan(300);
>> {/code}
>>
>> Where does the 300 ms rule come from? Shouldn't this just test that aborted
>> downloads do not complete?
>
>
> The test itself is verifying that file transfers are asynchronous. If it
> fails, it means that the ft.abort() call did not get through in 300ms,
> which is a good indication that the system was blocking on the transfer.
>
> File transfers should not leave the webview unresponsive, and should handle
> abort() calls immediately.

I currently wait until I have interrupted the downloading thread
before dispatching the abort error. This in no way blocks the UI
thread. Based on network conditions and buffer size, this could take
longer than 300 ms.
I could change native code to dispatch aborterror immediately to meet
the 300 ms deadline, but the number is still arbitrary and does not
mean the webview is not blocked. We might as well just have the js
code for abort() dispatch the error, then it will pass everywhere.


>> and that the target file is not partially
>> written when aborted?
>
> There's another test for that: filetransfer.spec.9 checks that partial (or
> complete) files aren't left around after an abort()ed transfer.
>
>
>> 3. filetransfer.spec.27 should be able to set custom headers
>> {code}
>> options.headers = {
>>                   "CustomHeader1": "CustomValue1",
>>                   "CustomHeader2": ["CustomValue2", "CustomValue3"],
>>               };
>> {/code}
>>
>> Is it required that we set headers to deeply nested object literals?
>
> I believe this syntax is used to set two values for the same header. This
> would appear in the request header as
>
> CustomHeader1: CustomValue1
> CustomHeader2: CustomValue2
> CustomHeader2: CustomValue3
>
> It should also be allowed to output them like this:
>
> CustomHeader1: CustomValue1
> CustomHeader2: CustomValue2, CustomValue3
>

I'll need to compare the outcome on other platforms then.



> Is it required that we support non-well-formed object literals? hence the
>> extra ','?
>
> No, that's an error.
>
>
>>
>> 4. Android test / dependency on device
>> {code}
>> var getMalformedUrl = function() {
>>       if (device.platform.match(/Android/i)) {
>>           // bad protocol causes a MalformedUrlException on Android
>>           return "httpssss://example.com";
>>       }...
>> {/code}
>>
>> This test is dependent on cordova-plugin-device, which may not be present.
>> I will switch this to use a userAgent check for Android.
>
> If you can wait until tomorrow, I am planning on fixing this :) Apparently
> there's a long-standing feature request somewhere to provide that info as
> 'cordova.platform', and now that Device is an optional plugin, it makes a
> lot more sense. I was lamenting earlier today that I had to do the same
> userAgent check in one of our plugins, so I'm just going to take care of it.
>
> Ian

Done yet?

Re: FileTransfer tests

Posted by Andrew Grieve <ag...@chromium.org>.
On Thu, Sep 5, 2013 at 10:58 PM, Ian Clelland <ic...@chromium.org>wrote:

> On Thu, Sep 5, 2013 at 8:45 PM, Jesse <pu...@gmail.com> wrote:
>
> > I am working through some of the failing tests for wp8 filetransfer and
> > have some issues with the following tests.  Can anyone add any input on
> > where some of the following requirements come from?
> >
> > 1. filetransfer.spec.7 should be able to download a file using file://
> > (when hosted from file://)
> >
> >
> > {code}
> > if (!/^file/.exec(remoteFile)) {
> >     expect(remoteFile).toMatch(/^file:/);
> >     return;
> > }
> > {/code}
> >
> >
> > This will fail on wp7, wp8, and windows8, all of which are NOT loaded
> from
> > the file:// protocol, but have their own specific file-like protocols.
> > ( windows phone uses: x-wmapp0:, and windows 8 uses ms-appx:// )
> > This should not be a failure, from what I can tell.  I plan to expand the
> > test to include the current protocol, + specifically load something using
> > the file:// protocol
> >
>
> I suspect that if WP and Win apps cannot be hosted on file:/// URLs, then
> this test shoudn't be run on them. We should probably change it, then, to
> test that file downloads work from same-scheme / same-host origins.
>
> (I figure that this test exists for platforms like Android, where you have
> to explicitly grant the webview access to file:/// URLs from native code,
> or else it will not allow you to access them at all, even if your app is
> also hosted at that origin.)
>
> I'm not familiar with the WP architecture -- are file:/// resources allowed
> at all? Or are all device-local-resources accessed from the custom schemes?
>

I think you'd be fine to make this test whatever scheme you're hosted on.
This behaviour is functionally the same as using
resolveLocalFIleSystemURI() + Entry.copy(), but has the advantage that it
provides progress callbacks.


>
>
> >
> > 2. filetransfer.spec.10 should be stopped by abort() right away
> >
> > {code}
> > expect(new Date() - startTime).toBeLessThan(300);
> > {/code}
> >
> > Where does the 300 ms rule come from? Shouldn't this just test that
> aborted
> > downloads do not complete?
>
>
> The test itself is verifying that file transfers are asynchronous. If it
> fails, it means that the ft.abort() call did not get through in 300ms,
> which is a good indication that the system was blocking on the transfer.
>
> File transfers should not leave the webview unresponsive, and should handle
> abort() calls immediately.
>

Right on. I added this to ensure the threading was working right on Android
/ iOS. On Android I actually fire the abort() callback as soon as the abort
is queued to achieve this performance.


>
>
> > and that the target file is not partially
> > written when aborted?
> >
>
> There's another test for that: filetransfer.spec.9 checks that partial (or
> complete) files aren't left around after an abort()ed transfer.
>
>
> > 3. filetransfer.spec.27 should be able to set custom headers
> > {code}
> > options.headers = {
> >                     "CustomHeader1": "CustomValue1",
> >                     "CustomHeader2": ["CustomValue2", "CustomValue3"],
> >                 };
> > {/code}
> >
> > Is it required that we set headers to deeply nested object literals?
> >
>
> I believe this syntax is used to set two values for the same header. This
> would appear in the request header as
>
> CustomHeader1: CustomValue1
> CustomHeader2: CustomValue2
> CustomHeader2: CustomValue3
>
> It should also be allowed to output them like this:
>
> CustomHeader1: CustomValue1
> CustomHeader2: CustomValue2, CustomValue3
>
> Is it required that we support non-well-formed object literals? hence the
> > extra ','?
> >
>
> No, that's an error.
>
>
> >
> > 4. Android test / dependency on device
> > {code}
> > var getMalformedUrl = function() {
> >         if (device.platform.match(/Android/i)) {
> >             // bad protocol causes a MalformedUrlException on Android
> >             return "httpssss://example.com";
> >         }...
> > {/code}
> >
> > This test is dependent on cordova-plugin-device, which may not be
> present.
> > I will switch this to use a userAgent check for Android.
> >
>
> If you can wait until tomorrow, I am planning on fixing this :) Apparently
> there's a long-standing feature request somewhere to provide that info as
> 'cordova.platform', and now that Device is an optional plugin, it makes a
> lot more sense. I was lamenting earlier today that I had to do the same
> userAgent check in one of our plugins, so I'm just going to take care of
> it.
>
> Ian
>

Re: FileTransfer tests

Posted by Ian Clelland <ic...@chromium.org>.
On Thu, Sep 5, 2013 at 8:45 PM, Jesse <pu...@gmail.com> wrote:

> I am working through some of the failing tests for wp8 filetransfer and
> have some issues with the following tests.  Can anyone add any input on
> where some of the following requirements come from?
>
> 1. filetransfer.spec.7 should be able to download a file using file://
> (when hosted from file://)
>
>
> {code}
> if (!/^file/.exec(remoteFile)) {
>     expect(remoteFile).toMatch(/^file:/);
>     return;
> }
> {/code}
>
>
> This will fail on wp7, wp8, and windows8, all of which are NOT loaded from
> the file:// protocol, but have their own specific file-like protocols.
> ( windows phone uses: x-wmapp0:, and windows 8 uses ms-appx:// )
> This should not be a failure, from what I can tell.  I plan to expand the
> test to include the current protocol, + specifically load something using
> the file:// protocol
>

I suspect that if WP and Win apps cannot be hosted on file:/// URLs, then
this test shoudn't be run on them. We should probably change it, then, to
test that file downloads work from same-scheme / same-host origins.

(I figure that this test exists for platforms like Android, where you have
to explicitly grant the webview access to file:/// URLs from native code,
or else it will not allow you to access them at all, even if your app is
also hosted at that origin.)

I'm not familiar with the WP architecture -- are file:/// resources allowed
at all? Or are all device-local-resources accessed from the custom schemes?


>
> 2. filetransfer.spec.10 should be stopped by abort() right away
>
> {code}
> expect(new Date() - startTime).toBeLessThan(300);
> {/code}
>
> Where does the 300 ms rule come from? Shouldn't this just test that aborted
> downloads do not complete?


The test itself is verifying that file transfers are asynchronous. If it
fails, it means that the ft.abort() call did not get through in 300ms,
which is a good indication that the system was blocking on the transfer.

File transfers should not leave the webview unresponsive, and should handle
abort() calls immediately.


> and that the target file is not partially
> written when aborted?
>

There's another test for that: filetransfer.spec.9 checks that partial (or
complete) files aren't left around after an abort()ed transfer.


> 3. filetransfer.spec.27 should be able to set custom headers
> {code}
> options.headers = {
>                     "CustomHeader1": "CustomValue1",
>                     "CustomHeader2": ["CustomValue2", "CustomValue3"],
>                 };
> {/code}
>
> Is it required that we set headers to deeply nested object literals?
>

I believe this syntax is used to set two values for the same header. This
would appear in the request header as

CustomHeader1: CustomValue1
CustomHeader2: CustomValue2
CustomHeader2: CustomValue3

It should also be allowed to output them like this:

CustomHeader1: CustomValue1
CustomHeader2: CustomValue2, CustomValue3

Is it required that we support non-well-formed object literals? hence the
> extra ','?
>

No, that's an error.


>
> 4. Android test / dependency on device
> {code}
> var getMalformedUrl = function() {
>         if (device.platform.match(/Android/i)) {
>             // bad protocol causes a MalformedUrlException on Android
>             return "httpssss://example.com";
>         }...
> {/code}
>
> This test is dependent on cordova-plugin-device, which may not be present.
> I will switch this to use a userAgent check for Android.
>

If you can wait until tomorrow, I am planning on fixing this :) Apparently
there's a long-standing feature request somewhere to provide that info as
'cordova.platform', and now that Device is an optional plugin, it makes a
lot more sense. I was lamenting earlier today that I had to do the same
userAgent check in one of our plugins, so I'm just going to take care of it.

Ian