Bug 178597 – Inspector should display service worker served responses properly

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[youenn fablet]

Following on https://bugs.webkit.org/show_bug.cgi?id=178593, inspector should leverage ResourceResponse::Source::ServiceWorker.

[Joseph Pecoraro]

Created attachment 324902 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 324904 [details]
[IMAGE] Service Worker Loads in Network Table

[Build Bot]

Attachment 324902 [details] did not pass style-queue:


ERROR: LayoutTests/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: LayoutTests/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
ERROR: LayoutTests/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
ERROR: LayoutTests/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Radar WebKit Bug Importer]

<rdar://problem/35186111>

[Joseph Pecoraro]

Created attachment 324927 [details]
[PATCH] Proposed Fix

Fixed ChangeLogs.

[Joseph Pecoraro]

Created attachment 325242 [details]
[PATCH] Proposed Fix

Rebaselined and simplified a bit.

[Blaze Burg]

Comment on attachment 325242 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=325242&action=review

r=me but I would like HAR export test cases to be updated.

> LayoutTests/http/tests/inspector/network/resource-response-service-worker.html:25
> +            test(resolve, reject) {

I think this could be an async function. Take care that you are awaiting the right thing when using Promise.all, however.

> LayoutTests/http/tests/inspector/network/resource-response-service-worker.html:58
> +        description: "ServiceWorker may response with an error.",

Nit: respond

> Source/WebInspectorUI/ChangeLog:15
> +        Only output timing data when we have real resource timing data.

I think you should update the HAR test cases to trigger SW loads? Its OK if this is a separate patch.

[Joseph Pecoraro]

Comment on attachment 325242 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=325242&action=review

>> LayoutTests/http/tests/inspector/network/resource-response-service-worker.html:25
>> +            test(resolve, reject) {
> 
> I think this could be an async function. Take care that you are awaiting the right thing when using Promise.all, however.

This test closely follows the pattern of other inspector/network/resource-response-*.html tests so I'm going to leave it as is.

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/224357/webkit>