Christine Ying (Gerrit)

unread,

Nov 4, 2025, 2:38:57 PM (3 days ago) Nov 4

to Dave Tapuska, Kevin Graney, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from Dave Tapuska, David Bokan and Kevin Graney

Christine Ying voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Dave Tapuska
David Bokan
Kevin Graney

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

Dave Tapuska (Gerrit)

unread,

Nov 4, 2025, 2:48:56 PM (3 days ago) Nov 4

to Christine Ying, Erik Chen, Kevin Graney, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from Christine Ying, David Bokan, Erik Chen and Kevin Graney

Dave Tapuska added 1 comment

File chrome/browser/actor/actor_task.h

Line 112, Patchset 9 (Latest): void Stop(State final_state);

Dave Tapuska . unresolved

Seems a little weird that we are using State here. Why can't we have an enum of Stopped reasons? We might not necessarily want to each stopped reason into a sepecific state no?

Open in Gerrit

Related details

Attention is currently required from:

Christine Ying
David Bokan
Erik Chen
Kevin Graney

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

Christine Ying (Gerrit)

unread,

Nov 4, 2025, 2:59:44 PM (3 days ago) Nov 4

to Erik Chen, Dave Tapuska, Kevin Graney, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from Dave Tapuska, David Bokan, Erik Chen and Kevin Graney

Christine Ying voted and added 1 comment

Votes added by Christine Ying

Commit-Queue

+1

1 comment

File chrome/browser/actor/actor_task.h

Line 112, Patchset 9: void Stop(State final_state);

Dave Tapuska . unresolved

Seems a little weird that we are using State here. Why can't we have an enum of Stopped reasons? We might not necessarily want to each stopped reason into a sepecific state no?

Christine Ying

I figured we could just re-use the states from the ActorTask::State enum since they're already being translated from glic::mojom::ActorTaskStopReasons in glic_actor_task_manager.

I'm fine with adding an intermediary stop reasons enum but i think eventually we will have to translate these back to ActorTask::State since that's whats getting passed via the event dispatcher to chrome ui atm.

Open in Gerrit

Related details

Attention is currently required from:

Dave Tapuska

satisfied_requirement

unsatisfied_requirement

open

diffy

Dave Tapuska (Gerrit)

unread,

Nov 4, 2025, 3:07:07 PM (3 days ago) Nov 4

to Christine Ying, Erik Chen, Kevin Graney, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from Christine Ying, David Bokan, Erik Chen and Kevin Graney

Dave Tapuska added 2 comments

Commit Message

Line 12, Patchset 9:Will not merge into 143

Dave Tapuska . unresolved

This should likely go into M143, as we do have to write some additional histograms based on this.

File chrome/browser/actor/actor_task.h

Line 112, Patchset 9: void Stop(State final_state);

Dave Tapuska . unresolved

Seems a little weird that we are using State here. Why can't we have an enum of Stopped reasons? We might not necessarily want to each stopped reason into a sepecific state no?

Christine Ying

I figured we could just re-use the states from the ActorTask::State enum since they're already being translated from glic::mojom::ActorTaskStopReasons in glic_actor_task_manager.
I'm fine with adding an intermediary stop reasons enum but i think eventually we will have to translate these back to ActorTask::State since that's whats getting passed via the event dispatcher to chrome ui atm.

Dave Tapuska

I think kFailed makes sense as a state. But I think we should have an intermediate enum that matches the glic API for identifing the stopped reason. If we want to further clarify it we can always store the stopped reason on ActorTask and it could still be in the kFailed state.

Open in Gerrit

Related details

Attention is currently required from:

Christine Ying

David Bokan
Erik Chen
Kevin Graney

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

Christine Ying (Gerrit)

unread,

Nov 4, 2025, 3:53:57 PM (3 days ago) Nov 4

to Erik Chen, Dave Tapuska, Kevin Graney, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from Dave Tapuska, David Bokan, Erik Chen and Kevin Graney

Christine Ying added 2 comments

Commit Message

Line 12, Patchset 9:Will not merge into 143

Dave Tapuska . resolved

This should likely go into M143, as we do have to write some additional histograms based on this.

Christine Ying

Acknowledged

File chrome/browser/actor/actor_task.h

Line 112, Patchset 9: void Stop(State final_state);

Dave Tapuska . resolved

Seems a little weird that we are using State here. Why can't we have an enum of Stopped reasons? We might not necessarily want to each stopped reason into a sepecific state no?

Christine Ying

I figured we could just re-use the states from the ActorTask::State enum since they're already being translated from glic::mojom::ActorTaskStopReasons in glic_actor_task_manager.
I'm fine with adding an intermediary stop reasons enum but i think eventually we will have to translate these back to ActorTask::State since that's whats getting passed via the event dispatcher to chrome ui atm.

Dave Tapuska

I think kFailed makes sense as a state. But I think we should have an intermediate enum that matches the glic API for identifing the stopped reason. If we want to further clarify it we can always store the stopped reason on ActorTask and it could still be in the kFailed state.

Christine Ying

Ack added ptal

Open in Gerrit

Related details

Attention is currently required from:

Dave Tapuska

David Bokan
Erik Chen
Kevin Graney

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

Erik Chen (Gerrit)

unread,

Nov 4, 2025, 4:35:18 PM (3 days ago) Nov 4

to Christine Ying, Dave Tapuska, Kevin Graney, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from Christine Ying, Dave Tapuska, David Bokan and Kevin Graney

Erik Chen voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Christine Ying
Dave Tapuska
David Bokan
Kevin Graney

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

Dave Tapuska (Gerrit)

unread,

Nov 4, 2025, 4:43:55 PM (3 days ago) Nov 4

to Christine Ying, Erik Chen, Kevin Graney, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from Christine Ying, David Bokan and Kevin Graney

Dave Tapuska voted and added 1 comment

Votes added by Dave Tapuska

Code-Review

+1

1 comment

Commit Message

Line 12, Patchset 13:Bug: 453997100

Dave Tapuska . unresolved

Add b:453724281

Open in Gerrit

Related details

Attention is currently required from:

Christine Ying
David Bokan
Kevin Graney

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

Christine Ying (Gerrit)

unread,

Nov 4, 2025, 4:51:52 PM (3 days ago) Nov 4

to Dave Tapuska, Erik Chen, Kevin Graney, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from David Bokan and Kevin Graney

Christine Ying added 1 comment

Commit Message

Line 12, Patchset 13:Bug: 453997100

Dave Tapuska . resolved

Add b:453724281

Christine Ying

Done

Open in Gerrit

Related details

Attention is currently required from:

David Bokan
Kevin Graney

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

Kevin Graney (Gerrit)

unread,

Nov 4, 2025, 8:39:24 PM (2 days ago) Nov 4

to Christine Ying, Dave Tapuska, Erik Chen, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from Christine Ying and David Bokan

Kevin Graney voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Christine Ying
David Bokan

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

Christine Ying (Gerrit)

unread,

Nov 5, 2025, 9:51:48 AM (2 days ago) Nov 5

to Kevin Graney, Dave Tapuska, Erik Chen, David Bokan, Chromium LUCI CQ, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Attention needed from David Bokan

Christine Ying voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

David Bokan

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

Chromium LUCI CQ (Gerrit)

unread,

Nov 5, 2025, 10:51:21 AM (2 days ago) Nov 5

to Christine Ying, Kevin Graney, Dave Tapuska, Erik Chen, David Bokan, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, asvitkine...@chromium.org, aashna...@google.com, boujan...@google.com, chromium-a...@chromium.org, chrstn...@google.com, dewitt...@chromium.org, extension...@chromium.org, kenok...@google.com, kmg+...@google.com, mfoltz+wa...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

14 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: chrome/browser/ui/tabs/glic_actor_task_icon_manager_unittest.cc
Insertions: 12, Deletions: 4.

The diff is too large to show. Please review the diff.
```

Change information

Commit message:

Pipe Failed state to Actor Task

This will be rebase and submitted after
https://chromium-review.googlesource.com/c/chromium/src/+/7113019

Bug: 453997100, 453724281

Change-Id: I511519a0c1f9760e309ba6e46d899f40c42f4bbb

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7118220

Reviewed-by: Dave Tapuska <dtap...@chromium.org>

Reviewed-by: Kevin Graney <k...@google.com>

Reviewed-by: Erik Chen <erik...@chromium.org>

Commit-Queue: Christine Ying <chr...@google.com>

Cr-Commit-Position: refs/heads/main@{#1540641}

Files:

M chrome/browser/actor/actor_keyed_service.cc
M chrome/browser/actor/actor_keyed_service.h
M chrome/browser/actor/actor_keyed_service_browsertest.cc
M chrome/browser/actor/actor_keyed_service_unittest.cc
M chrome/browser/actor/actor_task.cc
M chrome/browser/actor/actor_task.h
M chrome/browser/actor/actor_task_delegate.h
M chrome/browser/actor/execution_engine_unittest.cc
M chrome/browser/actor/tools/tool_agnostic_browsertest.cc
M chrome/browser/actor/ui/actor_ui_state_manager.cc
M chrome/browser/actor/ui/actor_ui_state_manager_unittest.cc
M chrome/browser/actor/ui/handoff_button_controller_interactive_uitest.cc
M chrome/browser/extensions/api/experimental_actor/experimental_actor_api.cc
M chrome/browser/glic/actor/glic_actor_task_manager.cc
M chrome/browser/glic/actor/glic_actor_task_manager.h
M chrome/browser/glic/browser_ui/glic_border_view_interactive_uitest.cc
M chrome/browser/glic/host/glic_page_handler.cc
M chrome/browser/ui/tabs/glic_actor_task_icon_manager_unittest.cc
M tools/metrics/histograms/metadata/actor/histograms.xml

Change size: M

Delta: 19 files changed, 137 insertions(+), 94 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Dave Tapuska, +1 by Kevin Graney, +1 by Erik Chen

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Pipe Failed state to Actor Task [chromium/src : main]

Christine Ying (Gerrit)

Christine Ying voted Commit-Queue+1

Related details

Dave Tapuska (Gerrit)

Dave Tapuska added 1 comment

Related details

Christine Ying (Gerrit)

Christine Ying voted and added 1 comment

Votes added by Christine Ying

1 comment

Related details

Dave Tapuska (Gerrit)

Dave Tapuska added 2 comments

Related details

Christine Ying (Gerrit)

Christine Ying added 2 comments

Related details

Erik Chen (Gerrit)

Erik Chen voted Code-Review+1

Related details

Dave Tapuska (Gerrit)

Dave Tapuska voted and added 1 comment

Votes added by Dave Tapuska

1 comment

Related details

Christine Ying (Gerrit)

Christine Ying added 1 comment

Related details

Kevin Graney (Gerrit)

Kevin Graney voted Code-Review+1

Related details

Christine Ying (Gerrit)

Christine Ying voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information