[Actor] Implement ActorTaskTimeoutManager for timeout enforcement (2/2) [chromium/src : main]

[Bhuvana Betini (Gerrit)]

Attention needed from Bhuvana Betini and Ritika Gupta

Message from Bhuvana Betini

Set Ready For Review

Open in Gerrit

Related details

Attention is currently required from:

• Bhuvana Betini
• Ritika Gupta

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

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I54fcd5546dac4382cb6c7b5c7ac1559b20db148b

Gerrit-Change-Number: 7910101

Gerrit-PatchSet: 17

Gerrit-Owner: Bhuvana Betini <bbe...@google.com>

Gerrit-Reviewer: Bhuvana Betini <bbe...@google.com>

Gerrit-Reviewer: Ritika Gupta <riti...@google.com>

Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>

Gerrit-CC: AJITH KUMAR V <aji...@chromium.org>

Gerrit-CC: Akihiro Ota <akihi...@chromium.org>

Gerrit-CC: Ale Bzk <ales...@chromium.org>

Gerrit-CC: Andrew Rayskiy <green...@google.com>

Gerrit-CC: Anurag Simgeker <anurags...@google.com>

Gerrit-CC: AyeAye Python Dispatcher <android-build-ayeay...@system.gserviceaccount.com>

Gerrit-CC: Charles Hager <clh...@google.com>

Gerrit-CC: Christian Biesinger <cbies...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Daniel Cheng <dch...@chromium.org>

Gerrit-CC: Dirk Schulze <dsch...@chromium.org>

Gerrit-CC: Enterprise Policy Reviews <enterprise-p...@google.com>

Gerrit-CC: Fredrik Söderquist <f...@opera.com>

Gerrit-CC: Hans Wennborg <ha...@chromium.org>

Gerrit-CC: Heron Yang <hero...@google.com>

Gerrit-CC: Hirokazu Honda <hi...@chromium.org>

Gerrit-CC: Hiroki Nakagawa <nhi...@chromium.org>

Gerrit-CC: Hongchan Choi <hong...@chromium.org>

Gerrit-CC: Hu, Ningxin <ningx...@intel.com>

Gerrit-CC: Ian Vollick <vol...@chromium.org>

Gerrit-CC: Ilya Biryukov <ibir...@google.com>

Gerrit-CC: James Maclean <wjma...@chromium.org>

Gerrit-CC: Javier Fernandez <jfern...@igalia.com>

Gerrit-CC: Jerome Jiang <ji...@chromium.org>

Gerrit-CC: Jiewei Qian <q...@chromium.org>

Gerrit-CC: Josh Karlin <jka...@chromium.org>

Gerrit-CC: Kenneth R Christiansen <kenneth.r.c...@intel.com>

Gerrit-CC: Kenneth Rohde Christiansen <kenneth.ch...@gmail.com>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>

Gerrit-CC: Kevin McNee <mc...@chromium.org>

Gerrit-CC: Khushal Sagar <khusha...@chromium.org>

Gerrit-CC: Lei Zhang <the...@chromium.org>

Gerrit-CC: Lin, Wanming <wanmi...@intel.com>

Gerrit-CC: Linyu He <lin...@google.com>

Gerrit-CC: Luna Lu <loon...@chromium.org>

Gerrit-CC: Mandy, Arnaud <arnaud...@intel.com>

Gerrit-CC: Mangesh Ghiware <mghi...@google.com>

Gerrit-CC: Mark Schillaci <mschi...@google.com>

Gerrit-CC: Menard, Alexis <alexis...@intel.com>

Gerrit-CC: Michael Moss <mm...@chromium.org>

Gerrit-CC: Michael Wilson <mjwi...@chromium.org>

Gerrit-CC: Mirko Bonadei <mbon...@chromium.org>

Gerrit-CC: Moe Adel <ad...@google.com>

Gerrit-CC: Morten Stenshorne <mste...@chromium.org>

Gerrit-CC: Nate Chapin <jap...@chromium.org>

Gerrit-CC: Neil Coronado <ne...@google.com>

Gerrit-CC: Nektarios Paisios <nek...@chromium.org>

Gerrit-CC: Nico Weber <tha...@chromium.org>

Gerrit-CC: Nikhil Nayunigari <nikh...@google.com>

Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>

Gerrit-CC: Per Åhgren <pe...@chromium.org>

Gerrit-CC: Peter Beverloo <pe...@chromium.org>

Gerrit-CC: Peter Collingbourne <p...@chromium.org>

Gerrit-CC: Peter Williamson <pet...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

Gerrit-CC: Reid Kleckner <r...@chromium.org>

Gerrit-CC: Rijubrata Bhaumik <rijubrat...@intel.com>

Gerrit-CC: Ryan Sultanem <rs...@google.com>

Gerrit-CC: Sadrul Chowdhury <sad...@chromium.org>

Gerrit-CC: Sam Zackrisson <sa...@chromium.org>

Gerrit-CC: Sean Maher <sp...@chromium.org>

Gerrit-CC: Shu Yang <shu...@google.com>

Gerrit-CC: Simon Hangl <sim...@google.com>

Gerrit-CC: Sinan Sahin <sinan...@google.com>

Gerrit-CC: Stephen Chenney <sche...@chromium.org>

Gerrit-CC: Thiabaud Engelbrecht <thia...@google.com>

Gerrit-CC: Thorsten Kober <thor...@google.com>

Gerrit-CC: Wang, Wei4 <wei4...@intel.com>

Gerrit-CC: Yao Xiao <yao...@chromium.org>

Gerrit-CC: Zewen Li <zew...@google.com>

Gerrit-CC: Zhe Su <su...@chromium.org>

Gerrit-CC: Zijie He <zij...@google.com>

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: srirama chandra sekhar <srir...@samsung.com>

Gerrit-Attention: Bhuvana Betini <bbe...@google.com>

Gerrit-Attention: Ritika Gupta <riti...@google.com>

Gerrit-Comment-Date: Mon, 15 Jun 2026 17:28:03 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: No

[Ritika Gupta (Gerrit)]

Attention needed from Bhuvana Betini

Ritika Gupta added 6 comments

Patchset-level comments

File-level comment, Patchset 17 (Latest):

Ritika Gupta . resolved

nits

File chrome/browser/actor/android/java/src/org/chromium/chrome/browser/actor/ActorForegroundServiceManager.java

Line 163, Patchset 17 (Latest): if (mNotificationService == null || mKeyedService == null) return;

Ritika Gupta . unresolved

check not needed.

or move it completely to refreshTaskUI

Line 230, Patchset 17 (Latest): mTimeoutManager != null
&& mTimeoutManager.isWarningMode(notificationId));

Ritika Gupta . unresolved

nit : could we have a helper method for this code snippet to improve readability?

File chrome/browser/actor/android/java/src/org/chromium/chrome/browser/actor/ActorTaskTimeoutManager.java

Line 46, Patchset 17 (Latest): public void onTaskStateChanged(int taskId, @ActorTaskState int newState) {
// If the task changed, reset everything.
if (taskId != mActiveTaskId) {
cancelTimer();
mIsWarningMode = false;
mActiveTaskId = taskId;
mLastState = INVALID_STATE;
}

// If the task just resumed from a warning, we MUST reset the timer.
boolean isResumingFromWarning = mIsWarningMode && ActorUtils.isRunningState(newState);

if (ActorUtils.isSameLogicalGroup(mLastState, newState) && !isResumingFromWarning) {
return;
}

cancelTimer();
mLastState = newState;

if (ActorUtils.isCompletedState(newState)) {
mActiveTaskId = INVALID_TASK_ID;
mIsWarningMode = false;
return;
}

// Resuming from warning clears the flag and starts a fresh timer.
if (isResumingFromWarning) {
mIsWarningMode = false;
startTimerForState(taskId, newState);
return;
}

if (mIsWarningMode) {
// We are already in warning mode (e.g. task was just paused due to running too long).
// Start the terminal countdown.
startTerminalTimer(taskId);
} else {
startTimerForState(taskId, newState);
}
}


Ritika Gupta . unresolved

is it hard to understand this.
So many if else. Could we refactor it better so that easy to understand.

Line 108, Patchset 17 (Latest): ActorTask task = mKeyedService.getTask(taskId);

Ritika Gupta . unresolved

Question for complete cl: is this ensured at all places that mKeyedService wouldnt be null?

Line 133, Patchset 17 (Latest): mKeyedService.stopTask(taskId, StoppedReason.CHROME_FAILURE);

Ritika Gupta . unresolved

good to add a new field saying task stopped due to time out?

And ensure we record metrics with this reason.

Open in Gerrit

Related details

Attention is currently required from:

• Bhuvana Betini

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• No-Unresolved-Comments

Gerrit-Comment-Date: Mon, 15 Jun 2026 21:35:41 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Bhuvana Betini (Gerrit)]

