Andy Paicu (Gerrit)

unread,

Oct 8, 2025, 6:21:58 AM (11 days ago) Oct 8

to Mason Freed, Chromium LUCI CQ, AI Code Reviewer, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Mason Freed

Andy Paicu voted and added 2 comments

Votes added by Andy Paicu

Commit-Queue

+1

2 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Andy Paicu . resolved

Hi Mason, PTAL

File third_party/blink/renderer/core/html/html_permission_element.h

Line 100, Patchset 1: bool is_registered_in_browser_process() const {

AI Code Reviewer . resolved

nit: Per the Blink Style Guide, function names should use `CamelCase` starting with an uppercase letter. Please rename `is_registered_in_browser_process` to `IsRegisteredInBrowserProcess`. (Blink Style Guide: Naming - Use 'CamelCase' for all function names)
_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason
This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_

Andy Paicu

Won't fix: outdated

Open in Gerrit

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Andy Paicu (Gerrit)

unread,

Oct 8, 2025, 6:22:29 AM (11 days ago) Oct 8

to Thomas Nguyen, Ravjit Uppal, Mason Freed, Chromium LUCI CQ, AI Code Reviewer, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Mason Freed

Andy Paicu added 1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Andy Paicu . resolved

FYI

Open in Gerrit

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mason Freed (Gerrit)

unread,

Oct 9, 2025, 9:38:10 PM (10 days ago) Oct 9

to Andy Paicu, Thomas Nguyen, Ravjit Uppal, Chromium LUCI CQ, AI Code Reviewer, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Andy Paicu

Mason Freed added 5 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Mason Freed . resolved

Seems ok, just some small things,

File third_party/blink/renderer/core/html/html_geolocation_element.cc

Line 88, Patchset 4 (Latest): // TODO(crbug.com/448593977): Remove this when the "preciselocation"

Mason Freed . unresolved

No access to this bug...

Line 91, Patchset 4 (Latest): if (params.name == html_names::kPreciselocationAttr) {

Mason Freed . unresolved

Case sensitive?

Line 100, Patchset 4 (Latest):

  }
  if (params.name == html_names::kWatchAttr) {

Mason Freed . unresolved

These should both be `} else if {`

Line 114, Patchset 4 (Latest): params.new_value == kAccuracyModePrecise) {

Mason Freed . unresolved

case sensitive?

Open in Gerrit

Related details

Attention is currently required from:

Andy Paicu

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Andy Paicu (Gerrit)

unread,

Oct 10, 2025, 7:01:08 AM (9 days ago) Oct 10

to Thomas Nguyen, Ravjit Uppal, Mason Freed, Chromium LUCI CQ, AI Code Reviewer, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Mason Freed

Andy Paicu added 5 comments

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Andy Paicu . resolved

Thank you Mason, PTAL

File third_party/blink/renderer/core/html/html_geolocation_element.cc

Line 88, Patchset 4: // TODO(crbug.com/448593977): Remove this when the "preciselocation"

Mason Freed . resolved

No access to this bug...

Andy Paicu

Done

Line 91, Patchset 4: if (params.name == html_names::kPreciselocationAttr) {

Mason Freed . resolved

Case sensitive?

Andy Paicu

Since this is an attribute name it seems like it's already lowercase. At least looking at other examples it doesn't seem like there is a need to do case-insensitive checks.

e.g.

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_element.cc;drc=62a9a00176f862e688fa47919ed6f19b59f77b5f;bpv=1;bpt=1;l=836

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_source_element.cc;l=162;drc=62a9a00176f862e688fa47919ed6f19b59f77b5f;bpv=1;bpt=1

Line 100, Patchset 4: }


  if (params.name == html_names::kWatchAttr) {

Mason Freed . resolved

These should both be `} else if {`

Andy Paicu

Done

Line 114, Patchset 4: params.new_value == kAccuracyModePrecise) {

Mason Freed . resolved

case sensitive?

Andy Paicu

Done

Open in Gerrit

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Ravjit Uppal (Gerrit)

unread,

Oct 10, 2025, 11:08:19 AM (9 days ago) Oct 10

to Andy Paicu, Thomas Nguyen, Mason Freed, Chromium LUCI CQ, AI Code Reviewer, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Andy Paicu and Mason Freed

Ravjit Uppal added 1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Ravjit Uppal . unresolved

We should also update the idl file, right?

Open in Gerrit

Related details

Attention is currently required from:

Andy Paicu
Mason Freed

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mason Freed (Gerrit)

unread,

Oct 10, 2025, 2:26:12 PM (9 days ago) Oct 10

to Andy Paicu, Thomas Nguyen, Ravjit Uppal, Chromium LUCI CQ, AI Code Reviewer, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Andy Paicu

Mason Freed voted and added 3 comments

Votes added by Mason Freed

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Mason Freed . resolved

Thanks! LGTM.

File third_party/blink/renderer/core/html/html_geolocation_element.cc

Line 91, Patchset 4: if (params.name == html_names::kPreciselocationAttr) {

Mason Freed . resolved

Case sensitive?

Andy Paicu

Since this is an attribute name it seems like it's already lowercase. At least looking at other examples it doesn't seem like there is a need to do case-insensitive checks.
e.g.
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_element.cc;drc=62a9a00176f862e688fa47919ed6f19b59f77b5f;bpv=1;bpt=1;l=836
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_source_element.cc;l=162;drc=62a9a00176f862e688fa47919ed6f19b59f77b5f;bpv=1;bpt=1

Mason Freed

You're right - thanks. I was reviewing too quickly.

File third_party/blink/renderer/core/html/html_geolocation_element_test.cc

Line 336, Patchset 5 (Latest): GeolocationPreciseLocationAttributeCamelCaseDoesNotChangeText) {

Mason Freed . resolved

Nice, thanks.

Open in Gerrit

Related details

Attention is currently required from:

Andy Paicu

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Andy Paicu (Gerrit)

unread,

Oct 16, 2025, 8:33:00 AM (3 days ago) Oct 16

to Mason Freed, Thomas Nguyen, Ravjit Uppal, Chromium LUCI CQ, AI Code Reviewer, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Mason Freed and Ravjit Uppal

Andy Paicu added 2 comments

Patchset-level comments

File-level comment, Patchset 5:

Ravjit Uppal . resolved

We should also update the idl file, right?

Andy Paicu

Done

File-level comment, Patchset 6 (Latest):

Andy Paicu . resolved

I've updated the IDL and the +1 was lost. Mason PTAL.

Open in Gerrit

Related details

Attention is currently required from:

Mason Freed
Ravjit Uppal

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mason Freed (Gerrit)

unread,

Oct 17, 2025, 7:24:05 PM (2 days ago) Oct 17

to Andy Paicu, AyeAye, Thomas Nguyen, Ravjit Uppal, Chromium LUCI CQ, AI Code Reviewer, chromium...@chromium.org, jmedle...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Andy Paicu and Ravjit Uppal

Mason Freed voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Andy Paicu
Ravjit Uppal

Submit Requirements:

Code-Coverage

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[PEPC] Implement the accuracymode attribute [chromium/src : main]

Andy Paicu (Gerrit)

Andy Paicu voted and added 2 comments

Votes added by Andy Paicu

2 comments

Related details

Andy Paicu (Gerrit)

Andy Paicu added 1 comment

Related details

Mason Freed (Gerrit)

Mason Freed added 5 comments

Related details

Andy Paicu (Gerrit)

Andy Paicu added 5 comments

Related details

Ravjit Uppal (Gerrit)

Ravjit Uppal added 1 comment

Related details

Mason Freed (Gerrit)

Mason Freed voted and added 3 comments

Votes added by Mason Freed

3 comments

Related details

Andy Paicu (Gerrit)

Andy Paicu added 2 comments

Related details

Mason Freed (Gerrit)

Mason Freed voted Code-Review+1

Related details