[Ash] Inject VideoConferenceManagerAsh to its clients as a dependency [chromium/src : main]
Di Wu (Gerrit)
Di Wu has uploaded the change for review![]( "Open in Gerrit")
Commit message
[Ash] Inject VideoConferenceManagerAsh to its clients as a dependency
This CL needs to go after a previous CL crrev.com/c/6218261.
This CL formally dismantles the singleton design of
VideoConferenceManagerAsh. In other words, all four current clients of
VideoConferenceManagerAsh can no longer easily access the single and
globally shared VideoConferenceManagerAsh object through a static get
method.
The four currently known clients are:
1. video_conference::VideoConferenceManagerClientImpl
2. VideoConferenceAppServiceClient
3. VideoConferenceAshFeatureClient
4. ChromeCaptureModeDelegate
For 1, 2, and 3, the dependency will be injected through each client
class's constructor. For 4, the dependency will be injected through a
newly added setter method.
This Cl also updates affected browser tests of each client to reflect
this dependency injection change.
Change diff
Change information
- M chrome/browser/ash/crosapi/BUILD.gn
- M chrome/browser/ash/crosapi/video_conference_ash_browsertest.cc
- M chrome/browser/ash/main_parts/BUILD.gn
- M chrome/browser/ash/main_parts/DEPS
- M chrome/browser/ash/main_parts/chrome_browser_main_parts_ash.cc
- M chrome/browser/ash/main_parts/chrome_browser_main_parts_ash.h
- M chrome/browser/ash/video_conference/BUILD.gn
- M chrome/browser/ash/video_conference/DEPS
- M chrome/browser/ash/video_conference/video_conference_app_service_client_browsertest.cc
- M chrome/browser/ash/video_conference/video_conference_ash_feature_client_browsertest.cc
- M chrome/browser/ash/video_conference/video_conference_manager_ash.cc
- M chrome/browser/ash/video_conference/video_conference_manager_ash.h
- M chrome/browser/chromeos/video_conference/BUILD.gn
- M chrome/browser/chromeos/video_conference/DEPS
- M chrome/browser/chromeos/video_conference/video_conference_manager_client_browsertest.cc
- M chrome/browser/chromeos/video_conference/video_conference_media_listener_browsertest.cc
- M chrome/browser/ui/ash/capture_mode/BUILD.gn
- M chrome/browser/ui/ash/capture_mode/DEPS
- M chrome/browser/ui/ash/capture_mode/capture_mode_browsertest.cc
- M chrome/browser/ui/ash/capture_mode/chrome_capture_mode_delegate.cc
- M chrome/browser/ui/ash/capture_mode/chrome_capture_mode_delegate.h
- M chrome/browser/ui/ash/main_extra_parts/BUILD.gn
- M chrome/browser/ui/ash/main_extra_parts/DEPS
- M chrome/browser/ui/ash/main_extra_parts/chrome_browser_main_extra_parts_ash.cc
- M chrome/browser/ui/ash/main_extra_parts/chrome_browser_main_extra_parts_ash.h
- M chrome/test/BUILD.gn
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Di Wu (Gerrit)
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Di Wu (Gerrit)
Di Wu added 1 comment![]( "Open in Gerrit")
ChromeCaptureModeDelegate::Get()->set_video_conference_manager_ash(@hide...@chromium.org Please note that I didn't use `ChromeShellDelegate` as a stepping stone to pass over the dependency from `ChromeBrowserMainPartsAsh` to `ChromeBrowserMainExtraPartsAsh` and finally to `ChromeCaptureModeDelegate`. And as you can see I definitely didn't pass it over through ctor injection, instead I created a dedicated setter to inject.
The reason is that the timing of the ctor for `ChromeShellDelegate` is earlier than the `PreProfileInit()` of both the `main parts` and the `main extra parts`. In addition, I cannot initialize `VideoConferenceManagerAsh` earlier than `PreProfileInit()` as `VideoConferenceManagerAsh` has its own dependency timing issues. What makes matters worse is, as `ChromeShellDelegate` itself has no static singleton access method, injecting a depedency to it at a later stage is not practical either.
So, as you see here, I opted for a tradeoff that piggybacks on the two static getter methods of `ChromeCaptureModeDelegate::Get()` and `ChromeBrowserMainExtraPartsAsh::Get()` to achieve the goal of injecting `VideoConferenceManagerAsh` at the right time.
I understand this whole chain of injection is convulted. I'm more than happy to keep revising this CL. Please let me know what you think.
Related details
- Hidehiko Abe
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Di Wu (Gerrit)
Di Wu abandoned this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |