service worker: Move lifetime UMA from embedded worker registry to worker.

This is a step toward removing EmbeddedWorkerRegistry, which should
no longer be needed since mojofication.

Bug: 931084
Change-Id: I53b6bc29d92427b2e36e48f585c309b0308cfc06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1502337Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637624}

content/browser/BUILD.gn

@@ -1674,8 +1674,6 @@ jumbo_source_set("browser") {
     "service_worker/service_worker_internals_ui.h",
     "service_worker/service_worker_job_coordinator.cc",
     "service_worker/service_worker_job_coordinator.h",
-    "service_worker/service_worker_lifetime_tracker.cc",
-    "service_worker/service_worker_lifetime_tracker.h",
     "service_worker/service_worker_metrics.cc",
     "service_worker/service_worker_metrics.h",
     "service_worker/service_worker_navigation_handle.cc",

content/browser/service_worker/embedded_worker_instance.cc

@@ -401,6 +401,28 @@ class EmbeddedWorkerInstance::DevToolsProxy {
   DISALLOW_COPY_AND_ASSIGN(DevToolsProxy);
 };
 
+// Tracks how long a service worker runs for, for UMA purposes.
+class EmbeddedWorkerInstance::ScopedLifetimeTracker {
+ public:
+  ScopedLifetimeTracker() : start_ticks_(base::TimeTicks::Now()) {}
+
+  ~ScopedLifetimeTracker() {
+    if (!start_ticks_.is_null()) {
+      ServiceWorkerMetrics::RecordRuntime(base::TimeTicks::Now() -
+                                          start_ticks_);
+    }
+  }
+
+  // Called when DevTools was attached to the worker. Ensures no metric is
+  // recorded for this worker.
+  void Abort() { start_ticks_ = base::TimeTicks(); }
+
+ private:
+  base::TimeTicks start_ticks_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedLifetimeTracker);
+};
+
 // A handle for a renderer process managed by ServiceWorkerProcessManager on the
 // UI thread. Lives on the IO thread.
 class EmbeddedWorkerInstance::WorkerProcessHandle {
@@ -919,6 +941,9 @@ void EmbeddedWorkerInstance::OnStarted(
     return;
   }
 
+  if (!devtools_attached_)
+    lifetime_tracker_ = std::make_unique<ScopedLifetimeTracker>();
+
   if (!registry_->OnWorkerStarted(process_id(), embedded_worker_id_))
     return;
   // Stop was requested before OnStarted was sent back from the worker. Just
@@ -1039,7 +1064,14 @@ void EmbeddedWorkerInstance::SetDevToolsAttached(bool attached) {
     return;
   if (inflight_start_task_)
     inflight_start_task_->set_skip_recording_startup_time();
-  registry_->AbortLifetimeTracking(embedded_worker_id_);
+  AbortLifetimeTracking();
+}
+
+void EmbeddedWorkerInstance::AbortLifetimeTracking() {
+  if (lifetime_tracker_) {
+    lifetime_tracker_->Abort();
+    lifetime_tracker_.reset();
+  }
 }
 
 void EmbeddedWorkerInstance::OnNetworkAccessedForScriptLoad() {
@@ -1056,6 +1088,7 @@ void EmbeddedWorkerInstance::ReleaseProcess() {
   instance_host_binding_.Close();
   devtools_proxy_.reset();
   process_handle_.reset();
+  lifetime_tracker_.reset();
   status_ = EmbeddedWorkerStatus::STOPPED;
   starting_phase_ = NOT_STARTING;
   thread_id_ = kInvalidEmbeddedWorkerThreadId;

content/browser/service_worker/embedded_worker_instance.h

@@ -169,6 +169,11 @@ class CONTENT_EXPORT EmbeddedWorkerInstance
   void SetDevToolsAttached(bool attached);
   bool devtools_attached() const { return devtools_attached_; }
 
+  // Ensures that the UMA for how long this worker ran for, normally emitted
+  // when the worker stops, is not emitted. Takes effect only for the current
+  // running session, and has no effect if the worker is not currently running.
+  void AbortLifetimeTracking();
+
   bool network_accessed_for_script() const {
     return network_accessed_for_script_;
   }
@@ -226,6 +231,7 @@ class CONTENT_EXPORT EmbeddedWorkerInstance
 
  private:
   typedef base::ObserverList<Listener>::Unchecked ListenerList;
+  class ScopedLifetimeTracker;
   class StartTask;
   class WorkerProcessHandle;
   friend class EmbeddedWorkerRegistry;
@@ -343,6 +349,7 @@ class CONTENT_EXPORT EmbeddedWorkerInstance
   std::unique_ptr<DevToolsProxy> devtools_proxy_;
 
   std::unique_ptr<StartTask> inflight_start_task_;
+  std::unique_ptr<ScopedLifetimeTracker> lifetime_tracker_;
 
   // This is valid only after a process is allocated for the worker.
   ServiceWorkerMetrics::StartSituation start_situation_ =

content/browser/service_worker/embedded_worker_instance_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "content/browser/service_worker/embedded_worker_registry.h"
 #include "content/browser/service_worker/embedded_worker_status.h"
@@ -52,6 +53,8 @@ EmbeddedWorkerInstance::StatusCallback ReceiveStatus(
       out_status, std::move(quit));
 }
 
+const char kHistogramServiceWorkerRuntime[] = "ServiceWorker.Runtime";
+
 }  // namespace
 
 class EmbeddedWorkerInstanceTest : public testing::Test,
@@ -876,7 +879,7 @@ TEST_F(EmbeddedWorkerInstanceTest, CacheStorageOptimizationIsDisabled) {
 // Starts the worker with kAbruptCompletion status.
 class AbruptCompletionInstanceClient : public FakeEmbeddedWorkerInstanceClient {
  public:
-  AbruptCompletionInstanceClient(EmbeddedWorkerTestHelper* helper)
+  explicit AbruptCompletionInstanceClient(EmbeddedWorkerTestHelper* helper)
       : FakeEmbeddedWorkerInstanceClient(helper) {}
   ~AbruptCompletionInstanceClient() override = default;
 
@@ -913,4 +916,90 @@ TEST_F(EmbeddedWorkerInstanceTest, AbruptCompletion) {
   worker->Stop();
 }
 
+// Tests recording the lifetime UMA.
+TEST_F(EmbeddedWorkerInstanceTest, Lifetime) {
+  const GURL scope("http://example.com/");
+  const GURL url("http://example.com/worker.js");
+
+  RegistrationAndVersionPair pair = PrepareRegistrationAndVersion(scope, url);
+  std::unique_ptr<EmbeddedWorkerInstance> worker =
+      embedded_worker_registry()->CreateWorker(pair.second.get());
+
+  base::HistogramTester metrics;
+
+  // Start the worker.
+  StartWorker(worker.get(), CreateStartParams(pair.second));
+  metrics.ExpectTotalCount(kHistogramServiceWorkerRuntime, 0);
+
+  // Stop the worker.
+  worker->Stop();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, worker->status());
+
+  // The runtime metric should have been recorded.
+  metrics.ExpectTotalCount(kHistogramServiceWorkerRuntime, 1);
+}
+
+// Tests that the lifetime UMA isn't recorded if DevTools was attached
+// while the worker was running.
+TEST_F(EmbeddedWorkerInstanceTest, Lifetime_DevToolsAttachedAfterStart) {
+  const GURL scope("http://example.com/");
+  const GURL url("http://example.com/worker.js");
+
+  RegistrationAndVersionPair pair = PrepareRegistrationAndVersion(scope, url);
+  std::unique_ptr<EmbeddedWorkerInstance> worker =
+      embedded_worker_registry()->CreateWorker(pair.second.get());
+
+  base::HistogramTester metrics;
+
+  // Start the worker.
+  StartWorker(worker.get(), CreateStartParams(pair.second));
+
+  // Attach DevTools.
+  worker->SetDevToolsAttached(true);
+
+  // To make things tricky, detach DevTools.
+  worker->SetDevToolsAttached(false);
+
+  // Stop the worker.
+  worker->Stop();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, worker->status());
+
+  // The runtime metric should not have ben recorded since DevTools
+  // was attached at some point during the worker's life.
+  metrics.ExpectTotalCount(kHistogramServiceWorkerRuntime, 0);
+}
+
+// Tests that the lifetime UMA isn't recorded if DevTools was attached
+// before the worker finished starting.
+TEST_F(EmbeddedWorkerInstanceTest, Lifetime_DevToolsAttachedDuringStart) {
+  const GURL scope("http://example.com/");
+  const GURL url("http://example.com/worker.js");
+
+  RegistrationAndVersionPair pair = PrepareRegistrationAndVersion(scope, url);
+  std::unique_ptr<EmbeddedWorkerInstance> worker =
+      embedded_worker_registry()->CreateWorker(pair.second.get());
+
+  base::HistogramTester metrics;
+
+  // Attach DevTools while the worker is starting.
+  worker->Start(CreateStartParams(pair.second), base::DoNothing());
+  worker->SetDevToolsAttached(true);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(EmbeddedWorkerStatus::RUNNING, worker->status());
+
+  // To make things tricky, detach DevTools.
+  worker->SetDevToolsAttached(false);
+
+  // Stop the worker.
+  worker->Stop();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, worker->status());
+
+  // The runtime metric should not have ben recorded since DevTools
+  // was attached at some point during the worker's life.
+  metrics.ExpectTotalCount(kHistogramServiceWorkerRuntime, 0);
+}
+
 }  // namespace content

content/browser/service_worker/embedded_worker_registry.cc

@@ -56,18 +56,12 @@ bool EmbeddedWorkerRegistry::OnWorkerStarted(int process_id,
     return false;
   }
 
-  lifetime_tracker_.StartTiming(embedded_worker_id);
   return true;
 }
 
 void EmbeddedWorkerRegistry::OnWorkerStopped(int process_id,
                                              int embedded_worker_id) {
   worker_process_map_[process_id].erase(embedded_worker_id);
-  lifetime_tracker_.StopTiming(embedded_worker_id);
-}
-
-void EmbeddedWorkerRegistry::AbortLifetimeTracking(int embedded_worker_id) {
-  lifetime_tracker_.AbortTiming(embedded_worker_id);
 }
 
 EmbeddedWorkerInstance* EmbeddedWorkerRegistry::GetWorker(
@@ -116,7 +110,6 @@ void EmbeddedWorkerRegistry::DetachWorker(int process_id,
   worker_process_map_[process_id].erase(embedded_worker_id);
   if (worker_process_map_[process_id].empty())
     worker_process_map_.erase(process_id);
-  lifetime_tracker_.StopTiming(embedded_worker_id);
 }
 
 }  // namespace content

content/browser/service_worker/embedded_worker_registry.h

@@ -15,7 +15,6 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
-#include "content/browser/service_worker/service_worker_lifetime_tracker.h"
 #include "content/common/content_export.h"
 #include "third_party/blink/public/common/service_worker/service_worker_status_code.h"
 
@@ -55,9 +54,6 @@ class CONTENT_EXPORT EmbeddedWorkerRegistry
   bool OnWorkerStarted(int process_id, int embedded_worker_id);
   void OnWorkerStopped(int process_id, int embedded_worker_id);
 
-  // Aborts the timer which tracks the lifetime of the worker for UMA logging.
-  void AbortLifetimeTracking(int embedded_worker_id);
-
   // Returns an embedded worker instance for given |embedded_worker_id|.
   EmbeddedWorkerInstance* GetWorker(int embedded_worker_id);
 
@@ -101,7 +97,6 @@ class CONTENT_EXPORT EmbeddedWorkerRegistry
 
   int next_embedded_worker_id_;
   const int initial_embedded_worker_id_;
-  ServiceWorkerLifetimeTracker lifetime_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(EmbeddedWorkerRegistry);
 };

content/browser/service_worker/service_worker_lifetime_tracker.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "content/browser/service_worker/service_worker_lifetime_tracker.h"
-
-#include "base/bind.h"
-#include "base/stl_util.h"
-#include "base/time/default_tick_clock.h"
-#include "content/browser/service_worker/service_worker_metrics.h"
-
-namespace content {
-
-ServiceWorkerLifetimeTracker::ServiceWorkerLifetimeTracker()
-    : ServiceWorkerLifetimeTracker(base::DefaultTickClock::GetInstance()) {}
-
-ServiceWorkerLifetimeTracker::ServiceWorkerLifetimeTracker(
-    const base::TickClock* tick_clock)
-    : tick_clock_(tick_clock) {}
-
-ServiceWorkerLifetimeTracker::~ServiceWorkerLifetimeTracker() = default;
-
-void ServiceWorkerLifetimeTracker::StartTiming(int64_t embedded_worker_id) {
-  DCHECK(!base::ContainsKey(running_workers_, embedded_worker_id));
-
-  running_workers_[embedded_worker_id] = tick_clock_->NowTicks();
-}
-
-void ServiceWorkerLifetimeTracker::StopTiming(int64_t embedded_worker_id) {
-  auto it = running_workers_.find(embedded_worker_id);
-  if (it == running_workers_.end())
-    return;
-  ServiceWorkerMetrics::RecordRuntime(tick_clock_->NowTicks() - it->second);
-  running_workers_.erase(it);
-}
-
-void ServiceWorkerLifetimeTracker::AbortTiming(int64_t embedded_worker_id) {
-  auto it = running_workers_.find(embedded_worker_id);
-  if (it == running_workers_.end())
-    return;
-  running_workers_.erase(it);
-}
-
-}  // namespace content

content/browser/service_worker/service_worker_lifetime_tracker.h

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_LIFETIME_TRACKER_H_
-#define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_LIFETIME_TRACKER_H_
-
-#include <map>
-#include <memory>
-
-#include "base/gtest_prod_util.h"
-#include "base/macros.h"
-#include "base/time/tick_clock.h"
-#include "base/timer/timer.h"
-#include "content/common/content_export.h"
-
-namespace content {
-
-// ServiceWorkerLifetimeTracker tracks how long service workers run, for UMA
-// purposes.
-class CONTENT_EXPORT ServiceWorkerLifetimeTracker {
- public:
-  ServiceWorkerLifetimeTracker();
-  explicit ServiceWorkerLifetimeTracker(const base::TickClock* tick_clock);
-  virtual ~ServiceWorkerLifetimeTracker();
-
-  // Called when the worker started running.
-  void StartTiming(int64_t embedded_worker_id);
-  // Called when the worker stopped running.
-  void StopTiming(int64_t embedded_worker_id);
-  // Called when DevTools was attached to the worker. Forgets the outstanding
-  // start timing.
-  void AbortTiming(int64_t embedded_worker_id);
-
- private:
-  friend class ServiceWorkerLifetimeTrackerTest;
-
-  void RecordHistograms();
-
-  const base::TickClock* tick_clock_;
-  std::map<int64_t /* embedded_worker_id */, base::TimeTicks /* start_time */>
-      running_workers_;
-
-  DISALLOW_COPY_AND_ASSIGN(ServiceWorkerLifetimeTracker);
-};
-
-}  // namespace content
-
-#endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_LIFETIME_TRACKER_H_

