RC: Flush the total obs time to the proto for the chrome://discards UI

Change-Id: If43a304a2deee1abc05df6c62344f538dd8b1de1
Reviewed-on: https://chromium-review.googlesource.com/1217447Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592152}

chrome/browser/resource_coordinator/local_site_characteristics_data_impl.cc

@@ -380,20 +380,13 @@ void LocalSiteCharacteristicsDataImpl::DecrementNumLoadedBackgroundTabs() {
   loaded_tabs_in_background_count_--;
   // Only update the observation durations if there's no more backgounded
   // instance of this origin.
-  if (loaded_tabs_in_background_count_ > 0U)
-    return;
-
-  DCHECK(!background_session_begin_.is_null());
-  base::TimeDelta extra_observation_duration =
-      NowTicks() - background_session_begin_;
-
-  // Update the observation duration fields.
-  for (auto* iter : GetAllFeaturesFromProto(&site_characteristics_))
-    IncrementFeatureObservationDuration(iter, extra_observation_duration);
+  if (loaded_tabs_in_background_count_ == 0U)
+    FlushFeaturesObservationDurationToProto();
 }
 
 const SiteCharacteristicsProto&
 LocalSiteCharacteristicsDataImpl::FlushStateToProto() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // Update the proto with the most current performance measurement averages.
   if (cpu_usage_estimate_.num_datums() ||
       private_footprint_kb_estimate_.num_datums()) {
@@ -407,8 +400,26 @@ LocalSiteCharacteristicsDataImpl::FlushStateToProto() {
     }
   }
 
+  if (loaded_tabs_in_background_count_ > 0U)
+    FlushFeaturesObservationDurationToProto();
+
   return site_characteristics_;
 }
 
+void LocalSiteCharacteristicsDataImpl::
+    FlushFeaturesObservationDurationToProto() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(!background_session_begin_.is_null());
+
+  base::TimeTicks now = NowTicks();
+
+  base::TimeDelta extra_observation_duration = now - background_session_begin_;
+  background_session_begin_ = now;
+
+  // Update the observation duration fields.
+  for (auto* iter : GetAllFeaturesFromProto(&site_characteristics_))
+    IncrementFeatureObservationDuration(iter, extra_observation_duration);
+}
+
 }  // namespace internal
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h

@@ -176,6 +176,8 @@ class LocalSiteCharacteristicsDataImpl
   FRIEND_TEST_ALL_PREFIXES(
       resource_coordinator::LocalSiteCharacteristicsDataReaderTest,
       FreeingReaderDoesntCauseWriteOperation);
+  FRIEND_TEST_ALL_PREFIXES(LocalSiteCharacteristicsDataImplTest,
+                           FlushingStateToProtoDoesntAffectData);
 
   // Add |extra_observation_duration| to the observation window of a given
   // feature if it hasn't been used yet, do nothing otherwise.
@@ -211,6 +213,10 @@ class LocalSiteCharacteristicsDataImpl
   // Flush any state that's maintained in member variables to the proto.
   const SiteCharacteristicsProto& FlushStateToProto();
 
+  // Updates the proto with the current total observation duration and updates
+  // |background_session_begin_| to NowTicks().
+  void FlushFeaturesObservationDurationToProto();
+
   // This site's characteristics, contains the features and other values are
   // measured.
   SiteCharacteristicsProto site_characteristics_;

chrome/browser/resource_coordinator/local_site_characteristics_data_impl_unittest.cc

@@ -136,6 +136,7 @@ class LocalSiteCharacteristicsDataImplTest : public ::testing::Test {
   }
 
   const url::Origin kDummyOrigin = url::Origin::Create(GURL("foo.com"));
+  const url::Origin kDummyOrigin2 = url::Origin::Create(GURL("bar.com"));
 
   base::SimpleTestTickClock test_clock_;
   ScopedSetTickClockForTesting scoped_set_tick_clock_for_testing_;
@@ -653,5 +654,47 @@ TEST_F(LocalSiteCharacteristicsDataImplTest,
   ::testing::Mock::VerifyAndClear(&mock_db);
 }
 
+TEST_F(LocalSiteCharacteristicsDataImplTest,
+       FlushingStateToProtoDoesntAffectData) {
+  // Create 2 DataImpl object and do the same operations on them, ensures that
+  // calling FlushStateToProto doesn't affect the data that gets recorded.
+
+  auto local_site_data =
+      GetDataImpl(kDummyOrigin, &destroy_delegate_, &database_);
+  auto local_site_data_ref =
+      GetDataImpl(kDummyOrigin2, &destroy_delegate_, &database_);
+
+  local_site_data->NotifySiteLoaded();
+  local_site_data->NotifyLoadedSiteBackgrounded();
+  local_site_data_ref->NotifySiteLoaded();
+  local_site_data_ref->NotifyLoadedSiteBackgrounded();
+
+  test_clock_.Advance(base::TimeDelta::FromSeconds(15));
+  local_site_data->FlushStateToProto();
+  test_clock_.Advance(base::TimeDelta::FromSeconds(15));
+
+  local_site_data->NotifyUsesAudioInBackground();
+  local_site_data_ref->NotifyUsesAudioInBackground();
+
+  local_site_data->FlushStateToProto();
+
+  EXPECT_EQ(local_site_data->FeatureObservationTimestamp(
+                local_site_data->site_characteristics_for_testing()
+                    .uses_audio_in_background()),
+            local_site_data_ref->FeatureObservationTimestamp(
+                local_site_data_ref->site_characteristics_for_testing()
+                    .uses_audio_in_background()));
+
+  EXPECT_EQ(local_site_data->FeatureObservationDuration(
+                local_site_data->site_characteristics_for_testing()
+                    .updates_title_in_background()),
+            local_site_data_ref->FeatureObservationDuration(
+                local_site_data_ref->site_characteristics_for_testing()
+                    .updates_title_in_background()));
+
+  local_site_data->NotifySiteUnloaded(TabVisibility::kBackground);
+  local_site_data_ref->NotifySiteUnloaded(TabVisibility::kBackground);
+}
+
 }  // namespace internal
 }  // namespace resource_coordinator