Evan Liu (Gerrit)

unread,

May 12, 2026, 8:29:49 PMMay 12

to (Julie)Jeongeun Kim, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, Tommy Steimel, Chromium LUCI CQ, chromium...@chromium.org, mar...@chromium.org, francisjp...@google.com, abigailbk...@google.com, grt+...@chromium.org, yuzo+...@chromium.org, jz...@chromium.org, nektar...@chromium.org, accessibility-a...@google.com, cblume...@chromium.org, devtools...@chromium.org, dtseng...@chromium.org, oshima...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, net-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org

Attention needed from Tommy Steimel

Evan Liu added 1 comment

Patchset-level comments

File-level comment, Patchset 16 (Latest):

Evan Liu . resolved

PTAL again ty!

Open in Gerrit

Related details

Attention is currently required from:

Tommy Steimel

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

Greg Thompson (Gerrit)

unread,

May 13, 2026, 5:46:58 AMMay 13

to Evan Liu, (Julie)Jeongeun Kim, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, Tommy Steimel, Chromium LUCI CQ, chromium...@chromium.org, mar...@chromium.org, francisjp...@google.com, abigailbk...@google.com, grt+...@chromium.org, yuzo+...@chromium.org, jz...@chromium.org, nektar...@chromium.org, accessibility-a...@google.com, cblume...@chromium.org, devtools...@chromium.org, dtseng...@chromium.org, oshima...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, net-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org

Attention needed from Evan Liu and Tommy Steimel

Greg Thompson added 6 comments

Patchset-level comments

File-level comment, Patchset 16 (Latest):

Greg Thompson . resolved

drive-by suggestion

File content/browser/speech/on_device_speech_recognition_engine_impl.h

Line 125, Patchset 16 (Latest): std::unique_ptr<Core, base::OnTaskRunnerDeleter> core_;

Greg Thompson . unresolved

could you use `base::SequenceBound<Core> core_;` here?

Line 79, Patchset 16 (Latest): void TryCreateSession();

Greg Thompson . unresolved

private method?

File content/browser/speech/on_device_speech_recognition_engine_impl.cc

