Introducing FetchEventInit Dictionary.

Introducing FetchEventInit Dictionary.

Spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#fetch-event-section

This cl is progress on https://codereview.chromium.org/1079073003/

BUG=411193, 479536
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194206

[Paritosh Kumar, 2015-04-21 09:05:34]

paritosh.in@samsung.com changed reviewers:
+ falken@chromium.org, tkent@chromium.org

[Paritosh Kumar, 2015-04-21 09:05:35]

@tkaent: As per your suggestion on https://codereview.chromium.org/1079073003/
created this cl
Please have a look.

https://codereview.chromium.org/1099073002/diff/1/Source/modules/serviceworke...
File Source/modules/serviceworkers/FetchEvent.cpp (right):

https://codereview.chromium.org/1099073002/diff/1/Source/modules/serviceworke...
Source/modules/serviceworkers/FetchEvent.cpp:21:
initializer.setCancelable(true);
Here, setting cancelable, true. Because For ExtendableEvent default value of
cancelable is false, but for FetchEvent it should be true.

[tkent, 2015-04-21 09:21:44]

https://codereview.chromium.org/1099073002/diff/1/Source/modules/serviceworke...
File Source/modules/serviceworkers/FetchEvent.cpp (right):

https://codereview.chromium.org/1099073002/diff/1/Source/modules/serviceworke...
Source/modules/serviceworkers/FetchEvent.cpp:21:
initializer.setCancelable(true);
On 2015/04/21 09:05:35, Paritosh Kumar wrote:
> Here, setting cancelable, true. Because For ExtendableEvent default value of
> cancelable is false, but for FetchEvent it should be true.

Please add a test to specify {cancelable:false} explicitly in FetchEventInit.

[Paritosh Kumar, 2015-04-21 09:31:01]

Thanks tkent

Added a test in interfaces-worker.js

https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
File LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js
(right):

https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:89:
assert_equals(
Added this test, we cannot set explicitly cancelable to false.

[tkent, 2015-04-21 09:45:47]

lgtm

[Paritosh Kumar, 2015-04-21 09:47:57]

@falken: Any suggestion please?

[falken, 2015-04-22 00:18:17]

lgtm, thanks

https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
File LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js
(right):

https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:76:
'ExtendableEvent', 'Type of ExtendableEvent should be ExtendableEvent:');
We don't add the ':' at the end of the description string, one is added
automatically by the test harness.

https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:99: new
FetchEvent('FetchEvent',{request : req, isReload : true}).request.url,
need a space after the ,

[Paritosh Kumar, 2015-04-22 06:35:39]

Thanks falken

Please suggest for this:

https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
File LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js
(right):

https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:76:
'ExtendableEvent', 'Type of ExtendableEvent should be ExtendableEvent:');
Thanks falken

Without ':' For Fail condition it will display like

FAIL Event constructors assert_equals: Type of ExtendableEvent should be
ExtendableEvent expected "ExtendebleEvent" but got "ExtendableEvent"

Here 'Type of ExtendableEvent should be ExtendableEvent expected
"ExtendebleEvent" but got "ExtendableEvent"' looks a single line.

I put ':' to separate 'Type of ExtendableEvent should be ExtendableEvent' and 
'expected "ExtendebleEvent" but got "ExtendableEvent"'.

Is it fine to leave as a single line, or we should separate them, may be using
','?

[falken, 2015-04-22 06:41:56]

On 2015/04/22 06:35:39, Paritosh Kumar wrote:
> Thanks falken
> 
> Please suggest for this:
> 
>
https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
> File LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js
> (right):
> 
>
https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
> LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:76:
> 'ExtendableEvent', 'Type of ExtendableEvent should be ExtendableEvent:');
> Thanks falken
> 
> Without ':' For Fail condition it will display like
> 
> FAIL Event constructors assert_equals: Type of ExtendableEvent should be
> ExtendableEvent expected "ExtendebleEvent" but got "ExtendableEvent"
> 
> Here 'Type of ExtendableEvent should be ExtendableEvent expected
> "ExtendebleEvent" but got "ExtendableEvent"' looks a single line.
> 
> I put ':' to separate 'Type of ExtendableEvent should be ExtendableEvent' and 
> 'expected "ExtendebleEvent" but got "ExtendableEvent"'.
> 
> Is it fine to leave as a single line, or we should separate them, may be using
> ','?

