Hidehiko Abe (Gerrit)

unread,

Jul 3, 2025, 2:54:13 AMJul 3

to Muyao Xu, Xiyuan Xia, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Attention needed from Muyao Xu and Xiyuan Xia

Hidehiko Abe added 1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Hidehiko Abe . resolved

PTAL.

Muyao: main reviewer.
Xiyuan: ash_test_helper update.

Open in Gerrit

Related details

Attention is currently required from:

Muyao Xu
Xiyuan Xia

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hidehiko Abe (Gerrit)

unread,

Jul 3, 2025, 3:03:15 AMJul 3

to Kinuko Yasuda, Muyao Xu, Xiyuan Xia, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Attention needed from Kinuko Yasuda, Muyao Xu and Xiyuan Xia

Hidehiko Abe added 1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Hidehiko Abe . resolved

kinuko@ for content/ owner review.

Open in Gerrit

Related details

Attention is currently required from:

Kinuko Yasuda
Muyao Xu
Xiyuan Xia

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Xiyuan Xia (Gerrit)

unread,

Jul 7, 2025, 12:24:28 PMJul 7

to Hidehiko Abe, Code Review Nudger, Kinuko Yasuda, Muyao Xu, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Attention needed from Hidehiko Abe, Kinuko Yasuda and Muyao Xu

Xiyuan Xia added 1 comment

File chrome/browser/ui/ash/global_media_controls/media_notification_provider_impl_unittest.cc

Line 190, Patchset 6 (Latest): session_manager::SessionManager session_manager_;

Xiyuan Xia . unresolved

Why do we need this here instead of using the instance provided by `AshTestHelper` ?

Open in Gerrit

Related details

Attention is currently required from:

Hidehiko Abe
Kinuko Yasuda
Muyao Xu

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Muyao Xu (Gerrit)

unread,

Jul 7, 2025, 1:11:21 PMJul 7

to Hidehiko Abe, Code Review Nudger, Kinuko Yasuda, Xiyuan Xia, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Attention needed from Hidehiko Abe and Kinuko Yasuda

Muyao Xu voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Hidehiko Abe
Kinuko Yasuda

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review

No-Unresolved-Comments

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hidehiko Abe (Gerrit)

unread,

Jul 8, 2025, 1:27:56 AMJul 8

to Muyao Xu, Code Review Nudger, Kinuko Yasuda, Xiyuan Xia, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Attention needed from Kinuko Yasuda and Xiyuan Xia

Hidehiko Abe voted and added 2 comments

Votes added by Hidehiko Abe

Commit-Queue

+1

2 comments

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Hidehiko Abe . resolved

Thank you for review!

File chrome/browser/ui/ash/global_media_controls/media_notification_provider_impl_unittest.cc

Line 190, Patchset 6: session_manager::SessionManager session_manager_;

Xiyuan Xia . unresolved

Why do we need this here instead of using the instance provided by `AshTestHelper` ?

Hidehiko Abe

To resolve the dependency issue.

Currently, in production, the events are happening in the following order.

ProfileManager is created.
SessionManager is created.
Ash is created
Primary profile is created.
...
Ash is destroyed
ProfileManager with any loaded Profiles are destroyed
SessionManager is destroyed.

Specifically, ash depends on some data held by profiles.
Some of the BrowserContextKeyedServices depends on SessionManager.

I'd like to resolve the order of destruction later (actually I'm experimenting locally w/ some chat w/ oshima@, fyi: crrev.com/c/6698631), but it's far beyond of this CL's scope, and probably will take some time.

Ash and SessionManager is tightly coupled in AshTestHelper in terms of destruction, we cannot simply use it.

Left some comment.

Open in Gerrit

Related details

Attention is currently required from:

Kinuko Yasuda
Xiyuan Xia

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Xiyuan Xia (Gerrit)

unread,

Jul 8, 2025, 11:39:24 AMJul 8

to Hidehiko Abe, Muyao Xu, Code Review Nudger, Kinuko Yasuda, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Attention needed from Hidehiko Abe and Kinuko Yasuda

Xiyuan Xia voted and added 2 comments

Votes added by Xiyuan Xia

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Xiyuan Xia . resolved

lgtm

File chrome/browser/ui/ash/global_media_controls/media_notification_provider_impl_unittest.cc

Line 190, Patchset 6: session_manager::SessionManager session_manager_;

Xiyuan Xia . resolved

Why do we need this here instead of using the instance provided by `AshTestHelper` ?

Hidehiko Abe

