Improve video capture service shutdown-when-inactive logic |
||
Issue descriptionThe video capture service shuts down when not in use. The current logic triggers a 5 second timer when the last client disconnects. When the timer expires, it either stays alive if there now is a client connect or it shuts down if there still is no client connected. This logic is not very suitable for usage patterns where clients may connect frequently but only for very brief periods of time. This is the case when clients only want to query device infos. What can happen is the following: 1. Client connects 2. Client disconnects -> A 5 second timer is started 3. 4 seconds later, another client connects and disconnects 4. 1 second later, the timer from 2. triggers, and since no client is connected, the service shuts down A better logic would be to cancel/renew the 5 second timeout as soon as a new client connects. Code where the timer is set is here: https://cs.chromium.org/chromium/src/services/video_capture/service_impl.cc?q=service_impl.cc&dr&l=91
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fa9f04c03d51a62847afc46ef45fe9f4309d9125 commit fa9f04c03d51a62847afc46ef45fe9f4309d9125 Author: Christian Fremerey <chfremer@chromium.org> Date: Tue Jun 19 19:02:17 2018 Make TestConnectorFactory release the Service and ServiceContext when a service requests quit Currently, TestConnectorFactory does not release the Service when it requests to be quit. This CL adds this functionality in order to allow testing of service shutdown behavior using TestConnectorFactory via observing the destruction of the service implementation class instance. Concrete example for which this was added: https://chromium-review.googlesource.com/c/chromium/src/+/1089921 Bug: 829581 Change-Id: I662c531329810fdb3dd1af5603628a0cb0dbba38 Reviewed-on: https://chromium-review.googlesource.com/1097991 Commit-Queue: Christian Fremerey <chfremer@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Cr-Commit-Position: refs/heads/master@{#568552} [modify] https://crrev.com/fa9f04c03d51a62847afc46ef45fe9f4309d9125/services/service_manager/public/cpp/test/test_connector_factory.cc [modify] https://crrev.com/fa9f04c03d51a62847afc46ef45fe9f4309d9125/services/service_manager/public/cpp/test/test_connector_factory.h
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f1bec8fbac2adf64ef308ccabcd0481201f6c03 commit 2f1bec8fbac2adf64ef308ccabcd0481201f6c03 Author: Christian Fremerey <chfremer@chromium.org> Date: Fri Jun 22 17:01:07 2018 [Video capture service] Improve delayed service quit logic The video capture service shuts down when not in use. The current logic triggers a 5 second timer when the last client disconnects. When the timer expires, it either stays alive if there now is a client connect or it shuts down if there still is no client connected. This logic is not very suitable for usage patterns where clients may connect frequently but only for very brief periods of time. If that happens, the service may end up shutting down and starting back up again frequently. This CL changes the logic such that when a new client connects, any previous timers are cancelled. Also part of this CL: * Move tests that are concerned with verifying when the service does or does not quit closer to the implementation. This allows eliminating the need for dynamically configuring a timeout for testing, and instead looking for ref counts. Bug: 829581 Test: services_unittests --gtest_filter=DeviceFactoryProviderConnectorTest.* Change-Id: I788775aaf6e77159a9142994d4772200ab42a14d Reviewed-on: https://chromium-review.googlesource.com/1089921 Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Commit-Queue: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#569666} [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/service_manager/public/cpp/service_keepalive.cc [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/service_manager/public/cpp/service_keepalive.h [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/device_factory_provider_impl.cc [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/device_factory_provider_impl.h [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/public/mojom/constants.mojom [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/public/mojom/device_factory_provider.mojom [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/service_impl.cc [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/service_impl.h [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/test/device_factory_provider_connectortest.cc [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/test/device_factory_provider_test.cc [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/test/device_factory_provider_test.h [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/test/device_factory_provider_unittest.cc [modify] https://crrev.com/2f1bec8fbac2adf64ef308ccabcd0481201f6c03/services/video_capture/test/fake_device_unittest.cc
,
Aug 28
|
||
►
Sign in to add a comment |
||
Comment 1 by chfremer@chromium.org
, Apr 25 2018