Ah, sorry I guess W3C harness does not add a ':'. It's fine to leave it as a
single line, that's what all of our tests do and how web-platform-tests are
written as well: https://github.com/w3c/web-platform-tests

[Paritosh Kumar, 2015-04-22 07:56:15]

On 2015/04/22 06:41:56, falken wrote:
> On 2015/04/22 06:35:39, Paritosh Kumar wrote:
> > Thanks falken
> > 
> > Please suggest for this:
> > 
> >
>
https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
> > File LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js
> > (right):
> > 
> >
>
https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
> > LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:76:
> > 'ExtendableEvent', 'Type of ExtendableEvent should be ExtendableEvent:');
> > Thanks falken
> > 
> > Without ':' For Fail condition it will display like
> > 
> > FAIL Event constructors assert_equals: Type of ExtendableEvent should be
> > ExtendableEvent expected "ExtendebleEvent" but got "ExtendableEvent"
> > 
> > Here 'Type of ExtendableEvent should be ExtendableEvent expected
> > "ExtendebleEvent" but got "ExtendableEvent"' looks a single line.
> > 
> > I put ':' to separate 'Type of ExtendableEvent should be ExtendableEvent'
and 
> > 'expected "ExtendebleEvent" but got "ExtendableEvent"'.
> > 
> > Is it fine to leave as a single line, or we should separate them, may be
using
> > ','?
> 
> Ah, sorry I guess W3C harness does not add a ':'. It's fine to leave it as a
> single line, that's what all of our tests do and how web-platform-tests are
> written as well: https://github.com/w3c/web-platform-tests

Thanks flaken for suggesting

Updated. Please have a look.

[falken, 2015-04-22 08:22:43]

On 2015/04/22 07:56:15, Paritosh Kumar wrote:
> On 2015/04/22 06:41:56, falken wrote:
> > On 2015/04/22 06:35:39, Paritosh Kumar wrote:
> > > Thanks falken
> > > 
> > > Please suggest for this:
> > > 
> > >
> >
>
https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
> > > File LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1099073002/diff/20001/LayoutTests/http/tests/...
> > > LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:76:
> > > 'ExtendableEvent', 'Type of ExtendableEvent should be ExtendableEvent:');
> > > Thanks falken
> > > 
> > > Without ':' For Fail condition it will display like
> > > 
> > > FAIL Event constructors assert_equals: Type of ExtendableEvent should be
> > > ExtendableEvent expected "ExtendebleEvent" but got "ExtendableEvent"
> > > 
> > > Here 'Type of ExtendableEvent should be ExtendableEvent expected
> > > "ExtendebleEvent" but got "ExtendableEvent"' looks a single line.
> > > 
> > > I put ':' to separate 'Type of ExtendableEvent should be ExtendableEvent'
> and 
> > > 'expected "ExtendebleEvent" but got "ExtendableEvent"'.
> > > 
> > > Is it fine to leave as a single line, or we should separate them, may be
> using
> > > ','?
> > 
> > Ah, sorry I guess W3C harness does not add a ':'. It's fine to leave it as a
> > single line, that's what all of our tests do and how web-platform-tests are
> > written as well: https://github.com/w3c/web-platform-tests
> 
> Thanks flaken for suggesting
> 
> Updated. Please have a look.

still lgtm

[Paritosh Kumar, 2015-04-22 08:31:39]

The CQ bit was checked by paritosh.in@samsung.com

[Paritosh Kumar, 2015-04-22 08:31:40]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1099073002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-04-22 08:31:53]

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

[commit-bot: I haz the power, 2015-04-22 10:16:56]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194206