[webkit-reviews] review granted: [Bug 178597] Inspector should display service worker served responses properly : [Attachment 325242] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Joseph Pecoraro <joepeck at webkit.org>'s
request for review:
Bug 178597: Inspector should display service worker served responses properly
https://bugs.webkit.org/show_bug.cgi?id=178597

Attachment 325242: [PATCH] Proposed Fix

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




--- Comment #7 from Brian Burg <bburg at apple.com> ---
Comment on attachment 325242
  --> https://bugs.webkit.org/attachment.cgi?id=325242
[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.