Bug 1865394 - added urgency and incremental to request header in netmonitor. r=necko-reviewers,extension-reviewers,devtools-reviewers,valentin,robwu,bomsy,webdriver-reviewers,whimboo

Differential Revision: https://phabricator.services.mozilla.com/D201265

devtools/client/netmonitor/src/har/test/browser_net_har_copy_all_as_har.js

@@ -7,7 +7,7 @@
  * Basic tests for exporting Network panel content into HAR format.
  */
 
-const EXPECTED_REQUEST_HEADER_COUNT = 9;
+const EXPECTED_REQUEST_HEADER_COUNT = 10;
 const EXPECTED_RESPONSE_HEADER_COUNT = 6;
 
 add_task(async function () {

devtools/client/netmonitor/test/browser_net_copy_headers.js

@@ -47,6 +47,7 @@ add_task(async function () {
     "Accept-Encoding: gzip, deflate",
     "Connection: keep-alive",
     "Upgrade-Insecure-Requests: 1",
+    "Priority: u=1",
     "Pragma: no-cache",
     "Cache-Control: no-cache",
   ].join("\n");

devtools/client/netmonitor/test/browser_net_headers_sorted.js

@@ -71,6 +71,7 @@ async function verifyHeaders(monitor) {
     "Cookie",
     "Host",
     "Pragma",
+    "Priority",
     "Sec-Fetch-Dest",
     "Sec-Fetch-Mode",
     "Sec-Fetch-Site",
@@ -144,6 +145,7 @@ async function verifyRawHeaders(monitor) {
     "Sec-Fetch-Dest",
     "Sec-Fetch-Mode",
     "Sec-Fetch-Site",
+    "Priority",
     "Pragma",
     "Cache-Control",
   ];

devtools/client/netmonitor/test/browser_net_http3_request_details.js

@@ -74,8 +74,8 @@ add_task(async function () {
     // The Text-Encoding header is not consistently displayed, exclude it from
     // the assertion. See Bug 1830053.
     headers.filter(cell => cell.textContent != "TE").length,
-    25,
-    "There should be 25 header values displayed in this tabpanel."
+    26,
+    "There should be 26 header values displayed in this tabpanel."
   );
 
   const headersTable = tabpanel.querySelector(".accordion");

devtools/client/netmonitor/test/browser_net_resend.js

@@ -60,14 +60,23 @@ async function testResendRequest() {
     "The resent request has the same url and query parameters and the first request"
   );
 
+  // The priority header only appears when the urgency and incremental values
+  // are not both default values (u=3 and i=false). In this case the original
+  // request has no priority header and the resent request does, hence we subtract one.
   is(
     firstResend.originalResource.requestHeaders.headers.length,
-    firstResend.newResource.requestHeaders.headers.length,
+    firstResend.newResource.requestHeaders.headers.length - 1,
     "The no of headers are the same"
   );
 
+  // Because a resent request has a different purpose and principal it will
+  // also have a different CoS flag (meaning a different priority header).
+  // So we can't compare the original and resent request's priority and skip it.
   firstResend.originalResource.requestHeaders.headers.forEach(
     ({ name, value }) => {
+      if (name === "Priority") {
+        return;
+      }
       const foundHeader = firstResend.newResource.requestHeaders.headers.find(
         header => header.name == name
       );

devtools/client/netmonitor/test/browser_net_simple-request-details.js

@@ -137,8 +137,8 @@ add_task(async function () {
 
     is(
       tabpanel.querySelectorAll(".accordion .treeLabelCell").length,
-      23,
-      "There should be 23 header values displayed in this tabpanel."
+      24,
+      "There should be 24 header values displayed in this tabpanel."
     );
 
     const headersTable = tabpanel.querySelector(".accordion");

modules/libpref/init/StaticPrefList.yaml

@@ -11623,6 +11623,12 @@
   mirror: always
   do_not_use_directly: true
 
+# Whether to add urgency and incremental to request headers
+- name: network.http.priority_header.enabled
+  type: bool
+  value: true
+  mirror: always
+
 # The maximum allowed length for a referrer header - 4096 default.
 # 0 means no limit.
 - name: network.http.referer.referrerLengthLimit

netwerk/protocol/http/Http3Stream.cpp

@@ -42,7 +42,7 @@ Http3Stream::Http3Stream(nsAHttpTransaction* httpTransaction,
     mTransactionBrowserId = trans->BrowserId();
   }
 
-  SetPriority(cos.Flags());
+  mPriorityUrgency = nsHttpHandler::UrgencyFromCoSFlags(cos.Flags());
   SetIncremental(cos.Incremental());
 }
 
@@ -80,31 +80,6 @@ bool Http3Stream::GetHeadersString(const char* buf, uint32_t avail,
   return true;
 }
 
-void Http3Stream::SetPriority(uint32_t aCos) {
-  if (aCos & nsIClassOfService::UrgentStart) {
-    // coming from an user interaction => response should be the highest
-    // priority
-    mPriorityUrgency = 1;
-  } else if (aCos & nsIClassOfService::Leader) {
-    // main html document normal priority
-    mPriorityUrgency = 2;
-  } else if (aCos & nsIClassOfService::Unblocked) {
-    mPriorityUrgency = 3;
-  } else if (aCos & nsIClassOfService::Follower) {
-    mPriorityUrgency = 4;
-  } else if (aCos & nsIClassOfService::Speculative) {
-    mPriorityUrgency = 6;
-  } else if (aCos & nsIClassOfService::Background) {
-    // background tasks can be deprioritzed to the lowest priority
-    mPriorityUrgency = 6;
-  } else if (aCos & nsIClassOfService::Tail) {
-    mPriorityUrgency = 6;
-  } else {
-    // all others get a lower priority than the main html document
-    mPriorityUrgency = 4;
-  }
-}
-
 void Http3Stream::SetIncremental(bool incremental) {
   mPriorityIncremental = incremental;
 }

netwerk/protocol/http/Http3Stream.h

@@ -67,7 +67,6 @@ class Http3Stream final : public nsAHttpSegmentReader,
   bool GetHeadersString(const char* buf, uint32_t avail, uint32_t* countUsed);
   nsresult StartRequest();
 
-  void SetPriority(uint32_t aCos);
   void SetIncremental(bool incremental);
 
   /**

netwerk/protocol/http/nsHttpChannel.cpp

@@ -485,6 +485,25 @@ void nsHttpChannel::HandleContinueCancellingByURLClassifier(
   ContinueCancellingByURLClassifier(aErrorCode);
 }
 
+void nsHttpChannel::SetPriorityHeader() {
+  uint8_t urgency = nsHttpHandler::UrgencyFromCoSFlags(mClassOfService.Flags());
+  bool incremental = mClassOfService.Incremental();
+
+  nsPrintfCString value(
+      "%s", urgency != 3 ? nsPrintfCString("u=%d", urgency).get() : "");
+
+  if (incremental) {
+    if (!value.IsEmpty()) {
+      value.Append(", ");
+    }
+    value.Append("i");
+  }
+
+  if (!value.IsEmpty()) {
+    SetRequestHeader("Priority"_ns, value, false);
+  }
+}
+
 nsresult nsHttpChannel::OnBeforeConnect() {
   nsresult rv = NS_OK;
 
@@ -1204,6 +1223,10 @@ nsresult nsHttpChannel::SetupTransaction() {
 
   mozilla::MutexAutoLock lock(mRCWNLock);
 
+  if (StaticPrefs::network_http_priority_header_enabled()) {
+    SetPriorityHeader();
+  }
+
   // If we're racing cache with network, conditional or byte range header
   // could be added in OnCacheEntryCheck. We cannot send conditional request
   // without having the entry, so we need to remove the headers here and

netwerk/protocol/http/nsHttpChannel.h

@@ -566,6 +566,8 @@ class nsHttpChannel final : public HttpBaseChannel,
   // This method can only be called on the main thread.
   void PerformBackgroundCacheRevalidationNow();
 
+  void SetPriorityHeader();
+
  private:
   nsCOMPtr<nsICancelable> mProxyRequest;
 

netwerk/protocol/http/nsHttpHandler.cpp

@@ -714,6 +714,34 @@ nsresult nsHttpHandler::GenerateHostPort(const nsCString& host, int32_t port,
   return NS_GenerateHostPort(host, port, hostLine);
 }
 
+// static
+uint8_t nsHttpHandler::UrgencyFromCoSFlags(uint32_t cos) {
+  uint8_t urgency;
+  if (cos & nsIClassOfService::UrgentStart) {
+    // coming from an user interaction => response should be the highest
+    // priority
+    urgency = 1;
+  } else if (cos & nsIClassOfService::Leader) {
+    // main html document normal priority
+    urgency = 2;
+  } else if (cos & nsIClassOfService::Unblocked) {
+    urgency = 3;
+  } else if (cos & nsIClassOfService::Follower) {
+    urgency = 4;
+  } else if (cos & nsIClassOfService::Speculative) {
+    urgency = 6;
+  } else if (cos & nsIClassOfService::Background) {
+    // background tasks can be deprioritzed to the lowest priority
+    urgency = 6;
+  } else if (cos & nsIClassOfService::Tail) {
+    urgency = 6;
+  } else {
+    // all others get a lower priority than the main html document
+    urgency = 4;
+  }
+  return urgency;
+}
+
 //-----------------------------------------------------------------------------
 // nsHttpHandler <private>
 //-----------------------------------------------------------------------------

netwerk/protocol/http/nsHttpHandler.h

@@ -429,6 +429,8 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
                                                  int32_t port,
                                                  nsACString& hostLine);
 
+  static uint8_t UrgencyFromCoSFlags(uint32_t cos);
+
   SpdyInformation* SpdyInfo() { return &mSpdyInfo; }
   bool IsH2MandatorySuiteEnabled() { return mH2MandatorySuiteEnabled; }
 

netwerk/test/unit/test_http3_prio_disabled.js

@@ -16,6 +16,7 @@ load("../unit/test_http3_prio_helpers.js");
 if (!inChildProcess()) {
   registerCleanupFunction(async () => {
     Services.prefs.clearUserPref("network.http.http3.priority");
+    Services.prefs.clearUserPref("network.http.priority_header.enabled");
     http3_clear_prefs();
   });
 }
@@ -75,7 +76,7 @@ async function test_http3_prio_disabled(incremental) {
     null
   );
   await test_flag_priority(
-    "disabled (background)",
+    "disabled (tail)",
     Ci.nsIClassOfService.Tail,
     null,
     incremental,
@@ -91,6 +92,7 @@ add_task(async function test_http3_prio_disabled_inc_true() {
   // wrapper handles when testing as content process for pref change
   if (!inChildProcess()) {
     Services.prefs.setBoolPref("network.http.http3.priority", false);
+    Services.prefs.setBoolPref("network.http.priority_header.enabled", false);
   }
   await test_http3_prio_disabled(true);
 });
@@ -101,6 +103,7 @@ add_task(async function test_http3_prio_disabled_inc_false() {
   // wrapper handles when testing as content process for pref change
   if (!inChildProcess()) {
     Services.prefs.setBoolPref("network.http.http3.priority", false);
+    Services.prefs.setBoolPref("network.http.priority_header.enabled", false);
   }
   await test_http3_prio_disabled(false);
 });

netwerk/test/unit/test_http3_prio_enabled.js

@@ -16,6 +16,7 @@ load("../unit/test_http3_prio_helpers.js");
 if (!inChildProcess()) {
   registerCleanupFunction(async () => {
     Services.prefs.clearUserPref("network.http.http3.priority");
+    Services.prefs.clearUserPref("network.http.priority_header.enabled");
     http3_clear_prefs();
   });
 }
@@ -82,7 +83,7 @@ async function test_http3_prio_enabled(incremental) {
     incremental
   );
   await test_flag_priority(
-    "enabled (background)",
+    "enabled (tail)",
     Ci.nsIClassOfService.Tail,
     "u=6",
     incremental,
@@ -95,6 +96,7 @@ add_task(async function test_http3_prio_enabled_incremental_true() {
   // wrapper handles when testing as content process for pref change
   if (!inChildProcess()) {
     Services.prefs.setBoolPref("network.http.http3.priority", true);
+    Services.prefs.setBoolPref("network.http.priority_header.enabled", true);
   }
   await test_http3_prio_enabled(true);
 });
@@ -103,6 +105,7 @@ add_task(async function test_http3_prio_enabled_incremental_false() {
   // wrapper handles when testing as content process for pref change
   if (!inChildProcess()) {
     Services.prefs.setBoolPref("network.http.http3.priority", true);
+    Services.prefs.setBoolPref("network.http.priority_header.enabled", true);
   }
   await test_http3_prio_enabled(false);
 });

netwerk/test/unit_ipc/test_http3_prio_disabled_wrap.js

@@ -4,6 +4,7 @@
 
 registerCleanupFunction(async () => {
   Services.prefs.clearUserPref("network.http.http3.priority");
+  Services.prefs.clearUserPref("network.http.priority_header.enabled");
   http3_clear_prefs();
 });
 
@@ -15,6 +16,7 @@ add_task(async function setup() {
 async function run_test() {
   // test priority urgency and incremental with priority disabled
   Services.prefs.setBoolPref("network.http.http3.priority", false);
+  Services.prefs.setBoolPref("network.http.priority_header.enabled", false);
   run_test_in_child("../unit/test_http3_prio_disabled.js");
   run_next_test(); // only pumps next async task from this file
 }

netwerk/test/unit_ipc/test_http3_prio_enabled_wrap.js

@@ -4,6 +4,7 @@
 
 registerCleanupFunction(async () => {
   Services.prefs.clearUserPref("network.http.http3.priority");
+  Services.prefs.clearUserPref("network.http.priority_header.enabled");
   http3_clear_prefs();
 });
 
@@ -15,6 +16,7 @@ add_task(async function setup() {
 async function run_test() {
   // test priority urgency and incremental with priority enabled
   Services.prefs.setBoolPref("network.http.http3.priority", true);
+  Services.prefs.setBoolPref("network.http.priority_header.enabled", true);
   run_test_in_child("../unit/test_http3_prio_enabled.js");
   run_next_test(); // only pumps next async task from this file
 }

remote/cdp/test/browser/network/browser_navigationEvents.js

@@ -76,6 +76,12 @@ add_task(async function eventsForTopFrameNavigation({ client }) {
     "The same loaderId is used for dependent responses (Bug 1637838)"
   );
   is(scriptResponse.response.url, FRAMESET_JS_URL, "Got the Script response");
+
+  // The priority header only appears when the urgency and incremental values
+  // are not both default values (u=3 and i=false). In this case the scriptRequest.request.headers
+  // has no priority header and scriptResponse.response.requestHeaders does, hence we delete.
+  delete scriptResponse.response.requestHeaders.priority;
+
   Assert.deepEqual(
     scriptResponse.response.requestHeaders,
     scriptRequest.request.headers,
@@ -125,6 +131,10 @@ add_task(async function eventsForTopFrameNavigation({ client }) {
     subscriptResponse.loaderId === subdocRequest.loaderId,
     "The same loaderId is used for dependent responses (Bug 1637838)"
   );
+
+  // see comment above
+  delete subscriptResponse.response.requestHeaders.priority;
+
   Assert.deepEqual(
     subscriptResponse.response.requestHeaders,
     subscriptRequest.request.headers,

services/common/tests/unit/test_restrequest.js

@@ -578,6 +578,7 @@ add_task(async function test_get_no_headers() {
     "sec-fetch-mode",
     "sec-fetch-site",
     "sec-fetch-user",
+    "priority",
   ];
   let request = new RESTRequest(server.baseURI + "/resource");
   await request.get();

toolkit/components/extensions/test/xpcshell/test_ext_dnr_modifyHeaders.js

@@ -29,6 +29,7 @@ server.registerPathHandler("/echoheaders", (req, res) => {
   dropDefaultHeader("accept-language");
   dropDefaultHeader("accept-encoding");
   dropDefaultHeader("connection");
+  dropDefaultHeader("priority");
 
   res.write(JSON.stringify(headers));
 });