SearchPrefetch: Remove kSearchPrefetchSkipsCancel [chromium/src : main]
1 view
Skip to first unread message
Hiroki Nakagawa (Gerrit)
3:54 AM (20 hours ago) 3:54 AM
to Lingqi Chi, Kouhei Ueno, Chromium Metrics Reviews, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, blundell+...@chromium.org, gavin...@chromium.org, lingqi...@chromium.org, tburkar...@chromium.org
Attention needed from Lingqi Chi
Hiroki Nakagawa voted and added 1 comment![]( "Open in Gerrit")
Votes added by Hiroki Nakagawa
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Lingqi Chi
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
3:55 AM (20 hours ago) 3:55 AM
to Lingqi Chi, Kouhei Ueno, Chromium Metrics Reviews, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, blundell+...@chromium.org, gavin...@chromium.org, lingqi...@chromium.org, tburkar...@chromium.org
Attention needed from Lingqi Chi
Hiroki Nakagawa added 1 comment![]( "Open in Gerrit")
File chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_request.cc
Line 603, Patchset 7 (Latest):
// Since we are cancelling prefetch when either request failed weHiroki Nakagawa . unresolved
I'll remove this `either` in the next patchset.
Related details
Attention is currently required from:
- Lingqi Chi
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lingqi Chi (Gerrit)
4:32 AM (19 hours ago) 4:32 AM
to Hiroki Nakagawa, Kouhei Ueno, Chromium Metrics Reviews, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, blundell+...@chromium.org, gavin...@chromium.org, lingqi...@chromium.org, tburkar...@chromium.org
Attention needed from Hiroki Nakagawa
Lingqi Chi voted and added 2 comments![]( "Open in Gerrit")
Votes added by Lingqi Chi
| Code-Review | +1 |
2 comments
Patchset-level comments
Lingqi Chi . resolved
thanks!
File chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.cc
Line 724, Patchset 7 (Latest):
// TODO(crbug.com/40214220): Unlike prefetch, which does not discard completed
// response to avoid wasting, prerender would like to cancel itself given the
// cost of a prerender. For now prenderer is canceled when the prerender hints
// changed, we need to revisit this decision.Lingqi Chi . resolved
I feel like we may want to skip this part for prerender as well. let me think of that....
Related details
Attention is currently required from:
- Hiroki Nakagawa
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
7:50 AM (16 hours ago) 7:50 AM
to Lingqi Chi, Kouhei Ueno, Chromium Metrics Reviews, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, blundell+...@chromium.org, gavin...@chromium.org, lingqi...@chromium.org, tburkar...@chromium.org
Hiroki Nakagawa added 3 comments![]( "Open in Gerrit")
Patchset-level comments
File chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_request.cc
Line 603, Patchset 7:
// Since we are cancelling prefetch when either request failed weHiroki Nakagawa . resolved
I'll remove this `either` in the next patchset.
Hiroki Nakagawa
Done
File chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.cc
Line 724, Patchset 7:
// TODO(crbug.com/40214220): Unlike prefetch, which does not discard completed
// response to avoid wasting, prerender would like to cancel itself given the
// cost of a prerender. For now prenderer is canceled when the prerender hints
// changed, we need to revisit this decision.Lingqi Chi . resolved
I feel like we may want to skip this part for prerender as well. let me think of that....
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
7:51 AM (16 hours ago) 7:51 AM
to Takashi Toyoshima, Lingqi Chi, Kouhei Ueno, Chromium Metrics Reviews, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, blundell+...@chromium.org, gavin...@chromium.org, lingqi...@chromium.org, tburkar...@chromium.org
Attention needed from Takashi Toyoshima
Hiroki Nakagawa added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Hiroki Nakagawa . resolved
+toyoshim@ for enums.xml. Thanks!
Related details
Attention is currently required from:
- Takashi Toyoshima
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Takashi Toyoshima (Gerrit)
9:25 PM (2 hours ago) 9:25 PM
to Hiroki Nakagawa, Lingqi Chi, Kouhei Ueno, Chromium Metrics Reviews, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, blundell+...@chromium.org, gavin...@chromium.org, lingqi...@chromium.org, tburkar...@chromium.org
Attention needed from Hiroki Nakagawa
Takashi Toyoshima voted and added 1 comment![]( "Open in Gerrit")
Votes added by Takashi Toyoshima
| Code-Review | +1 |
1 comment
File tools/metrics/histograms/metadata/omnibox/enums.xml
Line 185, Patchset 8 (Parent):
<int value="6" label="Prefetch request was cancelled"/>Takashi Toyoshima . unresolved
Maybe we keep this definition here, but add "(removed)" or something to the label? We will keep receiving this value from older releases for a while, and the value would appear in the dashboard. So, it's nice to keep the description there, I think.
Related details
Attention is currently required from:
- Hiroki Nakagawa
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
10:22 PM (1 hour ago) 10:22 PM
to Hiroki Nakagawa, Takashi Toyoshima, Lingqi Chi, Kouhei Ueno, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, prerendering-reviews, asvitkine...@chromium.org, blundell+...@chromium.org, gavin...@chromium.org, lingqi...@chromium.org, tburkar...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
8 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: tools/metrics/histograms/metadata/omnibox/enums.xml
Insertions: 2, Deletions: 0.
@@ -182,6 +182,7 @@
<int value="3" label="Navigation URL was not a default search URL"/>
<int value="4" label="No prefetch issued for the search terms"/>
<int value="5" label="Prefetch was for a different origin"/>
+ <int value="6" label="Prefetch request was cancelled (Obsolete)"/>
<int value="7" label="Prefetch request failed"/>
<int value="8" label="Another reason (unexpected)"/>
<int value="9" label="POST, reload, form, link, or other non-cache loads."/>
@@ -198,6 +199,7 @@
streaming responses)"/>
<int value="4" label="Completed the prefetch"/>
<int value="5" label="Request failed"/>
+ <int value="6" label="Request cancelled (Obsolete)"/>
<int value="7" label="Prefetch request served for real navigation"/>
<int value="8" label="Was served to prerender navigation stack"/>
<int value="9"
```
Change information
Commit message:
SearchPrefetch: Remove kSearchPrefetchSkipsCancel
This feature was enabled by default by https://crrev.com/c/4469310.
NO_IFTTT=Changes will be done in the separate repository later.
Change-Id: I04e933b7dd49e7c842bfd106b2536f5d516396c3
Bug: b/262915418
Reviewed-by: Takashi Toyoshima <toyo...@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhi...@chromium.org>
Reviewed-by: Lingqi Chi <lin...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1322533}
Files:
- M chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.cc
- M chrome/browser/preloading/prefetch/search_prefetch/field_trial_settings.h
- M chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_request.cc
- M chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_request.h
- M chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.cc
- M chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.h
- M chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_browsertest.cc
- M chrome/browser/preloading/prefetch/search_prefetch/search_preload_unified_browsertest.cc
- M tools/metrics/histograms/metadata/omnibox/enums.xml
Change size: M
Delta: 9 files changed, 16 insertions(+), 109 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Lingqi Chi, +1 by Takashi Toyoshima
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages