This also means that the service will be torn down and restarted when in process, I guess? If that made things explode, we would've noticed by now, but it's still some performance overhead.

Issue 913476 has been merged into this issue.

Sorry for the breakage. Simple solution will be to pass base::nullopt to the ServiceKeeplive constructor if a zero TimeDelta is passed to the service constructor.

Probably a better long-term solution is to push the special treatment of zero up even further (i.e. to whatever processes the command-line arg) and make the service constructor take an Optional<TimeDelta> too.

It's my fault - missed it during the code review. Good catch Max!

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

commit f10ae53e17c31f66b1597f535e4ceeacb820ced0
Author: Max Morin <maxmorin@chromium.org>
Date: Thu Jan 10 09:15:16 2019

Fix audio service quit timeout logic.

With this change, setting it to 0 is still taken as "tear down asap",
but the default value is "never tear down".

The field trial testing config is updated to not set the
teardown_timeout_s parameters (since the default is what we want
already).

Bug:  920173 
Change-Id: Id49a60c7a6ffd231827248431d017755901bd1ee
Tbr: dalecurtis
Reviewed-on: https://chromium-review.googlesource.com/c/1402766
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621514}
[modify] https://crrev.com/f10ae53e17c31f66b1597f535e4ceeacb820ced0/media/base/media_switches.cc
[modify] https://crrev.com/f10ae53e17c31f66b1597f535e4ceeacb820ced0/services/audio/service.cc
[modify] https://crrev.com/f10ae53e17c31f66b1597f535e4ceeacb820ced0/services/audio/service.h
[modify] https://crrev.com/f10ae53e17c31f66b1597f535e4ceeacb820ced0/services/audio/service_factory.cc
[modify] https://crrev.com/f10ae53e17c31f66b1597f535e4ceeacb820ced0/services/audio/test/service_lifetime_connector_test.cc
[modify] https://crrev.com/f10ae53e17c31f66b1597f535e4ceeacb820ced0/testing/variations/fieldtrial_testing_config.json

Verified with new unit tests and also manual testing.

Released in 73.0.3668.0 Canary.
The change fixes how an absence of audio service teardown timeout in finch config is interpreted by the audio service experiment; it does not affect non-experimental path. Safe change. Requesting a merge - we need this fix to continue experimentation.

This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approved

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

commit 356fbf675454dd6628da815fc8217275a8f82db5
Author: Max Morin <maxmorin@chromium.org>
Date: Mon Jan 14 13:12:30 2019

[M72]Fix audio service quit timeout logic.

With this change, setting it to 0 is still taken as "tear down asap",
but the default value is "never tear down".

The field trial testing config is updated to not set the
teardown_timeout_s parameters (since the default is what we want
already).

Bug:  920173 
Change-Id: Id49a60c7a6ffd231827248431d017755901bd1ee
Tbr: dalecurtis
Reviewed-on: https://chromium-review.googlesource.com/c/1402766
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#621514}(cherry picked from commit f10ae53e17c31f66b1597f535e4ceeacb820ced0)
Reviewed-on: https://chromium-review.googlesource.com/c/1409193
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#663}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/356fbf675454dd6628da815fc8217275a8f82db5/media/base/media_switches.cc
[modify] https://crrev.com/356fbf675454dd6628da815fc8217275a8f82db5/services/audio/service.cc
[modify] https://crrev.com/356fbf675454dd6628da815fc8217275a8f82db5/services/audio/service.h
[modify] https://crrev.com/356fbf675454dd6628da815fc8217275a8f82db5/services/audio/service_factory.cc
[modify] https://crrev.com/356fbf675454dd6628da815fc8217275a8f82db5/services/audio/test/service_lifetime_connector_test.cc
[modify] https://crrev.com/356fbf675454dd6628da815fc8217275a8f82db5/testing/variations/fieldtrial_testing_config.json

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

Commit: 356fbf675454dd6628da815fc8217275a8f82db5
Author: maxmorin@chromium.org
Commiter: maxmorin@chromium.org
Date: 2019-01-14 13:12:30 +0000 UTC

[M72]Fix audio service quit timeout logic.

With this change, setting it to 0 is still taken as "tear down asap",
but the default value is "never tear down".

The field trial testing config is updated to not set the
teardown_timeout_s parameters (since the default is what we want
already).

Bug:  920173 
Change-Id: Id49a60c7a6ffd231827248431d017755901bd1ee
Tbr: dalecurtis
Reviewed-on: https://chromium-review.googlesource.com/c/1402766
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#621514}(cherry picked from commit f10ae53e17c31f66b1597f535e4ceeacb820ced0)
Reviewed-on: https://chromium-review.googlesource.com/c/1409193
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#663}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}