Remove prerendered host from started_prerenders_ after successful activation of prerender-into-new-tab [chromium/src : main]
0 views
Skip to first unread message
Kenneth Lu (Gerrit)
Jun 11, 2026, 9:08:01 AMJun 11
to Hiroki Nakagawa, Chromium LUCI CQ, Peter Marshall, chromium...@chromium.org, gavin...@chromium.org, prerenderi...@chromium.org, tburkar...@chromium.org
Attention needed from Hiroki Nakagawa
Kenneth Lu added 3 comments![]( "Open in Gerrit")
Patchset-level comments
Commit Message
Line 7, Patchset 4:
Remove prerendered host from started_prerenders_ after successful activationHiroki Nakagawa . resolved
Kenneth LuCan you clarify this change is for prerender-into-new-tab in the CL description?
Done
Related details
Attention is currently required from:
- Hiroki Nakagawa
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. |
Kenneth Lu (Gerrit)
Jun 11, 2026, 9:09:16 AMJun 11
to Hiroki Nakagawa, Chromium LUCI CQ, Peter Marshall, chromium...@chromium.org, gavin...@chromium.org, prerenderi...@chromium.org, tburkar...@chromium.org
Attention needed from Hiroki Nakagawa
Kenneth Lu added 2 comments![]( "Open in Gerrit")
Patchset-level comments
Kenneth Lu . resolved
Updated naming and comments
File content/browser/preloading/prerender/prerender_browsertest.cc
Line 4197, Patchset 4:
const GURL kPrerenderingUrl = GetUrl("/title2.html");Hiroki Nakagawa . resolved
Kenneth LuCan you rename these variables to `initial_url` and `prerendering_url`?
I know some tests in this file use the k- prefix for URLs (this is my fault...), but it's not very appropriate according to the style guide that recommends the "k" prefix for variables whose storage duration is static, not for other storage types like these URLs:
https://google.github.io/styleguide/cppguide.html#Constant_Names(I'll make a rename CL for other tests later)
Done
Kenneth Lu (Gerrit)
Jun 11, 2026, 10:09:35 AMJun 11
to Hiroki Nakagawa, Chromium LUCI CQ, Peter Marshall, chromium...@chromium.org, gavin...@chromium.org, prerenderi...@chromium.org, tburkar...@chromium.org
Attention needed from Hiroki Nakagawa
Kenneth Lu added 3 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 7 (Latest):
Kenneth Lu . resolved
Renamed to onRetriggerable -- updated the comments referencing the old naming
File content/browser/preloading/prerender/prerender_host_registry.cc
Line 1346, Patchset 4:
NotifyCancel(Hiroki Nakagawa . resolved
Kenneth LuThe name `NotifyCancel` no longer matches its purpose (activation is not cancellation). Can we rename this to `NotifyRetriggerable()`?
Done
File content/browser/preloading/prerenderer_impl.cc
Line 587, Patchset 4:
void PrerendererImpl::OnCancel(PrerenderHostId host_id,Hiroki Nakagawa . resolved
Kenneth LuThis observer also needs to be renamed to `OnRetriggerable`.
Done
Related details
Attention is currently required from:
- Hiroki Nakagawa
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Jun 24, 2026, 12:12:28 PM (4 days ago) Jun 24
to Kenneth Lu, Peter Marshall, Chromium LUCI CQ, chromium...@chromium.org, gavin...@chromium.org, prerenderi...@chromium.org, tburkar...@chromium.org
Attention needed from Kenneth Lu
Hiroki Nakagawa voted and added 1 comment![]( "Open in Gerrit")
Votes added by Hiroki Nakagawa
| Code-Review | +1 |
1 comment
Patchset-level comments
Hiroki Nakagawa . resolved
LGTM, thank you! Sorry for the late response.
Related details
Attention is currently required from:
- Kenneth Lu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Marshall (Gerrit)
Jun 24, 2026, 8:06:46 PM (3 days ago) Jun 24
to Kenneth Lu, Hiroki Nakagawa, Chromium LUCI CQ, chromium...@chromium.org, gavin...@chromium.org, prerenderi...@chromium.org, tburkar...@chromium.org
Attention needed from Kenneth Lu
Peter Marshall voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Kenneth Lu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kenneth Lu (Gerrit)
Jun 24, 2026, 8:06:58 PM (3 days ago) Jun 24
to Peter Marshall, Hiroki Nakagawa, Chromium LUCI CQ, chromium...@chromium.org, gavin...@chromium.org, prerenderi...@chromium.org, tburkar...@chromium.org
Kenneth Lu voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Peter Marshall (Gerrit)
Jun 24, 2026, 8:09:31 PM (3 days ago) Jun 24
to Kenneth Lu, Hiroki Nakagawa, Chromium LUCI CQ, chromium...@chromium.org, gavin...@chromium.org, prerenderi...@chromium.org, tburkar...@chromium.org
Attention needed from Kenneth Lu
Peter Marshall voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Kenneth Lu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jun 24, 2026, 9:27:02 PM (3 days ago) Jun 24
to Kenneth Lu, Peter Marshall, Hiroki Nakagawa, chromium...@chromium.org, gavin...@chromium.org, prerenderi...@chromium.org, tburkar...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Remove prerendered host from started_prerenders_ after successful activation of prerender-into-new-tab
Change-Id: I9243c2b89425a523ed88eee1a3ddee10d252a4e8
Bug: 513412121
Reviewed-by: Peter Marshall <peterm...@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhi...@chromium.org>
Commit-Queue: Peter Marshall <peterm...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1652113}
Files:
- M content/browser/preloading/prerender/prerender_browsertest.cc
- M content/browser/preloading/prerender/prerender_host_registry.cc
- M content/browser/preloading/prerender/prerender_host_registry.h
- M content/browser/preloading/prerenderer_impl.cc
- M content/browser/preloading/prerenderer_impl.h
Change size: M
Delta: 5 files changed, 74 insertions(+), 16 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Hiroki Nakagawa, +1 by Peter Marshall
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages