Restore KeyedServiceFactory diagnostics for context use-after-destroy.

chrome/browser/engagement/site_engagement_service_unittest.cc

Index: chrome/browser/engagement/site_engagement_service_unittest.cc
diff --git a/chrome/browser/engagement/site_engagement_service_unittest.cc b/chrome/browser/engagement/site_engagement_service_unittest.cc
index 95c77a3c4986313a91977e8b00e3d7005526ae4e..96118989d39f428d318bc4ae7df155277fb2e452 100644
--- a/chrome/browser/engagement/site_engagement_service_unittest.cc
+++ b/chrome/browser/engagement/site_engagement_service_unittest.cc
@@ -156,6 +156,14 @@ class SiteEngagementServiceTest : public ChromeRenderViewHostTestHarness {
     SiteEngagementScore::SetParamValuesForTesting();
   }
 
+  void TearDown() override {
+    if (service_with_test_clock_) {
+      service_with_test_clock_->Shutdown();
+      service_with_test_clock_.reset();
+    }
+    ChromeRenderViewHostTestHarness::TearDown();
+  }
+
   void NavigateWithTransitionAndExpectHigherScore(
       SiteEngagementService* service,
       const GURL& url,
@@ -205,6 +213,15 @@ class SiteEngagementServiceTest : public ChromeRenderViewHostTestHarness {
     return score;
   }
 
+  void CreateServiceWithTestClock(SiteEngagementService** service,

[benwells, 2017/03/21 23:44:51]

Thanks for factoring out this too. We can delete all the setup from the tests
themselves by putting this function into the SetUp method, and having clock_ and
service_ fields on the test.

[sense (YandexTeam), 2017/03/22 07:38:21]

Okay, I think we can do this.

+                                  base::SimpleTestClock** clock) {
+    DCHECK(service && clock);
+    DCHECK(!service_with_test_clock_);
+    (*clock) = new base::SimpleTestClock();
+    (*service) = new SiteEngagementService(profile(), base::WrapUnique(*clock));
+    service_with_test_clock_ = base::WrapUnique(*service);
+  }
+
  private:
   void CheckScoreFromSettings(HostContentSettingsMap* settings_map,
                               const GURL& url,
@@ -213,6 +230,7 @@ class SiteEngagementServiceTest : public ChromeRenderViewHostTestHarness {
   }
 
   base::ScopedTempDir temp_dir_;
+  std::unique_ptr<SiteEngagementService> service_with_test_clock_;
 };
 
 TEST_F(SiteEngagementServiceTest, GetMedianEngagement) {
@@ -421,9 +439,9 @@ TEST_F(SiteEngagementServiceTest, RestrictedToHTTPAndHTTPS) {
 }
 
 TEST_F(SiteEngagementServiceTest, LastShortcutLaunch) {
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   base::HistogramTester histograms;
 
@@ -485,9 +503,9 @@ TEST_F(SiteEngagementServiceTest, LastShortcutLaunch) {
 }
 
 TEST_F(SiteEngagementServiceTest, NotificationPermission) {
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   GURL url1("https://www.google.com/");
   GURL url2("http://www.google.com/");
@@ -532,9 +550,9 @@ TEST_F(SiteEngagementServiceTest, NotificationPermission) {
 TEST_F(SiteEngagementServiceTest, CheckHistograms) {
   base::HistogramTester histograms;
 
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   base::Time current_day = GetReferenceTime();
   clock->SetNow(current_day);
@@ -822,9 +840,9 @@ TEST_F(SiteEngagementServiceTest, CheckHistograms) {
 // engagement times to be reset if too much time has passed since the last
 // engagement.
 TEST_F(SiteEngagementServiceTest, CleanupEngagementScores) {
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   // Set the base time to be 3 weeks past the stale period in the past.
   // Use a 1 second offset to make sure scores don't yet decay.
@@ -1042,9 +1060,9 @@ TEST_F(SiteEngagementServiceTest, CleanupEngagementScoresProportional) {
   SetParamValue(SiteEngagementScore::DECAY_POINTS, 0);
   SetParamValue(SiteEngagementScore::SCORE_CLEANUP_THRESHOLD, 0.5);
 
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   base::Time current_day = GetReferenceTime();
   clock->SetNow(current_day);
@@ -1100,9 +1118,9 @@ TEST_F(SiteEngagementServiceTest, NavigationAccumulation) {
 }
 
 TEST_F(SiteEngagementServiceTest, IsBootstrapped) {
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   base::Time current_day = GetReferenceTime();
   clock->SetNow(current_day);
@@ -1127,10 +1145,9 @@ TEST_F(SiteEngagementServiceTest, CleanupOriginsOnHistoryDeletion) {
   // balance out history deletion also accounts for the proportional decay.
   SetParamValue(SiteEngagementScore::DECAY_PROPORTION, 0.5);
 
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> engagement(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
-  ASSERT_TRUE(engagement.get());
+  SiteEngagementService* engagement;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&engagement, &clock);
 
   GURL origin1("http://www.google.com/");
   GURL origin1a("http://www.google.com/search?q=asdf");
@@ -1256,9 +1273,9 @@ TEST_F(SiteEngagementServiceTest, EngagementLevel) {
                     blink::mojom::EngagementLevel::MAX,
                 "enum values should not be equal");
 
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   base::Time current_day = GetReferenceTime();
   clock->SetNow(current_day);
@@ -1448,9 +1465,9 @@ TEST_F(SiteEngagementServiceTest, Observers) {
 }
 
 TEST_F(SiteEngagementServiceTest, ScoreDecayHistograms) {
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   base::Time current_day = GetReferenceTime();
   clock->SetNow(current_day);
@@ -1553,9 +1570,9 @@ TEST_F(SiteEngagementServiceTest, LastEngagementTime) {
 
   ASSERT_TRUE(last_engagement_time.is_null());
 
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
 
   ASSERT_TRUE(service->GetLastEngagementTime().is_null());
 
@@ -1607,9 +1624,9 @@ TEST_F(SiteEngagementServiceTest, LastEngagementTime) {
 }
 
 TEST_F(SiteEngagementServiceTest, CleanupMovesScoreBackToNow) {
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
   base::Time last_engagement_time;
 
   base::Time current_day = GetReferenceTime();
@@ -1677,9 +1694,9 @@ TEST_F(SiteEngagementServiceTest, CleanupMovesScoreBackToNow) {
 }
 
 TEST_F(SiteEngagementServiceTest, CleanupMovesScoreBackToRebase) {
-  base::SimpleTestClock* clock = new base::SimpleTestClock();
-  std::unique_ptr<SiteEngagementService> service(
-      new SiteEngagementService(profile(), base::WrapUnique(clock)));
+  SiteEngagementService* service;
+  base::SimpleTestClock* clock;
+  CreateServiceWithTestClock(&service, &clock);
   base::Time last_engagement_time;
 
   base::Time current_day = GetReferenceTime();