content/browser/service_worker/service_worker_lifetime_tracker_unittest.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "content/browser/service_worker/service_worker_lifetime_tracker.h"
-
-#include "base/test/metrics/histogram_tester.h"
-#include "base/test/simple_test_tick_clock.h"
-#include "base/time/tick_clock.h"
-#include "content/public/test/test_utils.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace content {
-
-class ServiceWorkerLifetimeTrackerTest : public testing::Test {
- public:
-  ServiceWorkerLifetimeTrackerTest() : tracker_(&tick_clock_) {}
-
-  base::SimpleTestTickClock* tick_clock() { return &tick_clock_; }
-  ServiceWorkerLifetimeTracker* tracker() { return &tracker_; }
-
- private:
-  base::SimpleTestTickClock tick_clock_;
-
-  ServiceWorkerLifetimeTracker tracker_;
-};
-
-TEST_F(ServiceWorkerLifetimeTrackerTest, Metrics) {
-  int64_t kVersion1 = 13;  // dummy value
-  int64_t kVersion2 = 14;  // dummy value
-
-  tick_clock()->SetNowTicks(base::TimeTicks::Now());
-
-  // Start a worker.
-  tracker()->StartTiming(kVersion2);
-
-  // Run a worker for 10 seconds.
-  {
-    base::HistogramTester metrics;
-    tracker()->StartTiming(kVersion1);
-    tick_clock()->Advance(base::TimeDelta::FromSeconds(10));
-    tracker()->StopTiming(kVersion1);
-    metrics.ExpectTimeBucketCount("ServiceWorker.Runtime",
-                                  base::TimeDelta::FromSeconds(10), 1);
-  }
-
-  // Advance 10 minutes and stop the worker. It should record Runtime.
-  {
-    base::HistogramTester metrics;
-    tick_clock()->Advance(base::TimeDelta::FromMinutes(10));
-    tracker()->StopTiming(kVersion2);
-    metrics.ExpectTimeBucketCount(
-        "ServiceWorker.Runtime",
-        base::TimeDelta::FromMinutes(10) + base::TimeDelta::FromSeconds(10), 1);
-  }
-
-  {
-    base::HistogramTester metrics;
-    // Start a worker and abort the timing.
-    tracker()->StartTiming(kVersion1);
-    tick_clock()->Advance(base::TimeDelta::FromSeconds(10));
-    tracker()->AbortTiming(kVersion1);
-
-    // Aborting multiple times should be fine.
-    tracker()->AbortTiming(kVersion1);
-    tracker()->AbortTiming(kVersion1);
-    // StopTiming should not record a timing.
-    tracker()->StopTiming(kVersion1);
-    tracker()->StopTiming(kVersion1);
-
-    metrics.ExpectTotalCount("ServiceWorker.Runtime", 0);
-  }
-}
-
-}  // namespace content

content/browser/service_worker/service_worker_version.cc

@@ -565,10 +565,8 @@ int ServiceWorkerVersion::StartRequestWithCustomTimeout(
   // request will be aborted soon, so don't bother aborting the request directly
   // here, and just skip this bookkeeping.
   if (context_) {
-    if (event_type == ServiceWorkerMetrics::EventType::LONG_RUNNING_MESSAGE) {
-      context_->embedded_worker_registry()->AbortLifetimeTracking(
-          embedded_worker_->embedded_worker_id());
-    }
+    if (event_type == ServiceWorkerMetrics::EventType::LONG_RUNNING_MESSAGE)
+      embedded_worker_->AbortLifetimeTracking();
 
     if (event_type != ServiceWorkerMetrics::EventType::INSTALL &&
         event_type != ServiceWorkerMetrics::EventType::ACTIVATE &&

content/test/BUILD.gn

@@ -1652,7 +1652,6 @@ test("content_unittests") {
     "../browser/service_worker/service_worker_dispatcher_host_unittest.cc",
     "../browser/service_worker/service_worker_installed_scripts_sender_unittest.cc",
     "../browser/service_worker/service_worker_job_unittest.cc",
-    "../browser/service_worker/service_worker_lifetime_tracker_unittest.cc",
     "../browser/service_worker/service_worker_metrics_unittest.cc",
     "../browser/service_worker/service_worker_navigation_loader_unittest.cc",
     "../browser/service_worker/service_worker_new_script_loader_unittest.cc",