[Blink] Refactor preload logic to fix parser queueing [chromium/src : main]

0 views

Skip to first unread message

Olivier Li (Gerrit)

unread,

Oct 28, 2025, 9:31:14 AM (9 days ago) Oct 28

to Alex Attar, AI Code Reviewer, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org

Attention needed from Alex Attar

Olivier Li added 1 comment

File third_party/blink/renderer/core/html/parser/html_resource_preloader.h

Line 53, Patchset 1: static bool ShouldPreload(const Document* document,

AI Code Reviewer . resolved

Blink Style Guide: Prefer enums or StrongAliases to bare bools for function parameters. To improve readability and type safety, consider using a `base::StrongAlias` for the `no_defer` parameter, for example: `using NoDefer = base::StrongAlias<class NoDeferTag, bool>;`.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason
This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_

Olivier Li

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:

Alex Attar

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Francois Pierre Doray (Gerrit)

unread,

Oct 28, 2025, 2:13:04 PM (9 days ago) Oct 28

to Alex Attar, Olivier Li, AI Code Reviewer, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org

Attention needed from Alex Attar

Francois Pierre Doray added 1 comment

File third_party/blink/renderer/core/html/parser/html_resource_preloader.cc

Line 158, Patchset 7 (Latest): return ShouldPreload(document_, preload->GetResourceType(),

Francois Pierre Doray . unresolved

Should this return true when `preload->IsPreconnect()`, so that we don't filter out preconnect requests? Those are fulfilled by HTMLResourcePreloader::Preload even if AllowPreloadRequest() returns false.

Open in Gerrit

Related details

Attention is currently required from:

Alex Attar

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Alex Attar (Gerrit)

unread,

Nov 4, 2025, 5:34:46 PM (2 days ago) Nov 4

to Kouhei Ueno, Francois Pierre Doray, Olivier Li, AI Code Reviewer, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org

Attention needed from Kouhei Ueno

Alex Attar voted and added 3 comments

Votes added by Alex Attar

Commit-Queue

3 comments

Patchset-level comments

File-level comment, Patchset 6:

Alex Attar . resolved

PTAL,
Thanks

File third_party/blink/renderer/core/html/parser/html_resource_preloader.h

Line 53, Patchset 1: static bool ShouldPreload(const Document* document,

AI Code Reviewer . resolved

Blink Style Guide: Prefer enums or StrongAliases to bare bools for function parameters. To improve readability and type safety, consider using a `base::StrongAlias` for the `no_defer` parameter, for example: `using NoDefer = base::StrongAlias<class NoDeferTag, bool>;`.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason
This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_

Alex Attar

Done

File third_party/blink/renderer/core/html/parser/html_resource_preloader.cc

Line 158, Patchset 7: return ShouldPreload(document_, preload->GetResourceType(),

Francois Pierre Doray . resolved

Should this return true when `preload->IsPreconnect()`, so that we don't filter out preconnect requests? Those are fulfilled by HTMLResourcePreloader::Preload even if AllowPreloadRequest() returns false.

Alex Attar

oh that's a good point, thanks for catching this. I've added a `is_preconnect` check.

Open in Gerrit

Related details

Attention is currently required from:

Kouhei Ueno

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages