A small renaming to reduce coupling.

The QueueingTimeEstimator had a flag that was, effectively, a
disabled/enabled flag. Instead of naming it for the external state that
causes updates to the flag, name it for what the flag is used for inside
the QueueingTimeEstimator.

Bug: 883487
Change-Id: I73c7f71ff10fcacaf1e85a3f89f5e1a123b78896
Reviewed-on: https://chromium-review.googlesource.com/c/1296822
Commit-Queue: Tom McKee <tommckee@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603535}

third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc

@@ -215,7 +215,10 @@ MainThreadSchedulerImpl::MainThreadSchedulerImpl(
                               base::Unretained(this)),
           helper_.ControlMainThreadTaskQueue()->CreateTaskRunner(
               TaskType::kMainThreadTaskQueueControl)),
-      queueing_time_estimator_(this, kQueueingTimeWindowDuration, 20),
+      queueing_time_estimator_(this,
+                               kQueueingTimeWindowDuration,
+                               20,
+                               kLaunchingProcessIsBackgrounded),
       main_thread_only_(this,
                         compositor_task_queue_,
                         helper_.GetClock(),
@@ -995,7 +998,7 @@ void MainThreadSchedulerImpl::SetRendererBackgrounded(bool backgrounded) {
   internal::ProcessState::Get()->is_process_backgrounded = backgrounded;
 
   main_thread_only().background_status_changed_at = tick_clock()->NowTicks();
-  queueing_time_estimator_.OnRendererStateChanged(
+  queueing_time_estimator_.OnRecordingStateChanged(
       backgrounded, main_thread_only().background_status_changed_at);
 
   UpdatePolicy();

third_party/blink/renderer/platform/scheduler/main_thread/queueing_time_estimator.cc

@@ -4,12 +4,11 @@
 
 #include "third_party/blink/renderer/platform/scheduler/main_thread/queueing_time_estimator.h"
 
+#include <algorithm>
+
 #include "third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.h"
 #include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
 
-#include <algorithm>
-#include <map>
-
 namespace blink {
 namespace scheduler {
 
@@ -42,8 +41,8 @@ base::TimeDelta ExpectedQueueingTimeFromTask(base::TimeTicks task_start,
   DCHECK_LE(task_start, task_end);
   DCHECK_LE(task_start, step_end);
   DCHECK_LT(step_start, step_end);
-  // Because we skip steps when the renderer is backgrounded, we may have gone
-  // into the future, and in that case we ignore this task completely.
+  // Because we skip steps when disabled, we may have gone into the future, and
+  // in that case we ignore this task completely.
   if (task_end < step_start)
     return base::TimeDelta();
 
@@ -67,10 +66,11 @@ base::TimeDelta ExpectedQueueingTimeFromTask(base::TimeTicks task_start,
 
 QueueingTimeEstimator::QueueingTimeEstimator(Client* client,
                                              base::TimeDelta window_duration,
-                                             int steps_per_window)
+                                             int steps_per_window,
+                                             bool start_disabled)
     : client_(client),
       window_step_width_(window_duration / steps_per_window),
-      renderer_backgrounded_(kLaunchingProcessIsBackgrounded),
+      disabled_(start_disabled),
       calculator_(steps_per_window) {
   DCHECK_GE(steps_per_window, 1);
 }
@@ -91,13 +91,13 @@ void QueueingTimeEstimator::OnExecutionStopped(base::TimeTicks now) {
   busy_period_start_time_ = base::TimeTicks();
 }
 
-void QueueingTimeEstimator::OnRendererStateChanged(
-    bool backgrounded,
+void QueueingTimeEstimator::OnRecordingStateChanged(
+    bool disabled,
     base::TimeTicks transition_time) {
-  DCHECK_NE(backgrounded, renderer_backgrounded_);
+  DCHECK_NE(disabled, disabled_);
   if (!busy_)
     AdvanceTime(transition_time);
-  renderer_backgrounded_ = backgrounded;
+  disabled_ = disabled;
 }
 
 void QueueingTimeEstimator::AdvanceTime(base::TimeTicks current_time) {
@@ -110,11 +110,10 @@ void QueueingTimeEstimator::AdvanceTime(base::TimeTicks current_time) {
   }
   base::TimeTicks reference_time =
       busy_ ? busy_period_start_time_ : step_start_time_;
-  if (renderer_backgrounded_ ||
-      current_time - reference_time > kInvalidPeriodThreshold) {
-    // Skip steps when the renderer was backgrounded, when a task took too long,
-    // or when we remained idle for too long. May cause |step_start_time_| to go
-    // slightly into the future.
+  if (disabled_ || current_time - reference_time > kInvalidPeriodThreshold) {
+    // Skip steps when we're disabled, when a task took too long, or when we
+    // remained idle for too long. May cause |step_start_time_| to go slightly
+    // into the future.
     // TODO(npm): crbug.com/776013. Base skipping long tasks/idling on a signal
     // that we've been suspended.
     step_start_time_ =

third_party/blink/renderer/platform/scheduler/main_thread/queueing_time_estimator.h

@@ -5,15 +5,14 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_SCHEDULER_MAIN_THREAD_QUEUEING_TIME_ESTIMATOR_H_
 #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_SCHEDULER_MAIN_THREAD_QUEUEING_TIME_ESTIMATOR_H_
 
+#include <array>
+#include <vector>
+
 #include "base/macros.h"
 #include "base/time/time.h"
-#include "third_party/blink/public/common/page/launching_process_state.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
-#include "third_party/blink/renderer/platform/scheduler/main_thread/main_thread_metrics_helper.h"
 #include "third_party/blink/renderer/platform/scheduler/main_thread/main_thread_task_queue.h"
-
-#include <array>
-#include <vector>
+#include "third_party/blink/renderer/platform/scheduler/public/frame_status.h"
 
 namespace blink {
 namespace scheduler {
@@ -108,12 +107,12 @@ class PLATFORM_EXPORT QueueingTimeEstimator {
 
   QueueingTimeEstimator(Client* client,
                         base::TimeDelta window_duration,
-                        int steps_per_window);
+                        int steps_per_window,
+                        bool start_disabled);
 
   void OnExecutionStarted(base::TimeTicks now, MainThreadTaskQueue* queue);
   void OnExecutionStopped(base::TimeTicks now);
-  void OnRendererStateChanged(bool backgrounded,
-                              base::TimeTicks transition_time);
+  void OnRecordingStateChanged(bool disabled, base::TimeTicks transition_time);
 
  private:
   void AdvanceTime(base::TimeTicks current_time);
@@ -127,8 +126,8 @@ class PLATFORM_EXPORT QueueingTimeEstimator {
   bool busy_ = false;
   base::TimeTicks busy_period_start_time_;
 
-  // |renderer_backgrounded_| is the renderer's current status.
-  bool renderer_backgrounded_;
+  // |disabled_| is true iff we want to ignore start/stop events.
+  bool disabled_;
   Calculator calculator_;
 
   DISALLOW_ASSIGN(QueueingTimeEstimator);

third_party/blink/renderer/platform/scheduler/main_thread/queueing_time_estimator_unittest.cc

@@ -4,6 +4,10 @@
 
 #include "third_party/blink/renderer/platform/scheduler/main_thread/queueing_time_estimator.h"
 
+#include <memory>
+#include <string>
+#include <vector>
+
 #include "base/logging.h"
 #include "base/metrics/histogram_functions.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -14,10 +18,6 @@
 #include "third_party/blink/renderer/platform/scheduler/test/test_queueing_time_estimator_client.h"
 #include "third_party/blink/renderer/platform/testing/histogram_tester.h"
 
-#include <map>
-#include <string>
-#include <vector>
-
 namespace blink {
 namespace scheduler {
 
@@ -195,8 +195,8 @@ TEST_F(QueueingTimeEstimatorTest, IgnoreExtremelyLongTasks) {
                 fine_grained);
 }
 
-// If we idle for too long, ignore idling time, even if the renderer is on the
-// foreground. Perhaps the user's machine went to sleep while we were idling.
+// If we idle for too long, ignore idling time, even if the estimator is
+// enabled. Perhaps the user's machine went to sleep while we were idling.
 TEST_F(QueueingTimeEstimatorTest, IgnoreExtremelyLongIdlePeriods) {
   QueueingTimeEstimatorForTest estimator(
       &client, base::TimeDelta::FromSeconds(5), 1, time);
@@ -405,9 +405,9 @@ TEST_F(QueueingTimeEstimatorTest,
 }
 
 // There are multiple windows, but some of the EQTs are not reported due to
-// backgrounded renderer. EQT(win1) = 0. EQT(win3) = (1500+500)/2 = 1000.
+// enabling/disabling. EQT(win1) = 0. EQT(win3) = (1500+500)/2 = 1000.
 // EQT(win4) = 1/2*500/2 = 250. EQT(win7) = 1/5*200/2 = 20.
-TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithSingleStepPerWindow) {
+TEST_F(QueueingTimeEstimatorTest, DisabledEQTsWithSingleStepPerWindow) {
   QueueingTimeEstimatorForTest estimator(
       &client, base::TimeDelta::FromSeconds(1), 1, time);
   time += base::TimeDelta::FromMilliseconds(1000);
@@ -416,12 +416,12 @@ TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithSingleStepPerWindow) {
   time += base::TimeDelta::FromMilliseconds(1001);
 
   // Second window should not be reported.
-  estimator.OnRendererStateChanged(true, time);
+  estimator.OnRecordingStateChanged(true, time);
   estimator.OnExecutionStarted(time, nullptr);
   time += base::TimeDelta::FromMilliseconds(456);
   estimator.OnExecutionStopped(time);
   time += base::TimeDelta::FromMilliseconds(200);
-  estimator.OnRendererStateChanged(false, time);
+  estimator.OnRecordingStateChanged(false, time);
   time += base::TimeDelta::FromMilliseconds(343);
 
   // Third, fourth windows should be reported
@@ -434,9 +434,9 @@ TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithSingleStepPerWindow) {
   estimator.OnExecutionStarted(time, nullptr);
   time += base::TimeDelta::FromMilliseconds(800);
   estimator.OnExecutionStopped(time);
-  estimator.OnRendererStateChanged(true, time);
+  estimator.OnRecordingStateChanged(true, time);
   time += base::TimeDelta::FromMilliseconds(200);
-  estimator.OnRendererStateChanged(false, time);
+  estimator.OnRecordingStateChanged(false, time);
   estimator.OnExecutionStarted(time, nullptr);
   time += base::TimeDelta::FromMilliseconds(999);
 
@@ -461,9 +461,9 @@ TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithSingleStepPerWindow) {
                 fine_grained);
 }
 
-// We only ignore steps that contain some part that is backgrounded. Thus a
-// window could be made up of non-contiguous steps. The following are EQTs, with
-// time deltas with respect to the end of the first, 0-time task:
+// We only ignore steps that contain some time span that is disabled. Thus a
+// window could be made up of non-contiguous steps. The following are EQTs,
+// with time deltas with respect to the end of the first, 0-time task:
 // Win1: [0-1000]. EQT of step [0-1000]: 500/2*1/2 = 125. EQT(win1) = 125/5 =
 // 25.
 // Win2: [0-1000],[2000-3000]. EQT of [2000-3000]: (1000+200)/2*4/5 = 480.
@@ -480,7 +480,7 @@ TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithSingleStepPerWindow) {
 // EQT(win7) = (0+145+900+80+1680)/5 = 561.
 // Win8: [12000-17000]. EQT of [16000-17000]: (1700+700)/2 = 1200. EQT(win8) =
 // (145+900+80+1680+1200)/5 = 801.
-TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithMutipleStepsPerWindow) {
+TEST_F(QueueingTimeEstimatorTest, DisabledEQTsWithMutipleStepsPerWindow) {
   QueueingTimeEstimatorForTest estimator(
       &client, base::TimeDelta::FromSeconds(5), 5, time);
   time += base::TimeDelta::FromMilliseconds(5000);
@@ -492,12 +492,12 @@ TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithMutipleStepsPerWindow) {
   time += base::TimeDelta::FromMilliseconds(500);
   estimator.OnExecutionStopped(time);
 
-  estimator.OnRendererStateChanged(true, time);
+  estimator.OnRecordingStateChanged(true, time);
   // This task should be ignored.
   estimator.OnExecutionStarted(time, nullptr);
   time += base::TimeDelta::FromMilliseconds(800);
   estimator.OnExecutionStopped(time);
-  estimator.OnRendererStateChanged(false, time);
+  estimator.OnRecordingStateChanged(false, time);
 
   time += base::TimeDelta::FromMilliseconds(400);
   estimator.OnExecutionStarted(time, nullptr);
@@ -505,7 +505,7 @@ TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithMutipleStepsPerWindow) {
   estimator.OnExecutionStopped(time);
 
   time += base::TimeDelta::FromMilliseconds(300);
-  estimator.OnRendererStateChanged(true, time);
+  estimator.OnRecordingStateChanged(true, time);
   time += base::TimeDelta::FromMilliseconds(2000);
   // These tasks should be ignored.
   estimator.OnExecutionStarted(time, nullptr);
@@ -514,7 +514,7 @@ TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithMutipleStepsPerWindow) {
   estimator.OnExecutionStarted(time, nullptr);
   time += base::TimeDelta::FromMilliseconds(3400);
   estimator.OnExecutionStopped(time);
-  estimator.OnRendererStateChanged(false, time);
+  estimator.OnRecordingStateChanged(false, time);
 
   time += base::TimeDelta::FromMilliseconds(2000);
   estimator.OnExecutionStarted(time, nullptr);
@@ -527,7 +527,7 @@ TEST_F(QueueingTimeEstimatorTest, BackgroundedEQTsWithMutipleStepsPerWindow) {
   estimator.OnExecutionStopped(time);
 
   // Window with last step should not be reported.
-  estimator.OnRendererStateChanged(true, time);
+  estimator.OnRecordingStateChanged(true, time);
   time += base::TimeDelta::FromMilliseconds(1000);
   estimator.OnExecutionStarted(time, nullptr);
   estimator.OnExecutionStopped(time);

third_party/blink/renderer/platform/scheduler/test/test_queueing_time_estimator_client.cc

@@ -7,6 +7,7 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/safe_conversions.h"
+#include "third_party/blink/public/common/page/launching_process_state.h"
 #include "third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h"
 
 namespace blink {
@@ -140,10 +141,13 @@ QueueingTimeEstimatorForTest::QueueingTimeEstimatorForTest(
     base::TimeDelta window_duration,
     int steps_per_window,
     base::TimeTicks time)
-    : QueueingTimeEstimator(client, window_duration, steps_per_window) {
-  // If initial state is not foregrounded, foreground.
+    : QueueingTimeEstimator(client,
+                            window_duration,
+                            steps_per_window,
+                            kLaunchingProcessIsBackgrounded) {
+  // If initial state is disabled, enable the estimator.
   if (kLaunchingProcessIsBackgrounded) {
-    this->OnRendererStateChanged(false, time);
+    this->OnRecordingStateChanged(false, time);
   }
 }