[glic] Clarify sensitive list vs various blocklists [chromium/src : main]
Chris Fredrickson (Gerrit)
Chris Fredrickson voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Dylan Cutler
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dylan Cutler (Gerrit)
Dylan Cutler added 1 comment![]( "Open in Gerrit")
I appreciate the rename, originally this was supposed to be a blocklist when I wrote it so this update is needed.
That being said, I think replacing `Blocklist` with `Confirmlist` is a better rename. It conveys what action these sites require more clearly than "sensitive" which requires a further step of explanation. Furthermore, if we want to have sites be manually confirmed for other reasons then the name remains accurate :)
Related details
- Chris Fredrickson
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. |
Chris Fredrickson (Gerrit)
Chris Fredrickson added 1 comment![]( "Open in Gerrit")
I appreciate the rename, originally this was supposed to be a blocklist when I wrote it so this update is needed.
That being said, I think replacing `Blocklist` with `Confirmlist` is a better rename. It conveys what action these sites require more clearly than "sensitive" which requires a further step of explanation. Furthermore, if we want to have sites be manually confirmed for other reasons then the name remains accurate :)
Hm, I was under the impression that there wasn't necessarily a confirm dialog either? I.e. if kGlicPromptUserForSensitiveNavigations is disabled then the navigation is blocked. I would argue that that means "confirmlist" is misleading for the same reason that "blocklist" is, therefore.
That's why I chose "sensitive", since that's really the semantic that's associated with those origins -- and it's up to the client's feature state to decide how to handle a sensitive origin. WDYT?
Related details
- Dylan Cutler
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. |
Dylan Cutler (Gerrit)
Dylan Cutler voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dylan Cutler
| Code-Review | +1 |
1 comment
Chris FredricksonI appreciate the rename, originally this was supposed to be a blocklist when I wrote it so this update is needed.
That being said, I think replacing `Blocklist` with `Confirmlist` is a better rename. It conveys what action these sites require more clearly than "sensitive" which requires a further step of explanation. Furthermore, if we want to have sites be manually confirmed for other reasons then the name remains accurate :)
Hm, I was under the impression that there wasn't necessarily a confirm dialog either? I.e. if kGlicPromptUserForSensitiveNavigations is disabled then the navigation is blocked. I would argue that that means "confirmlist" is misleading for the same reason that "blocklist" is, therefore.
That's why I chose "sensitive", since that's really the semantic that's associated with those origins -- and it's up to the client's feature state to decide how to handle a sensitive origin. WDYT?
- Chris Fredrickson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chris Fredrickson (Gerrit)
Chris Fredrickson voted Auto-Submit+1![]( "Open in Gerrit")
- Jeremy Roman
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jeremy Roman (Gerrit)
Jeremy Roman voted![]( "Open in Gerrit")
- Chris Fredrickson
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)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[glic] Clarify sensitive list vs various blocklists
ExecutionEngine currently supports multiple origin blocklists (global
list via component updater; enterprise policy). It also supports a list
of origins provided by OptimizationGuide for which navigation to those
origins is considered "sensitive". Depending on feature flag
configuration, navigation to sensitive origins may be either blocked
outright, or may show a user confirmation dialog.
This CL renames/rewords everything about the OptimizationGuide
"blocklist"/"confirmlist" and calls it a "sensitive" list instead, since
presence on the list doesn't guarantee the navigation will be blocked
(or show a user confirmation, conversely).
- M chrome/browser/actor/execution_engine.cc
- M chrome/browser/actor/execution_engine.h
Code-Review: +1 by Jeremy Roman, +1 by Dylan Cutler
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |