Implement DOM.forceShowInterest [chromium/src : main]
0 views
Skip to first unread message
Philip Pfaffe (Gerrit)
Jul 31, 2026, 7:27:49 AM (19 hours ago) Jul 31
to Mason Freed, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, kinuko...@chromium.org
Attention needed from Mason Freed
Philip Pfaffe added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Mason Freed
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Jul 31, 2026, 7:46:32 PM (7 hours ago) Jul 31
to Philip Pfaffe, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, kinuko...@chromium.org
Attention needed from Philip Pfaffe
Mason Freed voted and added 6 comments![]( "Open in Gerrit")
Votes added by Mason Freed
| Code-Review | +1 |
6 comments
Patchset-level comments
Mason Freed . resolved
Nice! Thanks for adding this feature! The code looks good - just some small stuff.
File third_party/blink/common/features.cc
Line 2428, Patchset 2 (Parent):
BASE_FEATURE(kDevToolsAllowPopoverForcing, base::FEATURE_ENABLED_BY_DEFAULT);Mason Freed . unresolved
Did these all mean to overwrite the old feature? (Usually it's preferable to remove the old flag in one CL and add a new one in a separate one, in case of issues.)
Up to you - I'm ok as-is, if you prefer.
File third_party/blink/renderer/core/dom/element.cc
Line 2019, Patchset 2 (Latest):
probe::WillLoseInterest(this, &force_interest);Mason Freed . unresolved
WillLoseInterest feels a little confusing to me. Perhaps something more like `ShouldInterestBeForced` or similar?
File third_party/blink/renderer/core/inspector/inspector_dom_agent.cc
Line 2252, Patchset 2 (Latest):
return protocol::Response::ServerError("node has no interest target");Mason Freed . unresolved
nit: maybe "node is not an interest invoker"?
Line 2263, Patchset 2 (Latest):
element->InterestGained(target,
Element::InterestState::kExplicitInterest);Mason Freed . unresolved
I think it might be better to just use `ShowInterestNow()`.
Line 2274, Patchset 2 (Latest):
*force_interest = true;Mason Freed . unresolved
Maybe always set it, rather than only to true?
Related details
Attention is currently required from:
- Philip Pfaffe
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. |
Reply all
Reply to author
Forward
0 new messages