Have Device Service observe MessageLoop destruction

The Device Service needs to clean up parts of its internal state as part
of browser shutdown. However, it also needs to run on the UI thread,
and embedded services that run on the UI thread are not guaranteed to
be destroyed as part of browser shutdown (tasks to destroy these
services are posted from the IO thread by
ServiceManagerConnectionImpl::ShutDownOnIOThread, but the UI thread is
typically shut down before these posted tasks are run).

To solve this issue we discussed adding plumbing wherein embedded
services could inform //content that they wanted to be notified when
shutdown was occurring on the main thread. However, on investigation
this plumbing would be painful to implement: it is only
EmbeddedInstanceManager that has direct information of these service
instances, and that object lives far away from //content's
ServiceManagerContext, the object that knows when shutdown is occurring
on the main thread.

This CL takes an alternative approach of having the Device Service
implementation observe the destruction of its MessageLoop. I have
verified that the observation is triggered on shutdown of Chrome.

Bug: 794105
Change-Id: I3b383871679d42f544812be4bcb13c872cf276ff
Reviewed-on: https://chromium-review.googlesource.com/1099245
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567260}

services/device/device_service.cc

@@ -108,10 +108,19 @@ DeviceService::DeviceService(
       io_task_runner_(std::move(io_task_runner)),
       geolocation_request_context_producer_(
           geolocation_request_context_producer),
-      geolocation_api_key_(geolocation_api_key) {}
+      geolocation_api_key_(geolocation_api_key) {
+  base::MessageLoopCurrent::Get()->AddDestructionObserver(this);
+}
 #endif
 
 DeviceService::~DeviceService() {
+  if (base::MessageLoopCurrent::Get()) {
+    base::MessageLoopCurrent::Get()->RemoveDestructionObserver(this);
+    ShutDown();
+  }
+}
+
+void DeviceService::ShutDown() {
 #if !defined(OS_ANDROID)
   device::BatteryStatusService::GetInstance()->Shutdown();
 #endif
@@ -122,6 +131,17 @@ DeviceService::~DeviceService() {
   }
 }
 
+// On embedder shutdown, the Device Service needs to shut down certain parts of
+// its state to avoid DCHECKs going off (see. https://crbug.com/794105 for
+// details). However, when used in the context of the browser the Device Service
+// is not guaranteed to have its destructor run, as the sequence on which it
+// runs is stopped before the task posted to run the Device Service destructor
+// is run. Hence, observe destruction of that sequence in order to clean up the
+// necessary state.
+void DeviceService::WillDestroyCurrentMessageLoop() {
+  ShutDown();
+}
+
 void DeviceService::OnStart() {
   registry_.AddInterface<mojom::Fingerprint>(base::Bind(
       &DeviceService::BindFingerprintRequest, base::Unretained(this)));

services/device/device_service.h

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop_current.h"
 #include "build/build_config.h"
 #include "device/geolocation/geolocation_provider.h"
 #include "device/geolocation/geolocation_provider_impl.h"
@@ -85,7 +86,8 @@ std::unique_ptr<service_manager::Service> CreateDeviceService(
     const CustomLocationProviderCallback& custom_location_provider_callback);
 #endif
 
-class DeviceService : public service_manager::Service {
+class DeviceService : public service_manager::Service,
+                      public base::MessageLoopCurrent::DestructionObserver {
  public:
 #if defined(OS_ANDROID)
   DeviceService(scoped_refptr<base::SingleThreadTaskRunner> file_task_runner,
@@ -102,9 +104,17 @@ class DeviceService : public service_manager::Service {
                     geolocation_request_context_producer,
                 const std::string& geolocation_api_key);
 #endif
+  // Not guaranteed to run on embedder shutdown; see Shutdown().
   ~DeviceService() override;
 
  private:
+  // base::MessageLoopCurrent::DestructionObserver:
+  void WillDestroyCurrentMessageLoop() override;
+
+  // Any state that *must* be cleaned up when the embedder shuts down should be
+  // placed in this method.
+  void ShutDown();
+
   // service_manager::Service:
   void OnStart() override;
   void OnBindInterface(const service_manager::BindSourceInfo& source_info,