Line 49, Patchset 16 (Latest): core_(new Core(), base::OnTaskRunnerDeleter(ui_task_runner_)) {

Greg Thompson . unresolved

```
      core_(content::GetUIThreadTaskRunner({}),
            base::BindPostTaskToCurrentDefault(base::BindOnce(
                &OnDeviceSpeechRecognitionEngine::OnAsrStreamCreated,
                weak_factory_.GetWeakPtr()))) {
```

Line 55, Patchset 16 (Latest):

  // Safe because core_ is deleted via DeleteSoon on the same task runner,
  // guaranteeing this task will run before the Deleter task.
  ui_task_runner_->PostTask(
      FROM_HERE,
      base::BindOnce(&Core::CreateModelClient, base::Unretained(core_.get()),
                     config_.initial_context.global_id, config_.quality));

Greg Thompson . unresolved

```
  core_.AsyncCall(&Core::CreateModelClient)
      .WithArgs(config_.initial_context.global_id, config_.quality);
```

Line 91, Patchset 16 (Latest):

  // Safe because core_ is deleted via DeleteSoon on the same task runner,
  // guaranteeing this task will run before the Deleter task.
  ui_task_runner_->PostTask(
      FROM_HERE,
      base::BindOnce(&Core::SetAudioParameters, base::Unretained(core_.get()),
                     audio_parameters.sample_rate()));

Greg Thompson . unresolved

```
  core_.AsyncCall(&Core::SetAudioParameters)
      .WithArgs(audio_parameters.sample_rate());
```

Open in Gerrit

Related details

Attention is currently required from:

Evan Liu
Tommy Steimel

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

Evan Liu (Gerrit)

unread,

May 13, 2026, 2:50:50 PMMay 13

to Greg Thompson, (Julie)Jeongeun Kim, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, Tommy Steimel, Chromium LUCI CQ, chromium...@chromium.org, mar...@chromium.org, francisjp...@google.com, abigailbk...@google.com, grt+...@chromium.org, yuzo+...@chromium.org, jz...@chromium.org, nektar...@chromium.org, accessibility-a...@google.com, cblume...@chromium.org, devtools...@chromium.org, dtseng...@chromium.org, oshima...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, net-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org

Attention needed from Greg Thompson and Tommy Steimel

Evan Liu voted and added 5 comments

Votes added by Evan Liu

Commit-Queue

+1

5 comments

File content/browser/speech/on_device_speech_recognition_engine_impl.h

Line 125, Patchset 16: std::unique_ptr<Core, base::OnTaskRunnerDeleter> core_;

Greg Thompson . resolved

could you use `base::SequenceBound<Core> core_;` here?

Evan Liu

Done

Line 79, Patchset 16: void TryCreateSession();

Greg Thompson . resolved

private method?

Evan Liu

Done

File content/browser/speech/on_device_speech_recognition_engine_impl.cc

Line 49, Patchset 16: core_(new Core(), base::OnTaskRunnerDeleter(ui_task_runner_)) {

Greg Thompson . resolved

```
      core_(content::GetUIThreadTaskRunner({}),
            base::BindPostTaskToCurrentDefault(base::BindOnce(
                &OnDeviceSpeechRecognitionEngine::OnAsrStreamCreated,
                weak_factory_.GetWeakPtr()))) {
```

Evan Liu

Done

Line 55, Patchset 16: // Safe because core_ is deleted via DeleteSoon on the same task runner,


  // guaranteeing this task will run before the Deleter task.
  ui_task_runner_->PostTask(
      FROM_HERE,
      base::BindOnce(&Core::CreateModelClient, base::Unretained(core_.get()),
                     config_.initial_context.global_id, config_.quality));

Greg Thompson . resolved

```
  core_.AsyncCall(&Core::CreateModelClient)
      .WithArgs(config_.initial_context.global_id, config_.quality);
```

Evan Liu

Done

Line 91, Patchset 16: // Safe because core_ is deleted via DeleteSoon on the same task runner,


  // guaranteeing this task will run before the Deleter task.
  ui_task_runner_->PostTask(
      FROM_HERE,
      base::BindOnce(&Core::SetAudioParameters, base::Unretained(core_.get()),
                     audio_parameters.sample_rate()));

Greg Thompson . resolved

```
  core_.AsyncCall(&Core::SetAudioParameters)
      .WithArgs(audio_parameters.sample_rate());
```

Evan Liu

Done

Open in Gerrit

Related details

Attention is currently required from:

Greg Thompson
Tommy Steimel

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

Greg Thompson (Gerrit)

unread,

May 14, 2026, 8:33:34 AMMay 14

to Evan Liu, (Julie)Jeongeun Kim, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, Tommy Steimel, Chromium LUCI CQ, chromium...@chromium.org, mar...@chromium.org, francisjp...@google.com, abigailbk...@google.com, grt+...@chromium.org, yuzo+...@chromium.org, jz...@chromium.org, nektar...@chromium.org, accessibility-a...@google.com, cblume...@chromium.org, devtools...@chromium.org, dtseng...@chromium.org, oshima...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, net-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org

Attention needed from Evan Liu and Tommy Steimel

Greg Thompson voted and added 3 comments

Votes added by Greg Thompson

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 18 (Latest):

Greg Thompson . resolved

nice. lgtm w/ two tweaks. thanks.

File content/browser/speech/on_device_speech_recognition_engine_impl.cc

Line 44, Patchset 18 (Latest): core_ = base::SequenceBound<Core>(

Greg Thompson . unresolved

move into initializer list; i.e., `core_(...)` just after `config_` above

Line 212, Patchset 18 (Latest): session_created_ = true;

Greg Thompson . unresolved

already done on line 189

Open in Gerrit

Related details

Attention is currently required from:

Evan Liu
Tommy Steimel

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

Tommy Steimel (Gerrit)

unread,

May 14, 2026, 12:43:38 PMMay 14

to Evan Liu, Greg Thompson, (Julie)Jeongeun Kim, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mar...@chromium.org, francisjp...@google.com, abigailbk...@google.com, grt+...@chromium.org, yuzo+...@chromium.org, jz...@chromium.org, nektar...@chromium.org, accessibility-a...@google.com, cblume...@chromium.org, devtools...@chromium.org, dtseng...@chromium.org, oshima...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, net-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org

Attention needed from Evan Liu

Tommy Steimel voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Evan Liu

satisfied_requirement

unsatisfied_requirement

open

diffy

Evan Liu (Gerrit)

unread,

May 14, 2026, 1:56:59 PMMay 14

to Tommy Steimel, Greg Thompson, (Julie)Jeongeun Kim, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mar...@chromium.org, francisjp...@google.com, abigailbk...@google.com, grt+...@chromium.org, yuzo+...@chromium.org, jz...@chromium.org, nektar...@chromium.org, accessibility-a...@google.com, cblume...@chromium.org, devtools...@chromium.org, dtseng...@chromium.org, oshima...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, net-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org

Evan Liu voted and added 2 comments

Votes added by Evan Liu

Commit-Queue

+2

2 comments

File content/browser/speech/on_device_speech_recognition_engine_impl.cc

Line 44, Patchset 18: core_ = base::SequenceBound<Core>(

Greg Thompson . resolved

move into initializer list; i.e., `core_(...)` just after `config_` above

Evan Liu

Reverting this change since `weak_factory_` isn't initialized in the initializer list.

Line 212, Patchset 18: session_created_ = true;

Greg Thompson . resolved

already done on line 189

Evan Liu

Fix applied.

Open in Gerrit

Related details

Attention set is empty

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

Evan Liu (Gerrit)

unread,

May 14, 2026, 5:02:19 PMMay 14

to Tommy Steimel, Greg Thompson, (Julie)Jeongeun Kim, Jerome Jiang, Mirko Bonadei, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, mar...@chromium.org, francisjp...@google.com, abigailbk...@google.com, grt+...@chromium.org, yuzo+...@chromium.org, jz...@chromium.org, nektar...@chromium.org, accessibility-a...@google.com, cblume...@chromium.org, devtools...@chromium.org, dtseng...@chromium.org, oshima...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, net-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org

Evan Liu voted Commit-Queue+2

Commit-Queue

+2

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

May 14, 2026, 7:22:18 PMMay 14

to Evan Liu, Tommy Steimel, Greg Thompson, (Julie)Jeongeun Kim, Jerome Jiang, Akihiro Ota, Mirko Bonadei, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, mar...@chromium.org, francisjp...@google.com, abigailbk...@google.com, grt+...@chromium.org, yuzo+...@chromium.org, jz...@chromium.org, nektar...@chromium.org, accessibility-a...@google.com, cblume...@chromium.org, devtools...@chromium.org, dtseng...@chromium.org, oshima...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, browser-comp...@chromium.org, feature-me...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, net-r...@chromium.org, chrome-intelligence-te...@google.com, chrome-intell...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

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

```
The name of the file: content/browser/speech/on_device_speech_recognition_engine_impl.cc
Insertions: 0, Deletions: 2.

@@ -208,8 +208,6 @@
     std::move(on_stream_created_callback_)
         .Run(std::move(asr_stream), std::move(asr_stream_responder));
   }
-
-  session_created_ = true;
 }
 
 void OnDeviceSpeechRecognitionEngine::OnAsrStreamCreated(
```

Change information

Commit message:

speech: Fix threading sequence checker in OnDeviceSpeechRecognitionEngine

Encapsulates all UI-thread-specific state into a dedicated Core class to
ensure state mutations only happen on the UI thread. Cross-thread
communication is handled using base::BindPostTask to seamlessly hop
between the IO and UI threads, fixing a sequence checker crash during
model initialization.

Change-Id: I9d662b4dd5fdedb997cdcf032b67c8fadb4744d9

Fixed: 511710464

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

Reviewed-by: Greg Thompson <g...@chromium.org>

Reviewed-by: Tommy Steimel <ste...@chromium.org>

Commit-Queue: Evan Liu <ev...@google.com>

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

Files:

M content/browser/speech/DEPS
M content/browser/speech/on_device_speech_recognition_engine_impl.cc
M content/browser/speech/on_device_speech_recognition_engine_impl.h
M content/browser/speech/on_device_speech_recognition_engine_unittest.cc
M content/test/BUILD.gn

Change size: L

Delta: 5 files changed, 184 insertions(+), 79 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Greg Thompson, +1 by Tommy Steimel

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

speech: Fix threading sequence checker in OnDeviceSpeechRecognitionEngine [chromium/src : main]

Evan Liu (Gerrit)

Evan Liu added 1 comment

Related details

Greg Thompson (Gerrit)

Greg Thompson added 6 comments

Related details

Evan Liu (Gerrit)

Evan Liu voted and added 5 comments

Votes added by Evan Liu

5 comments

Related details

Greg Thompson (Gerrit)

Greg Thompson voted and added 3 comments

Votes added by Greg Thompson

3 comments

Related details

Tommy Steimel (Gerrit)

Tommy Steimel voted Code-Review+1

Related details

Evan Liu (Gerrit)

Evan Liu voted and added 2 comments

Votes added by Evan Liu

2 comments

Related details

Evan Liu (Gerrit)

Evan Liu voted Commit-Queue+2

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information