RESOLVED FIXED 179642
ServiceWorker Inspector: Frontend changes to support Network tab and sub resources
https://bugs.webkit.org/show_bug.cgi?id=179642
Summary ServiceWorker Inspector: Frontend changes to support Network tab and sub reso...
Joseph Pecoraro
Reported 2017-11-13 15:36:34 PST
ServiceWorker Inspector: Frontend changes to support Network tab and sub resources Network Tab: - Should show requests made by the ServiceWorker Resource Tab: (UI like a Dedicated Worker) - Should show sub-resources loaded by the ServiceWorker - Should categorize the sub-resources underneath the ServiceWorker component
Attachments
[PATCH] Proposed Fix (27.21 KB, patch)
2017-11-17 15:49 PST, Joseph Pecoraro
ews-watchlist: commit-queue-
[IMAGE] Resources Tab (163.53 KB, image/png)
2017-11-17 15:50 PST, Joseph Pecoraro
no flags
[IMAGE] Network Tab - Table (98.38 KB, image/png)
2017-11-17 15:51 PST, Joseph Pecoraro
no flags
[IMAGE] Network Tab - Detail (130.08 KB, image/png)
2017-11-17 15:51 PST, Joseph Pecoraro
no flags
Archive of layout-test-results from ews100 for mac-elcapitan (2.56 MB, application/zip)
2017-11-17 16:43 PST, EWS Watchlist
no flags
Archive of layout-test-results from ews114 for mac-elcapitan (3.52 MB, application/zip)
2017-11-17 17:24 PST, EWS Watchlist
no flags
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (3.79 MB, application/zip)
2017-11-17 17:43 PST, EWS Watchlist
no flags
[PATCH] Proposed Fix (30.71 KB, patch)
2017-11-27 16:49 PST, Joseph Pecoraro
bburg: review+
bburg: commit-queue-
Radar WebKit Bug Importer
Comment 1 2017-11-13 15:36:53 PST
Joseph Pecoraro
Comment 2 2017-11-17 15:49:37 PST
Created attachment 327258 [details] [PATCH] Proposed Fix There are a few things I want to address in follow-ups: 1. Image resources added via CacheStorage.prototype.add / fetch() don't show up properly - this is essentially pre-existing bad behavior for fetching any binary content 2. Metrics data is not included - this requires enabling ResourceTiming in ServiceWorkers, which I'll be looking into
Joseph Pecoraro
Comment 3 2017-11-17 15:50:40 PST
Created attachment 327259 [details] [IMAGE] Resources Tab
Joseph Pecoraro
Comment 4 2017-11-17 15:51:03 PST
Created attachment 327260 [details] [IMAGE] Network Tab - Table
Joseph Pecoraro
Comment 5 2017-11-17 15:51:23 PST
Created attachment 327261 [details] [IMAGE] Network Tab - Detail
Joseph Pecoraro
Comment 6 2017-11-17 15:52:00 PST
Note that the ResponseSource is an emdash right now, that is related to the other metrics that will be enabled with ResourceTiming.
EWS Watchlist
Comment 7 2017-11-17 16:43:39 PST Comment hidden (obsolete)
EWS Watchlist
Comment 8 2017-11-17 16:43:40 PST Comment hidden (obsolete)
EWS Watchlist
Comment 9 2017-11-17 17:24:49 PST Comment hidden (obsolete)
EWS Watchlist
Comment 10 2017-11-17 17:24:50 PST Comment hidden (obsolete)
EWS Watchlist
Comment 11 2017-11-17 17:43:16 PST Comment hidden (obsolete)
EWS Watchlist
Comment 12 2017-11-17 17:43:18 PST Comment hidden (obsolete)
Joseph Pecoraro
Comment 13 2017-11-27 16:49:57 PST
Created attachment 327713 [details] [PATCH] Proposed Fix I was missing `WI.pageTarget` in Test.js
Blaze Burg
Comment 14 2017-11-28 12:58:15 PST
Comment on attachment 327713 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=327713&action=review r=me > Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:522 > + if (type === "Document" && frame.mainResource.url === url && frame.loaderIdentifier === loaderIdentifier) I'd prefer we move the enum somewhere else, rather than rely on bare string literals. Could it go on NetworkAgent? This can be a followup if it gets messy. > Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:524 > + else if (type === "Document" && frame.provisionalMainResource && frame.provisionalMainResource.url === url && frame.provisionalLoaderIdentifier === loaderIdentifier) Ditto. > Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:710 > + if (resourcePayload.type === "Document" && resourcePayload.url === payload.frame.url) Ditto. > Source/WebInspectorUI/UserInterface/Models/Resource.js:98 > + static preferredType(type, mimeType) This name is somewhat vague. > Source/WebInspectorUI/UserInterface/Protocol/Connection.js:275 > +InspectorBackend.MainConnection = class InspectorBackendMainConnection extends InspectorBackend.Connection OK > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:876 > + let populateTargetResources = (target) => { Nit: I would use Cocoa style naming, populateResourcesForFrame() populateResourcesForTarget()
Joseph Pecoraro
Comment 15 2017-11-28 15:37:09 PST
Comment on attachment 327713 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=327713&action=review >> Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:522 >> + if (type === "Document" && frame.mainResource.url === url && frame.loaderIdentifier === loaderIdentifier) > > I'd prefer we move the enum somewhere else, rather than rely on bare string literals. Could it go on NetworkAgent? This can be a followup if it gets messy. Yeah, we will want to move this to Network eventually. At this point Network should be considered the base and Page / ServiceWorker are disjoint and both would have a Network component. >> Source/WebInspectorUI/UserInterface/Models/Resource.js:98 >> + static preferredType(type, mimeType) > > This name is somewhat vague. Hmm, would bestType() or mostSpecificType() be better? This is something I'm going to be circling back to as I try to make an XHR/Fetch of an Image resource show properly. >> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:876 >> + let populateTargetResources = (target) => { > > Nit: I would use Cocoa style naming, populateResourcesForFrame() populateResourcesForTarget() I like that suggestion.
Joseph Pecoraro
Comment 16 2017-11-28 15:45:54 PST
Note You need to log in before you can comment on or make changes to this bug.