Attention needed from Ritika Gupta

Bhuvana Betini added 5 comments

File chrome/browser/actor/android/java/src/org/chromium/chrome/browser/actor/ActorForegroundServiceManager.java

Line 163, Patchset 17: if (mNotificationService == null || mKeyedService == null) return;

Ritika Gupta . resolved

check not needed.

or move it completely to refreshTaskUI

Bhuvana Betini

Done

Line 230, Patchset 17: mTimeoutManager != null
&& mTimeoutManager.isWarningMode(notificationId));

Ritika Gupta . resolved

nit : could we have a helper method for this code snippet to improve readability?

Bhuvana Betini

Done

File chrome/browser/actor/android/java/src/org/chromium/chrome/browser/actor/ActorTaskTimeoutManager.java

Line 46, Patchset 17: public void onTaskStateChanged(int taskId, @ActorTaskState int newState) {

Ritika Gupta . resolved

is it hard to understand this.
So many if else. Could we refactor it better so that easy to understand.

Bhuvana Betini

Done

Line 108, Patchset 17: ActorTask task = mKeyedService.getTask(taskId);

Ritika Gupta . resolved

Question for complete cl: is this ensured at all places that mKeyedService wouldnt be null?

Bhuvana Betini

Yes, we only create the timeout manager if

Line 133, Patchset 17: mKeyedService.stopTask(taskId, StoppedReason.CHROME_FAILURE);

Ritika Gupta . resolved

good to add a new field saying task stopped due to time out?

And ensure we record metrics with this reason.

Bhuvana Betini

Will address this and metrics in the next CL

Open in Gerrit

Related details

Attention is currently required from:

• Ritika Gupta

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

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I54fcd5546dac4382cb6c7b5c7ac1559b20db148b

Gerrit-Change-Number: 7910101

Gerrit-PatchSet: 19

Gerrit-Attention: Ritika Gupta <riti...@google.com>

Gerrit-Comment-Date: Mon, 15 Jun 2026 22:08:36 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Ritika Gupta <riti...@google.com>

[Takuto Ikuta (Gerrit)]

Attention needed from Bhuvana Betini and Ritika Gupta

Takuto Ikuta removed chrome-b...@google.com from this change

Deleted Reviewers:

• chrome-b...@google.com

Open in Gerrit

Related details

Attention is currently required from:

• Bhuvana Betini
• Ritika Gupta

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

Gerrit-MessageType: deleteReviewer

[Ritika Gupta (Gerrit)]

Attention needed from Bhuvana Betini

Ritika Gupta added 2 comments

File chrome/browser/actor/android/java/src/org/chromium/chrome/browser/actor/ActorNotificationService.java

Line 118, Patchset 19 (Latest): Boolean wasWarning = mWarningStates.get(taskId);

Ritika Gupta . unresolved

Discussed offline to verify if we need wasWarning?

File chrome/browser/actor/android/java/src/org/chromium/chrome/browser/actor/ActorTaskTimeoutManager.java

Line 21, Patchset 19 (Latest):public class ActorTaskTimeoutManager {

Ritika Gupta . unresolved

discussed offline on design for simplification. Key points:

• To make sure that we have one active task tracked for the whole time, if there are more than one task, we dont want to keep switching task.
• Timer and warning state should be cleaned up if running/completed states.
• Start timer if not in warning state : startTimerForState(taskId, newState);
• In triggerSoftWarning, always call startTerminalTimer(taskId);

Open in Gerrit

Related details

Attention is currently required from:

• Bhuvana Betini

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

Gerrit-MessageType: comment

Gerrit-Comment-Date: Tue, 16 Jun 2026 20:40:01 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Ritika Gupta (Gerrit)]

Attention needed from Bhuvana Betini

Ritika Gupta removed Sadrul Chowdhury from this change

Deleted Reviewers:

• Sadrul Chowdhury

Open in Gerrit

Related details

Attention is currently required from:

• Bhuvana Betini

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

Gerrit-MessageType: deleteReviewer

[Ritika Gupta (Gerrit)]

Attention needed from Bhuvana Betini

Ritika Gupta removed Zhe Su from this change

Deleted Reviewers:

• Zhe Su

[Bhuvana Betini (Gerrit)]

Bhuvana Betini abandoned this change.

View Change

Abandoned

Bhuvana Betini abandoned this change

Related details

Attention set is empty

Submit Requirements:

• Code-Coverage

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

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

Gerrit

Gerrit-MessageType: abandon