Change ambiguous bool `is_ephemeral` to enum VisitedContextEphemerality [chromium/src : main]
0 views
Skip to first unread message
Svend Larsen (Gerrit)
Sep 25, 2025, 10:51:00 AM (14 hours ago) Sep 25
to Tiffany Song, Andrew Liu, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org
Attention needed from Tiffany Song
Svend Larsen voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Tiffany Song
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tiffany Song (Gerrit)
Sep 25, 2025, 11:19:51 AM (14 hours ago) Sep 25
to Andrew Liu, Svend Larsen, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org
Attention needed from Svend Larsen
Tiffany Song added 2 comments![]( "Open in Gerrit")
File components/history/core/browser/history_backend.h
Line 873, Patchset 3:
// credentialless iframe (which is an ephemeral context). When true, we wantSvend Larsen . resolved
Tiffany SongSorry I missed this earlier — this still refers to a bool
```suggestion
// credentialless iframe (which is an ephemeral context). When `kEphemeral`, we want
```
Done
File components/history/core/browser/history_backend.cc
Line 1449, Patchset 3:
// Determine if the visited link is already in the database.Svend Larsen . resolved
Tiffany SongI think this is another accidental revert from merge conflict resolution — this comment was recently removed.
Done
Related details
Attention is currently required from:
- Svend Larsen
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Svend Larsen (Gerrit)
Sep 25, 2025, 11:39:43 AM (13 hours ago) Sep 25
to Tiffany Song, Marc Treib, Andrew Liu, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org
Attention needed from Marc Treib and Tiffany Song
Svend Larsen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Svend Larsen
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Marc Treib
- Tiffany Song
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marc Treib (Gerrit)
Sep 25, 2025, 12:10:59 PM (13 hours ago) Sep 25
to Tiffany Song, Marc Treib, Svend Larsen, Andrew Liu, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org
Attention needed from Tiffany Song
Marc Treib voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Tiffany Song
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tiffany Song (Gerrit)
Sep 25, 2025, 12:51:58 PM (12 hours ago) Sep 25
to Marc Treib, Svend Larsen, Andrew Liu, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org
Tiffany Song voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Sep 25, 2025, 1:54:45 PM (11 hours ago) Sep 25
to Tiffany Song, Marc Treib, Svend Larsen, Andrew Liu, chromium...@chromium.org, browser-comp...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, marq+...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Change ambiguous bool `is_ephemeral` to enum VisitedContextEphemerality
The HistoryAddPageArgs and AddPageVisit default constructors had a
boolean parameter, `is_ephemeral`, with a default value that is
placed after several other booleans with no default values. This creates
dangerous ambiguity when adding new boolean params, as the compiler
would not flag any issues if one were to mistakenly omit a parameter.
This change converts the boolean parameter into an effectively
equivalent enum to be more explicit.
Bug: 439920192
Change-Id: I178d9466063847cdd3880d3c8b8db527d2d883f3
Commit-Queue: Tiffany Song <tiffa...@google.com>
Reviewed-by: Svend Larsen <sv...@chromium.org>
Reviewed-by: Marc Treib <tr...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1520740}
Files:
- M chrome/browser/history/history_tab_helper.cc
- M chrome/browser/supervised_user/supervised_user_navigation_observer.cc
- M components/history/core/browser/history_backend.cc
- M components/history/core/browser/history_backend.h
- M components/history/core/browser/history_backend_unittest.cc
- M components/history/core/browser/history_service.cc
- M components/history/core/browser/history_service_unittest.cc
- M components/history/core/browser/history_types.cc
- M components/history/core/browser/history_types.h
- M ios/chrome/browser/history/model/history_tab_helper.mm
Change size: M
Delta: 10 files changed, 105 insertions(+), 85 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Svend Larsen, +1 by Marc Treib
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages