LayoutTests/http/tests/serviceworker/fetch-event-client-attribute.html - Issue 1102363002: Add Client Attribute to FetchEvent- Blink Side.

Side by Side Diff: LayoutTests/http/tests/serviceworker/fetch-event-client-attribute.html

Issue 1102363002: Add Client Attribute to FetchEvent- Blink Side. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Created 5 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « no previous file | LayoutTests/http/tests/serviceworker/resources/fetch-event-client-attribute-test-worker.js » ('j') | LayoutTests/http/tests/serviceworker/resources/fetch-event-client-attribute-test-worker.js » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <script src="../resources/testharness.js"></script>

	3 <script src="../resources/testharnessreport.js"></script>

	4 <script src="resources/test-helpers.js"></script>

	5 <body>

	6 <script>

	7 var worker = 'resources/fetch-event-client-attribute-test-worker.js';

	8

	9 var scope = 'resources/simple.html';

	10 promise_test(function(test) {

	11 return service_worker_unregister_and_register(test, worker, scope)

	12 .then(function(registration) {

	13 return wait_for_state(test, registration.installing, 'activated');

	14 })

	15 .then(function() { return with_iframe(scope); })

	16 .then(function(frame) {

	17 assert_equals(frame.contentDocument.body.textContent, "top-level", "f rameType should be top-level");
	jsbell 2015/05/06 16:21:26 Interesting... given the table in fetch.spec.whatw Interesting... given the table in fetch.spec.whatwg.org I would have expected the context frame type to be "nested" for an iframe. Is this a bug in our implementation of fetch? If so, fine to leave this as-is. Likely I'm just misunderstanding some subtlety in fetch and sw specs. jsbell 2015/05/06 16:21:26 Please wrap to 80 columns (e.g. break after the "t Please wrap to 80 columns (e.g. break after the "top-level", argument) falken 2015/05/07 04:06:03 Can you file a bug if it's an existing bug? Show quoted text On 2015/05/06 16:21:26, jsbell wrote: > Interesting... given the table in http://fetch.spec.whatwg.org I would have expected > the context frame type to be "nested" for an iframe. > > Is this a bug in our implementation of fetch? If so, fine to leave this as-is. > Likely I'm just misunderstanding some subtlety in fetch and sw specs. Can you file a bug if it's an existing bug? Paritosh Kumar 2015/05/07 16:46:54 I'm not updating on this now, first we will have t Show quoted text On 2015/05/07 04:06:03, falken wrote: > On 2015/05/06 16:21:26, jsbell wrote: > > Interesting... given the table in http://fetch.spec.whatwg.org I would have > expected > > the context frame type to be "nested" for an iframe. > > > > Is this a bug in our implementation of fetch? If so, fine to leave this as-is. > > Likely I'm just misunderstanding some subtlety in fetch and sw specs. > > Can you file a bug if it's an existing bug? I'm not updating on this now, first we will have to test it by sending it to 'event.client' using postMessage from worker that we should receive in frame window, from there we can check it. falken 2015/05/08 22:20:25 Can you elaborate on this? It looks like you're te Show quoted text On 2015/05/07 16:46:54, Paritosh Kumar wrote: > On 2015/05/07 04:06:03, falken wrote: > > On 2015/05/06 16:21:26, jsbell wrote: > > > Interesting... given the table in http://fetch.spec.whatwg.org I would have > > expected > > > the context frame type to be "nested" for an iframe. > > > > > > Is this a bug in our implementation of fetch? If so, fine to leave this > as-is. > > > Likely I'm just misunderstanding some subtlety in fetch and sw specs. > > > > Can you file a bug if it's an existing bug? > > I'm not updating on this now, first we will have to test it by sending it to > 'event.client' using > postMessage from worker that we should receive in frame window, from there we > can check it. Can you elaborate on this? It looks like you're testing it in this line because the SW populates body with event.client.frameType. I'm wondering why event.client.frameType is top-level rather than nested. Paritosh Kumar 2015/05/12 11:52:28 I investigated this issue and found that, this is Show quoted text On 2015/05/08 22:20:25, falken wrote: > On 2015/05/07 16:46:54, Paritosh Kumar wrote: > > On 2015/05/07 04:06:03, falken wrote: > > > On 2015/05/06 16:21:26, jsbell wrote: > > > > Interesting... given the table in http://fetch.spec.whatwg.org I would > have > > > expected > > > > the context frame type to be "nested" for an iframe. > > > > > > > > Is this a bug in our implementation of fetch? If so, fine to leave this > > as-is. > > > > Likely I'm just misunderstanding some subtlety in fetch and sw specs. > > > > > > Can you file a bug if it's an existing bug? > > > > I'm not updating on this now, first we will have to test it by sending it to > > 'event.client' using > > postMessage from worker that we should receive in frame window, from there we > > can check it. > > Can you elaborate on this? It looks like you're testing it in this line because > the SW populates body with event.client.frameType. I'm wondering why > event.client.frameType is top-level rather than nested. I investigated this issue and found that, this is not due to our implementation of fetch. We can see this problem only with event.client, actually in my implementation of client attribute of FetchEvent, I was not initializing the client attribute of ServiceWorkerFetchRequest in Chromium Side changes, that lead to default value of client, and hence other property of client also.
	18 frame.remove();

	19 add_completion_callback(function() {
	falken 2015/05/07 04:06:03 I don't think you need this since you call service I don't think you need this since you call service_worker_unregister_and_done() Paritosh Kumar 2015/05/07 16:46:54 Yes, Not Required. Thanks. Show quoted text On 2015/05/07 04:06:03, falken wrote: > I don't think you need this since you call service_worker_unregister_and_done() Yes, Not Required. Thanks.
	20 registration.unregister();

	21 });

	22 return service_worker_unregister_and_done(test, scope);

	23 });

	24 }, 'Service Worker responds to fetch event with client frameType');
	falken 2015/05/07 04:06:03 this indentation is 2 spaces too many this indentation is 2 spaces too many Paritosh Kumar 2015/05/07 16:46:54 Done. Show quoted text On 2015/05/07 04:06:03, falken wrote: > this indentation is 2 spaces too many Done.
	25

	26 </script>

	27 </body>

OLD	NEW