To resolve the dependency issue.
Currently, in production, the events are happening in the following order.
ProfileManager is created.
SessionManager is created.
Ash is created
Primary profile is created.
...
Ash is destroyed
ProfileManager with any loaded Profiles are destroyed
SessionManager is destroyed.
Specifically, ash depends on some data held by profiles.
Some of the BrowserContextKeyedServices depends on SessionManager.
I'd like to resolve the order of destruction later (actually I'm experimenting locally w/ some chat w/ oshima@, fyi: crrev.com/c/6698631), but it's far beyond of this CL's scope, and probably will take some time.
Ash and SessionManager is tightly coupled in AshTestHelper in terms of destruction, we cannot simply use it.
Left some comment.

Xiyuan Xia

Makes sense. Thanks for the clarification.

Open in Gerrit

Related details

Attention is currently required from:

Hidehiko Abe
Kinuko Yasuda

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Kinuko Yasuda (Gerrit)

unread,

Jul 8, 2025, 11:50:38 AMJul 8

to Hidehiko Abe, Xiyuan Xia, Muyao Xu, Code Review Nudger, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Attention needed from Hidehiko Abe

Kinuko Yasuda voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Hidehiko Abe

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Hidehiko Abe (Gerrit)

unread,

Jul 8, 2025, 3:20:11 PMJul 8

to Kinuko Yasuda, Xiyuan Xia, Muyao Xu, Code Review Nudger, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Hidehiko Abe voted and added 1 comment

Votes added by Hidehiko Abe

Commit-Queue

+2

1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Hidehiko Abe . resolved

Thank you for review!

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jul 8, 2025, 4:29:59 PMJul 8

to Hidehiko Abe, Kinuko Yasuda, Xiyuan Xia, Muyao Xu, Code Review Nudger, Yiren Wang, Di Wu, Georg Neis, Jun Ishiguro, AyeAye, chromium...@chromium.org, feature-me...@chromium.org, ffred...@chromium.org, ipc-securi...@chromium.org, oshima...@chromium.org, steimel+...@chromium.org, tluk+...@chromium.org

Chromium LUCI CQ submitted the change

Unreviewed changes

7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Change information

Commit message:

Remove MediaUIAsh crosapi.

BUG=354842935
TEST=Tryjob

Change-Id: I11aca18ff0a5252948651963933e6936f6e739d6

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

Reviewed-by: Muyao Xu <muy...@google.com>

Commit-Queue: Hidehiko Abe <hide...@chromium.org>

Reviewed-by: Xiyuan Xia <xiy...@chromium.org>

Reviewed-by: Kinuko Yasuda <kin...@chromium.org>

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

Files:

M ash/test/ash_test_helper.cc
M ash/test/ash_test_helper.h
M chrome/browser/ash/crosapi/BUILD.gn
M chrome/browser/ash/crosapi/crosapi_ash.cc
M chrome/browser/ash/crosapi/crosapi_ash.h
D chrome/browser/ash/crosapi/media_ui_ash.cc
D chrome/browser/ash/crosapi/media_ui_ash.h
D chrome/browser/ash/crosapi/media_ui_ash_browsertest.cc
M chrome/browser/ui/ash/global_media_controls/BUILD.gn
M chrome/browser/ui/ash/global_media_controls/DEPS
M chrome/browser/ui/ash/global_media_controls/media_notification_provider_impl.cc
M chrome/browser/ui/ash/global_media_controls/media_notification_provider_impl.h
M chrome/browser/ui/ash/global_media_controls/media_notification_provider_impl_unittest.cc
M chrome/browser/ui/global_media_controls/media_notification_service.cc
M chromeos/crosapi/mojom/BUILD.gn
M chromeos/crosapi/mojom/crosapi.mojom
D chromeos/crosapi/mojom/media_ui.mojom
M content/browser/media/session/media_session_impl.cc
M content/public/browser/media_session.h

Change size: L

Delta: 19 files changed, 56 insertions(+), 273 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Xiyuan Xia, +1 by Muyao Xu, +1 by Kinuko Yasuda

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

Remove MediaUIAsh crosapi. [chromium/src : main]

Hidehiko Abe (Gerrit)

Hidehiko Abe added 1 comment

Related details

Hidehiko Abe (Gerrit)

Hidehiko Abe added 1 comment

Related details

Xiyuan Xia (Gerrit)

Xiyuan Xia added 1 comment

Related details

Muyao Xu (Gerrit)

Muyao Xu voted Code-Review+1

Related details

Hidehiko Abe (Gerrit)

Hidehiko Abe voted and added 2 comments

Votes added by Hidehiko Abe

2 comments

Related details

Xiyuan Xia (Gerrit)

Xiyuan Xia voted and added 2 comments

Votes added by Xiyuan Xia

2 comments

Related details

Kinuko Yasuda (Gerrit)

Kinuko Yasuda voted Code-Review+1

Related details

Hidehiko Abe (Gerrit)

Hidehiko Abe voted and added 1 comment

Votes added by Hidehiko Abe

1 comment

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Unreviewed changes

Change information