Bug 195693 - Web Inspector: HAR Extension for Resource Priority
Summary: Web Inspector: HAR Extension for Resource Priority
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-13 12:40 PDT by Joseph Pecoraro
Modified: 2019-03-18 11:35 PDT (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (8.74 KB, patch)
2019-03-15 13:26 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (9.28 KB, patch)
2019-03-15 15:53 PDT, Joseph Pecoraro
hi: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2019-03-13 12:40:59 PDT
HAR Extension for Resource Priority

• We include _fetchType we can include _priority
Comment 1 Joseph Pecoraro 2019-03-15 13:26:20 PDT
Created attachment 364829 [details]
[PATCH] Proposed Fix
Comment 2 Joseph Pecoraro 2019-03-15 15:53:11 PDT
Created attachment 364862 [details]
[PATCH] Proposed Fix
Comment 3 Devin Rousso 2019-03-15 17:10:31 PDT
Comment on attachment 364862 [details]
[PATCH] Proposed Fix

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

r=me

> Source/WebInspectorUI/UserInterface/Controllers/HARBuilder.js:332
> +        case WI.Resource.NetworkPriority.Low:
> +            return "low";
> +        case WI.Resource.NetworkPriority.Medium:
> +            return "medium";
> +        case WI.Resource.NetworkPriority.High:
> +            return "high";

Could we use the `NetworkAgent.MetricsPriority` values instead, or are you worried about backward/forward compatibility?

> Source/WebInspectorUI/UserInterface/Controllers/HARBuilder.js:335
> +        console.assert(false);

`console.assert()` works instead of this :P

> Source/WebInspectorUI/UserInterface/Controllers/HARBuilder.js:380
> -            console.warn("Unknown HAR Protocol _fetchType", fetchType);
> +            console.warn("Unknown HAR _fetchType value", fetchType);

Oops :P

> Source/WebInspectorUI/UserInterface/Controllers/HARBuilder.js:392
> +        case "low":
> +            return WI.Resource.NetworkPriority.Low;
> +        case "medium":
> +            return WI.Resource.NetworkPriority.Medium;
> +        case "high":
> +            return WI.Resource.NetworkPriority.High;

Ditto (>327).
Comment 4 Joseph Pecoraro 2019-03-15 18:56:14 PDT
Comment on attachment 364862 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Controllers/HARBuilder.js:332
>> +            return "high";
> 
> Could we use the `NetworkAgent.MetricsPriority` values instead, or are you worried about backward/forward compatibility?

Yeah I don't want changing the protocol to break HAR import/export expectations.

>> Source/WebInspectorUI/UserInterface/Controllers/HARBuilder.js:392
>> +            return WI.Resource.NetworkPriority.High;
> 
> Ditto (>327).

These we are converting back from the HAR to WI.Resource so we'd want what we serialized into the HAR.
Comment 5 Joseph Pecoraro 2019-03-15 20:16:01 PDT
https://trac.webkit.org/r243031
Comment 6 Joseph Pecoraro 2019-03-18 11:34:51 PDT
bump to try and get radar importer to import
Comment 7 Radar WebKit Bug Importer 2019-03-18 11:35:09 PDT
<rdar://problem/48986580>