Add Shipment Entity to Autofill [chromium/src : main]

0 views

Skip to first unread message

Victor Shu (Gerrit)

unread,

Feb 23, 2026, 5:18:57 PM (23 hours ago) Feb 23

to Teo Lamort de Gail, Florian Leimgruber, Zeyad El-arabaty, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, marq+...@chromium.org, ios-revie...@chromium.org, tmartino+tran...@chromium.org, ios-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, asvitkine...@chromium.org, vinnypersky+...@google.com, chromium-a...@chromium.org, siyua+aut...@chromium.org, extension...@chromium.org, armalhotra+a...@google.com, osaul+aut...@google.com, shgar+aut...@google.com, siashah+au...@chromium.org, moqati-team+chr...@google.com, browser-comp...@chromium.org

Attention needed from Florian Leimgruber

Victor Shu added 5 comments

File chrome/browser/ui/android/autofill/autofill_ai_save_update_entity_flow_manager.cc

Line 72, Patchset 16:

    case EntityTypeName::kOrder:
      NOTREACHED() << "Entity is read only and doesn't support save prompts.";
    case EntityTypeName::kShipment:
      NOTREACHED() << "Entity is read only and doesn't support save prompts.";

Florian Leimgruber . unresolved

Is this read only status decided or should this rather be a NOTIMPLEMENTED()?
Same in some of the other files.

Victor Shu

I followed the FlightReservation entity implementation and Order and Shipment seem similar in nature to be read only.

File components/autofill/core/browser/data_model/autofill_ai/entity_type.cc

Line 66, Patchset 16: // Shipments are read-only and do not use attribute strings.

Florian Leimgruber . unresolved

Why do we define strings in autofill_strings.grdp then?

Victor Shu

removed from autofill_strings.grdp

File components/autofill/core/browser/field_types.h

Line 571, Patchset 16: SHIPMENT_ASSOCIATED_ORDER_ID = 215,

Florian Leimgruber . unresolved

What is the associated order ID? Is this the value of the ORDER_ID or is it the guid() of the associated order entity?

Victor Shu

should match with the ORDER_ID.

File components/autofill/core/browser/permissions/autofill_ai/autofill_ai_permission_utils.cc

Line 148, Patchset 16: case EntityTypeName::kShipment:

Florian Leimgruber . unresolved

Has it been decided which preferences will control order and shipment?

Victor Shu

Not sure about this

File components/autofill/core/browser/suggestions/autofill_ai/autofill_ai_suggestion_generator.cc

Line 376, Patchset 16: return Suggestion::Icon::kNoIcon;

Florian Leimgruber . resolved

Should this rather be a NOTIMPLEMENTED()?

Victor Shu

Done

Open in Gerrit

Related details

Attention is currently required from:

Florian Leimgruber

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

Florian Leimgruber (Gerrit)

unread,

10:09 AM (6 hours ago) 10:09 AM

to Victor Shu, Teo Lamort de Gail, Zeyad El-arabaty, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, marq+...@chromium.org, ios-revie...@chromium.org, tmartino+tran...@chromium.org, ios-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org, asvitkine...@chromium.org, vinnypersky+...@google.com, chromium-a...@chromium.org, siyua+aut...@chromium.org, extension...@chromium.org, armalhotra+a...@google.com, osaul+aut...@google.com, shgar+aut...@google.com, siashah+au...@chromium.org, moqati-team+chr...@google.com, browser-comp...@chromium.org

Attention needed from Victor Shu

Florian Leimgruber added 4 comments

File chrome/browser/ui/android/autofill/autofill_ai_save_update_entity_flow_manager.cc

Line 72, Patchset 16:

    case EntityTypeName::kOrder:
      NOTREACHED() << "Entity is read only and doesn't support save prompts.";
    case EntityTypeName::kShipment:
      NOTREACHED() << "Entity is read only and doesn't support save prompts.";

Florian Leimgruber . unresolved

Is this read only status decided or should this rather be a NOTIMPLEMENTED()?
Same in some of the other files.

Victor Shu

I followed the FlightReservation entity implementation and Order and Shipment seem similar in nature to be read only.

Florian Leimgruber

Yes, but for flight reservations, product has actually decided that the should be read-only in Chrome.

I think until we know, this should just be `NOTIMPLEMENTED()`

File components/autofill/core/browser/data_model/autofill_ai/entity_type.cc

Line 66, Patchset 16: // Shipments are read-only and do not use attribute strings.

Florian Leimgruber . unresolved

Why do we define strings in autofill_strings.grdp then?

Victor Shu

removed from autofill_strings.grdp

Florian Leimgruber

We are still defining some strings? Is this necessary/recommended (I genuinely don't know. Perhaps the entitiy names are necessary to display them in settings somehow)?

File components/autofill/core/browser/field_types.h

Line 571, Patchset 16: SHIPMENT_ASSOCIATED_ORDER_ID = 215,

Florian Leimgruber . unresolved

What is the associated order ID? Is this the value of the ORDER_ID or is it the guid() of the associated order entity?

Victor Shu

should match with the ORDER_ID.

Florian Leimgruber

Please add a comment then describing this relation.

File components/autofill/core/browser/permissions/autofill_ai/autofill_ai_permission_utils.cc

Line 148, Patchset 16: case EntityTypeName::kShipment:

Florian Leimgruber . resolved

Has it been decided which preferences will control order and shipment?

Victor Shu

Not sure about this

Florian Leimgruber

Ack. Defaulting them to false then SGTM.

Open in Gerrit

Related details

Attention is currently required from:

Victor Shu

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

Victor Shu (Gerrit)

unread,

1:59 PM (3 hours ago) 1:59 PM

Attention needed from Florian Leimgruber

Victor Shu added 3 comments

File chrome/browser/ui/android/autofill/autofill_ai_save_update_entity_flow_manager.cc

Line 72, Patchset 16:

    case EntityTypeName::kOrder:
      NOTREACHED() << "Entity is read only and doesn't support save prompts.";
    case EntityTypeName::kShipment:
      NOTREACHED() << "Entity is read only and doesn't support save prompts.";

Florian Leimgruber . unresolved

Is this read only status decided or should this rather be a NOTIMPLEMENTED()?
Same in some of the other files.

Victor Shu

I followed the FlightReservation entity implementation and Order and Shipment seem similar in nature to be read only.

Florian Leimgruber

Yes, but for flight reservations, product has actually decided that the should be read-only in Chrome.
I think until we know, this should just be `NOTIMPLEMENTED()`

Victor Shu

looks like product decided both Order and Shipment are read only.

File components/autofill/core/browser/data_model/autofill_ai/entity_type.cc

Line 66, Patchset 16: // Shipments are read-only and do not use attribute strings.

Florian Leimgruber . unresolved

Why do we define strings in autofill_strings.grdp then?

Victor Shu

removed from autofill_strings.grdp

Florian Leimgruber

We are still defining some strings? Is this necessary/recommended (I genuinely don't know. Perhaps the entitiy names are necessary to display them in settings somehow)?

Victor Shu

Yes, i think it's recommended. Also, following the pattern from other entity cls.

File components/autofill/core/browser/field_types.h

Line 571, Patchset 16: SHIPMENT_ASSOCIATED_ORDER_ID = 215,

Florian Leimgruber . resolved

What is the associated order ID? Is this the value of the ORDER_ID or is it the guid() of the associated order entity?

Victor Shu

should match with the ORDER_ID.

Florian Leimgruber

Please add a comment then describing this relation.

Victor Shu

Done

Open in Gerrit

Related details

Attention is currently required from:

Florian Leimgruber

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

Reply all

Reply to author

Forward

0 new messages