Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(200)

Issue 2701883002: service_manager: More consistent Service lifecycle API (Closed)

Created:
3 years, 10 months ago by Ken Rockot(use gerrit already)
Modified:
3 years, 10 months ago
Reviewers:
sky
CC:
Aaron Boodman, abarth-chromium, alokp+watch_chromium.org, chromium-reviews, darin (slow to review), darin-cc_chromium.org, feature-media-reviews_chromium.org, jam, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

service_manager: More consistent Service lifecycle API Renames OnStop() to a more accurate OnServiceManagerConnectionLost() and clarifies the meaning and usage in the documentation. Ensures that Service::context() is always valid after the constructor rather than only after OnStart(). Renames SetConnectionLostClosure to SetQuitClosure to reflect its actual meaning. Removes the unused DestroyService() API. BUG=672614 R=sky@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel TBR=xhwang@chromium.org for rename in media Review-Url: https://codereview.chromium.org/2701883002 Cr-Commit-Position: refs/heads/master@{#451453} Committed: https://chromium.googlesource.com/chromium/src/+/1033a629e2246612bf9c8b37b3400db53f53133e

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+105 lines, -97 lines) Patch
M chrome/app/mash/mash_runner.cc View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/test/base/mash_browser_tests_main.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M content/common/service_manager/embedded_service_runner.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/common/service_manager/service_manager_connection_impl.cc View 1 chunk +1 line, -1 line 0 comments Download
M media/mojo/services/media_service.h View 1 chunk +1 line, -1 line 0 comments Download
M media/mojo/services/media_service.cc View 1 chunk +1 line, -1 line 0 comments Download
M services/image_decoder/image_decoder_service.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M services/image_decoder/image_decoder_service.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M services/service_manager/background/tests/test_service.cc View 1 chunk +0 lines, -3 lines 0 comments Download
M services/service_manager/public/cpp/lib/service.cc View 2 chunks +23 lines, -4 lines 0 comments Download
M services/service_manager/public/cpp/lib/service_context.cc View 4 chunks +15 lines, -24 lines 0 comments Download
M services/service_manager/public/cpp/lib/service_runner.cc View 1 chunk +1 line, -1 line 0 comments Download
M services/service_manager/public/cpp/service.h View 3 chunks +21 lines, -20 lines 0 comments Download
M services/service_manager/public/cpp/service_context.h View 4 chunks +31 lines, -21 lines 0 comments Download
M services/service_manager/tests/connect/connect_test_package.cc View 1 chunk +1 line, -1 line 0 comments Download
M services/service_manager/tests/lifecycle/app_client.h View 1 chunk +1 line, -1 line 0 comments Download
M services/service_manager/tests/lifecycle/app_client.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M services/service_manager/tests/service_manager/embedder.cc View 1 chunk +1 line, -1 line 0 comments Download
M services/shape_detection/shape_detection_service.h View 1 chunk +0 lines, -1 line 0 comments Download
M services/shape_detection/shape_detection_service.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M services/tracing/service.h View 1 chunk +1 line, -1 line 0 comments Download
M services/tracing/service.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 25 (21 generated)
Ken Rockot(use gerrit already)
PTAL. I ended up leaving the bool return value for now, because I don't like ...
3 years, 10 months ago (2017-02-17 18:47:58 UTC) #4
sky
LGTM
3 years, 10 months ago (2017-02-17 20:36:58 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2701883002/40001
3 years, 10 months ago (2017-02-18 09:58:36 UTC) #22
commit-bot: I haz the power
3 years, 10 months ago (2017-02-18 11:04:19 UTC) #25
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/1033a629e2246612bf9c8b37b340...

Powered by Google App Engine
This is Rietveld 408576698