1839315 - Add "fetchpriority" attribute to <link> element and `Link` headers, first parts

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P2)

Description

[Mirko Brodesser (:mbrodesser-Igalia)]

https://bugzilla.mozilla.org/show_bug.cgi?id=1797715#c2

Comment 1

[Mirko Brodesser (:mbrodesser-Igalia)]

The standard defines multiple link types (https://html.spec.whatwg.org/multipage/semantics.html#attr-link-rel), so perhaps this requires updating more loader functions than the ones mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1797715#c2.

Comment 2

[Mirko Brodesser (:mbrodesser-Igalia)]

When implementing this, grep the code for references to this bug (e.g. grep -i "Bug <bugnumber>").

Comment 3

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 1) Lift code for `fetchpriority` attribute from `HTMLScriptElement` to `nsGenericHTMLElementWithFetchPriorityAttribute`. r=smaug

Since the HTMLLinkElement requires it too.

Comment 4

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 2) Add `fetchPriority` attribute to HTMLLinkElement's DOM interface. r=saschanaz

Setting the internal priority depending on the fetchPriority will be
implemented in a following part.

Depends on D189935

Comment 5

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 3) Remove no-op method `PreloaderBase::PrioritizeAsPreload`. r=valentin

Because it's confusing.

Depends on D189936

Comment 6

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 4) Link from `SheetLoadData::mWasAlternate` to spec. r=emilio

Otherwise it's unclear what's meant.

Comment 7

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 5) Add links to spec for parsing the Link header fields a HTTP header contains. r=#necko

Otherwise it's hard to understand what's supposed to happen there.

Depends on D190666

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2598def65c5c
part 3) Remove no-op method `PreloaderBase::PrioritizeAsPreload`. r=necko-reviewers,valentin

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c66a394ad061
part 4) Link from `SheetLoadData::mWasAlternate` to spec. r=emilio DONTBUILD

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2598def65c5c
https://hg.mozilla.org/mozilla-central/rev/c66a394ad061

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9cafd79c8405
part 5) Add links to spec for parsing the Link header fields a HTTP header contains. r=necko-reviewers,valentin

Comment 12

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 6) Lift code for `fetchpriority` attribute from `nsGenericHTMLElementWithFetchPriorityAttribute` to `nsGenericHTMLElement`. r=smaug

As requested in the review of part 1.

Comment 13

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 7) Factor updating an attribute out of `net::ParseLinkHeader`. r=#necko

The function is more than 250 lines long
(https://searchfox.org/mozilla-central/rev/5ad226c7379b0564c76dc3b54b44985356f94c5a/netwerk/base/nsNetUtil.cpp#3448-3703)
and will be extended in a following part.
This refactoring allows to extend the new, simpler function instead.

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/9cafd79c8405

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b6c07041133
part 7) Factor updating an attribute out of `net::ParseLinkHeader`. r=necko-reviewers,jesup

Comment 16

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 8) Store the `fetchpriority` value from Link header fields in the `LinkHeader` struct. r=manuel

Setting the value currently has no user-observable effect. It will be
used in a following part.

Comment 17

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/0b6c07041133

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75b27b040343
part 1) Lift code for `fetchpriority` attribute from `HTMLScriptElement` to `nsGenericHTMLElementWithFetchPriorityAttribute`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/7621c3c4c268
part 2) Add `fetchPriority` attribute to HTMLLinkElement's DOM interface. r=necko-reviewers,webidl,saschanaz,valentin
https://hg.mozilla.org/integration/autoland/rev/9cf48ec99a06
part 6) Lift code for `fetchpriority` attribute from `nsGenericHTMLElementWithFetchPriorityAttribute` to `nsGenericHTMLElement`. r=smaug

Comment 19

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/75b27b040343
https://hg.mozilla.org/mozilla-central/rev/7621c3c4c268
https://hg.mozilla.org/mozilla-central/rev/9cf48ec99a06

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d15361d7639
part 8) Store the `fetchpriority` value from Link header fields in the `LinkHeader` struct. r=manuel

Comment 21

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 9) Add partial support for the "fetchpriority" attribute for loading style sheets from `<link>` elements or `Link` header fields. r=smaug,emilio

Includes only support for dynamically loading style sheets from <link>
elements and Link header fields.
The remaining support, including preloading, will be added in other
patches.

Comment 22

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 10) Separate tests for `<link rel=preload as=style>` and corresponding `Link` header tests from other `as=` tests. r=emilio

This is allows enabling the style tests in a following part. The other
tests can be separated when needed.

Depends on D191745

Comment 23

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/8d15361d7639

Comment 24

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 11) Add `fetchpriority` attribute support for preloaded stylesheets. r=smaug,emilio,#necko-reviewers

Includes corresponding Link header fields.

Comment 25

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 12) Add `fetchpriority` attribute support for `rel=modulepreload` and `rel=preload as=script`. r=smaug,valentin

Comment 26

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 13) Separate tests for `<link rel=preload as=fetch>` from other `<link rel=preload>` tests. r=valentin

Allows passing the separated tests once as=fetch is supported, which
will happen in a following part.

For interoperability the expectations for as=fetch are adapted too
(https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/http/tests/priority-hints/link-preload-initial-load.html;l=39-52;drc=a7b04c9f4d4083e8e1a9d3ba15ba8030fe819719).

Depends on D192331

Comment 27

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 14) Separate tests for `<link rel=preload as=font>` from other preload tests. r=valentin

Allows the separate tests to pass. Otherwise passes are shadowed by
failures. The next parts make use of that.

The tests for images might be adapted in a following part to match the
priorities at
https://web.dev/articles/fetch-priority#browser_priority_and_fetchpriority.

Comment 28

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 15) Add test for internally assigned priority of font-face loaded from `<head>`. r=dholbert

Tests existing behavior. It differs from Chromium's, which assigns
highest priority instead of high.

The test is added for the fetchpriority feature because loading a
font-face font has overlapping code with linking fonts via <link rel=preload as=font> in FontPreloader
(https://searchfox.org/mozilla-central/rev/d6576544301cacc0e393fbc919c53e4e6b0d46ec/layout/style/FontPreloader.h#25).

Comment 29

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 16) Add test for internally assigned priority of font-face loaded from a worker. r=dholbert

Test is added for the same reason as the one for part 15:
in Gecko FontPreloader code is called.

Comment 30

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1839315: part 17) Add test for internally assigned priority of font-face loaded from a script. r=dholbert

Complements parts 15 and 16.

Depends on D193657

Comment 31

[Phabricator Automation]

Comment on attachment 9356374 [details]
Bug 1839315: part 1) Lift code for fetchpriority attribute from HTMLScriptElement to nsGenericHTMLElementWithFetchPriorityAttribute. r=smaug

Revision D189935 was moved to bug 1865610. Setting attachment 9356374 [details] to obsolete.

Comment 32

[Phabricator Automation]

Comment on attachment 9360017 [details]
Bug 1839315: part 9) Add partial support for the "fetchpriority" attribute for loading style sheets from <link> elements or Link header fields. r=smaug,emilio

Revision D191745 was moved to bug 1865610. Setting attachment 9360017 [details] to obsolete.

Comment 33

[Phabricator Automation]

Comment on attachment 9360018 [details]
Bug 1839315: part 10) Separate tests for <link rel=preload as=style> and corresponding Link header tests from other as= tests. r=emilio

Revision D191746 was moved to bug 1865610. Setting attachment 9360018 [details] to obsolete.

Comment 34

[Phabricator Automation]

Comment on attachment 9360234 [details]
Bug 1839315: part 11) Add fetchpriority attribute support for preloaded stylesheets. r=smaug,emilio,#necko-reviewers

Revision D191866 was moved to bug 1865610. Setting attachment 9360234 [details] to obsolete.

Comment 35

[Phabricator Automation]

Comment on attachment 9361204 [details]
Bug 1839315: part 12) Add fetchpriority attribute support for rel=modulepreload and rel=preload as=script. r=smaug,valentin

Revision D192331 was moved to bug 1865610. Setting attachment 9361204 [details] to obsolete.

Comment 36

[Phabricator Automation]

Comment on attachment 9361205 [details]
Bug 1839315: part 13) Separate tests for <link rel=preload as=fetch> from other <link rel=preload> tests. r=valentin

Revision D192332 was moved to bug 1865610. Setting attachment 9361205 [details] to obsolete.

Comment 37

[Phabricator Automation]

Comment on attachment 9363277 [details]
Bug 1839315: part 14) Separate tests for <link rel=preload as=font> from other preload tests. r=valentin

Revision D193435 was moved to bug 1865610. Setting attachment 9363277 [details] to obsolete.

Comment 38

[Phabricator Automation]

Comment on attachment 9363497 [details]
Bug 1839315: part 15) Add test for internally assigned priority of font-face loaded from <head>. r=dholbert

Revision D193550 was moved to bug 1865610. Setting attachment 9363497 [details] to obsolete.

Comment 39

[Phabricator Automation]

Comment on attachment 9363690 [details]
Bug 1839315: part 16) Add test for internally assigned priority of font-face loaded from a worker. r=dholbert

Revision D193657 was moved to bug 1865610. Setting attachment 9363690 [details] to obsolete.

Comment 40

[Phabricator Automation]

Comment on attachment 9363872 [details]
Bug 1839315: part 17) Add test for internally assigned priority of font-face loaded from a script. r=dholbert

Revision D193766 was moved to bug 1865610. Setting attachment 9363872 [details] to obsolete.

Comment 41

[Mirko Brodesser (:mbrodesser-Igalia)]

Work continued in https://bugzilla.mozilla.org/show_bug.cgi?id=1865610.