New issue
Advanced search Search tips

Issue 904240 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Service Manager cleanup meta

Project Member Reported by rockot@google.com, Nov 11

Issue description

Lots of things inside and around Service Manager are pretty unpolished. This is a general metabug for tracking cleanup work, since it's a bunch of little things. Examples:

- The logic in ServiceManager::Connect is not unlike spaghetti
- ServiceManager has some unnecessary API surface like StartService()
- BackgroundServiceManager no longer needs to exist
- Many Service Manager tests can be rewritten and greatly simplified
  given our more current APIs and test helpers.
- Random thought: we could support constexpr Identity construction 
 
Owner: rockot@google.com
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53ce5a6136b04aff8a653cdcee1f4ef788ba2515

commit 53ce5a6136b04aff8a653cdcee1f4ef788ba2515
Author: Ken Rockot <rockot@google.com>
Date: Fri Nov 16 02:19:00 2018

[service-manager] Require valid fields in Identity

Changes Identity so that none of its fields are optional. All Identity
objects are now either empty and invalid, or fully populated and refer
to a specific, concrete service instance that did, does, or may exist at
some point.

Bug:  902590 ,904240
Change-Id: I5b35909afb39b2588f0841d5b1713a6f7c06c800
Reviewed-on: https://chromium-review.googlesource.com/c/1333126
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#608634}
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/safe_browsing/client_side_detection_service.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/spellchecker/spell_check_host_chrome_impl_mac.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/spellchecker/spellcheck_factory.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/spellchecker/spellcheck_service.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/chrome/browser/spellchecker/test/spellcheck_content_browser_client.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/content/common/service_manager/service_manager_connection_impl_unittest.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/ios/web/service_manager_connection_impl_unittest.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/catalog/catalog.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/file/file_service.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/BUILD.gn
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/connect_params.h
[delete] https://crrev.com/2faebc81dd1b73ae2f63b46e5aa02092fb533178/services/service_manager/connect_util.cc
[delete] https://crrev.com/2faebc81dd1b73ae2f63b46e5aa02092fb533178/services/service_manager/connect_util.h
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/BUILD.gn
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/connector.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/identity.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/identity.h
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/identity.typemap
[add] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/identity_mojom_traits.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/identity_mojom_traits.h
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/service_filter.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/service_filter.h
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/service_test.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/cpp/test/test_connector_factory.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/public/mojom/connector.mojom
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/runner/host/service_process_launcher.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/service_manager.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/service_manager.h
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/standalone/context.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/tests/connect/connect_test.mojom
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/tests/connect/connect_test_app.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/tests/connect/connect_test_class_app.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/tests/connect/connect_test_exe.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/tests/connect/connect_test_package.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/tests/connect/connect_unittest.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/tests/service_manager/service_manager_listener_unittest.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/service_manager/tests/service_manager/service_manager_unittest.cc
[modify] https://crrev.com/53ce5a6136b04aff8a653cdcee1f4ef788ba2515/services/test/user_id/user_id_service.cc

Comment 3 by rockot@google.com, Jan 17 (5 days ago)

Sign in to add a comment