Ken, I took a look to this offline with hjd@. Looks like there isn't a reliable way to get the PID, as sometimes the SvcManager itself receives the PID too late and doesn't propagate it back to us.

Here are two proposals which can address this issue, and I lean towards implementing the second one:

1. Defer OnServiceStarted until the SM has received a PID (or the PIDReceiver pipe is broken, if a service instance fails to report its PID for whatever reason). Drawback is this could lead to situations where a service is started and then stopped before ever receiving a pid; solvable but awkward behavior to specify.

2. Change OnServiceStarted to pass a PIDReceiver& instead of a pid. Then an interested listener can bind the request and wait for the PID on that pipe. This adds a small amount of complexity to the listener but I think makes it easier to specify reliable behavior.

Can we 3: have an explicit OnServicePIDReceived?
1, as you say, could have very funky corner cases and 2 seems overcomplicated.

I kind of wanted to avoid introducing that API, but on second thought...
sure, SGTM!

Great

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

commit 64b7ed15ff30507422bed93da5de23f9fb447b8b
Author: Hector Dearman <hjd@google.com>
Date: Tue Jul 11 11:57:19 2017

Add OnServicePIDReceived to ServiceManagerListener

This provides a more consistent way for listeners to get the PID
of a service. The existing APIs for doing this (for example the
pid argument of OnServiceStarted) will be removed in a follow up.

Bug:  739710 
Change-Id: Ia41cdb12e412bd478aefb1481431db1cd2824265
Reviewed-on: https://chromium-review.googlesource.com/563366
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Hector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485596}
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/mash/task_viewer/task_viewer.cc
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/services/resource_coordinator/memory_instrumentation/process_map.cc
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/services/resource_coordinator/memory_instrumentation/process_map.h
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/services/service_manager/public/interfaces/service_manager.mojom
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/services/service_manager/service_manager.cc
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/services/service_manager/service_manager.h
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/services/service_manager/tests/lifecycle/lifecycle_unittest.cc
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/services/service_manager/tests/service_manager/service_manager_unittest.cc
[modify] https://crrev.com/64b7ed15ff30507422bed93da5de23f9fb447b8b/services/video_capture/test/device_factory_provider_test.h

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

commit 6a576a58c36bcaf95478b30392ae6ecf2cfc5af6
Author: Hector Dearman <hjd@google.com>
Date: Tue Jul 18 11:15:09 2017

memory-infra: Use new OnServicePIDReceived API

Bug:  739710 
Change-Id: Ib926ae1ec013b33b7a90dcedb78960713cfa97e0
Reviewed-on: https://chromium-review.googlesource.com/565510
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: Hector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487442}
[modify] https://crrev.com/6a576a58c36bcaf95478b30392ae6ecf2cfc5af6/services/resource_coordinator/memory_instrumentation/process_map.cc
[modify] https://crrev.com/6a576a58c36bcaf95478b30392ae6ecf2cfc5af6/services/resource_coordinator/memory_instrumentation/process_map.h
[modify] https://crrev.com/6a576a58c36bcaf95478b30392ae6ecf2cfc5af6/services/resource_coordinator/memory_instrumentation/process_map_unittest.cc

There is still references to this bug number in code.

Yes, I saw that recently as well, I think sadly this wasn't/couldn't be fixed in a way that allows us to put the NOTREACHED back in so we're stuck with the VLOG :( I will take out the TODO tomorrow.

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

commit 08835ff17afdaaa200c729682d7bcd8ea92e6230
Author: Hector Dearman <hjd@google.com>
Date: Wed Sep 13 18:42:33 2017

memory-infra: Remove TODOs

Bug:  739710 
Change-Id: Ib09f456847890865370d4a07b52dfced05adefb8
Reviewed-on: https://chromium-review.googlesource.com/664757
Commit-Queue: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501703}
[modify] https://crrev.com/08835ff17afdaaa200c729682d7bcd8ea92e6230/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc