Bug 178597 - Inspector should display service worker served responses properly
Summary: Inspector should display service worker served responses properly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-20 11:51 PDT by youenn fablet
Modified: 2017-11-02 15:01 PDT (History)
9 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (28.82 KB, patch)
2017-10-25 14:56 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[IMAGE] Service Worker Loads in Network Table (108.43 KB, image/png)
2017-10-25 14:57 PDT, Joseph Pecoraro
no flags Details
[PATCH] Proposed Fix (29.90 KB, patch)
2017-10-25 17:08 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (24.26 KB, patch)
2017-10-27 21:54 PDT, Joseph Pecoraro
bburg: review+
bburg: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-10-20 11:51:34 PDT
Following on https://bugs.webkit.org/show_bug.cgi?id=178593, inspector should leverage ResourceResponse::Source::ServiceWorker.
Comment 1 Joseph Pecoraro 2017-10-25 14:56:43 PDT
Created attachment 324902 [details]
[PATCH] Proposed Fix
Comment 2 Joseph Pecoraro 2017-10-25 14:57:59 PDT
Created attachment 324904 [details]
[IMAGE] Service Worker Loads in Network Table
Comment 3 Build Bot 2017-10-25 14:58:04 PDT
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.
Comment 4 Radar WebKit Bug Importer 2017-10-25 16:21:23 PDT
<rdar://problem/35186111>
Comment 5 Joseph Pecoraro 2017-10-25 17:08:37 PDT
Created attachment 324927 [details]
[PATCH] Proposed Fix

Fixed ChangeLogs.
Comment 6 Joseph Pecoraro 2017-10-27 21:54:39 PDT
Created attachment 325242 [details]
[PATCH] Proposed Fix

Rebaselined and simplified a bit.
Comment 7 Brian Burg 2017-11-02 11:49:36 PDT
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.
Comment 8 Joseph Pecoraro 2017-11-02 14:57:04 PDT
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.
Comment 9 Joseph Pecoraro 2017-11-02 15:01:24 PDT
<https://trac.webkit.org/changeset/224357/webkit>