Revert of Plumb activation time to main (patchset #16 id:280001 of...

Revert of Plumb activation time to main (patchset #16 id:280001 of https://codereview.chromium.org/2778223005/ )

Reason for revert:
Failure observed in LayerTreeHostProxyTestActivationTime.RunMultiThread_DelegatingRenderer

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.win%2FWin7_Tests__dbg__1_%2F59244%2F%2B%2Frecipes%2Fsteps%2Fcc_unittests%2F0%2Flogs%2FLayerTreeHostProxyTestActivationTime.RunMultiThread_DelegatingRenderer%2F0

Original issue's description:
> Plumb activation time in cc to Blink Scheduler in Main
>
> BUG=657826,657825
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Review-Url: https://codereview.chromium.org/2778223005
> Cr-Commit-Position: refs/heads/master@{#465701}
> Committed: https://chromium.googlesource.com/chromium/src/+/9a2eb472fde51de48c095c9c208101c534ac211e

TBR=brianderson@chromium.org,ccameron@chromium.org,alexclarke@chromium.org,miguelg@chromium.org,tedchoc@chromium.org,panicker@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=657826,657825

Review-Url: https://codereview.chromium.org/2833603002
Cr-Commit-Position: refs/heads/master@{#465757}

cc/output/begin_frame_args.cc

@@ -25,10 +25,6 @@ const char* BeginFrameArgs::TypeToString(BeginFrameArgsType type) {
 
 constexpr uint64_t BeginFrameArgs::kInvalidFrameNumber;
 constexpr uint64_t BeginFrameArgs::kStartingFrameNumber;
-constexpr uint32_t BeginFrameArgs::kDefaultSourceFrameNumber;
-
-BeginFrameArgs::BeginFrameArgs(const BeginFrameArgs& other) = default;
-BeginFrameArgs::~BeginFrameArgs() = default;
 
 BeginFrameArgs::BeginFrameArgs()
     : frame_time(base::TimeTicks()),
@@ -41,7 +37,6 @@ BeginFrameArgs::BeginFrameArgs()
 
 BeginFrameArgs::BeginFrameArgs(uint32_t source_id,
                                uint64_t sequence_number,
-                               uint32_t source_frame_number,
                                base::TimeTicks frame_time,
                                base::TimeTicks deadline,
                                base::TimeDelta interval,
@@ -59,7 +54,6 @@ BeginFrameArgs::BeginFrameArgs(uint32_t source_id,
 BeginFrameArgs BeginFrameArgs::Create(BeginFrameArgs::CreationLocation location,
                                       uint32_t source_id,
                                       uint64_t sequence_number,
-                                      uint32_t source_frame_number,
                                       base::TimeTicks frame_time,
                                       base::TimeTicks deadline,
                                       base::TimeDelta interval,
@@ -67,12 +61,11 @@ BeginFrameArgs BeginFrameArgs::Create(BeginFrameArgs::CreationLocation location,
   DCHECK_NE(type, BeginFrameArgs::INVALID);
   DCHECK_NE(type, BeginFrameArgs::BEGIN_FRAME_ARGS_TYPE_MAX);
 #ifdef NDEBUG
-  return BeginFrameArgs(source_id, sequence_number, source_frame_number,
-                        frame_time, deadline, interval, type);
+  return BeginFrameArgs(source_id, sequence_number, frame_time, deadline,
+                        interval, type);
 #else
-  BeginFrameArgs args =
-      BeginFrameArgs(source_id, sequence_number, source_frame_number,
-                     frame_time, deadline, interval, type);
+  BeginFrameArgs args = BeginFrameArgs(source_id, sequence_number, frame_time,
+                                       deadline, interval, type);
   args.created_from = location;
   return args;
 #endif

cc/output/begin_frame_args.h

@@ -59,11 +59,8 @@ struct CC_EXPORT BeginFrameArgs {
   static constexpr uint64_t kInvalidFrameNumber = 0;
   static constexpr uint64_t kStartingFrameNumber = 1;
 
-  static constexpr uint32_t kDefaultSourceFrameNumber = 0;
-
   // Creates an invalid set of values.
   BeginFrameArgs();
-  ~BeginFrameArgs();
 
 #ifdef NDEBUG
   typedef const void* CreationLocation;
@@ -78,12 +75,10 @@ struct CC_EXPORT BeginFrameArgs {
   static BeginFrameArgs Create(CreationLocation location,
                                uint32_t source_id,
                                uint64_t sequence_number,
-                               uint32_t source_frame_number,
                                base::TimeTicks frame_time,
                                base::TimeTicks deadline,
                                base::TimeDelta interval,
                                BeginFrameArgsType type);
-  BeginFrameArgs(const BeginFrameArgs&);
 
   // This is the default delta that will be used to adjust the deadline when
   // proper draw-time estimations are not yet available.
@@ -109,26 +104,12 @@ struct CC_EXPORT BeginFrameArgs {
   uint64_t sequence_number;
   uint32_t source_id;  // |source_id| after |sequence_number| for packing.
 
-  // |source_frame_number| is set by the LayerTreeHost when the BeginFrame
-  // reaches blink and corresponds to the commit count.
-  uint32_t source_frame_number;
-
-  // This is to report Activation (ReadyToActivate signal) time back to main.
-  // A vector of pairs of source_frame_number and timestamp is captured, and
-  // sent back with the next ProxyMain::BeginMainFrame, and from there routed to
-  // the Blink Scheduler. A vector is needed as there can be multiple
-  // ReadyToActivate signals before the next BeginMainFrame.
-  // TODO(panicker): Sub-class BeginFrameArgs as BeginMainFrameArgs and move
-  // this there.
-  std::vector<std::pair<uint32_t, base::TimeTicks>> ready_to_activate_time;
-
   BeginFrameArgsType type;
   bool on_critical_path;
 
  private:
   BeginFrameArgs(uint32_t source_id,
                  uint64_t sequence_number,
-                 uint32_t source_frame_number,
                  base::TimeTicks frame_time,
                  base::TimeTicks deadline,
                  base::TimeDelta interval,

cc/output/begin_frame_args_unittest.cc

@@ -97,8 +97,7 @@ TEST(BeginFrameArgsTest, Create) {
   EXPECT_TRUE(args1.on_critical_path);
 
   BeginFrameArgs args2 = BeginFrameArgs::Create(
-      BEGINFRAME_FROM_HERE, 123, 10, BeginFrameArgs::kDefaultSourceFrameNumber,
-      base::TimeTicks::FromInternalValue(1),
+      BEGINFRAME_FROM_HERE, 123, 10, base::TimeTicks::FromInternalValue(1),
       base::TimeTicks::FromInternalValue(2),
       base::TimeDelta::FromInternalValue(3), BeginFrameArgs::NORMAL);
   EXPECT_TRUE(args2.IsValid()) << args2;

cc/scheduler/begin_frame_source.cc

@@ -129,8 +129,7 @@ void BackToBackBeginFrameSource::OnTimerTick() {
   base::TimeTicks frame_time = time_source_->LastTickTime();
   base::TimeDelta default_interval = BeginFrameArgs::DefaultInterval();
   BeginFrameArgs args = BeginFrameArgs::Create(
-      BEGINFRAME_FROM_HERE, source_id(), next_sequence_number_,
-      BeginFrameArgs::kDefaultSourceFrameNumber, frame_time,
+      BEGINFRAME_FROM_HERE, source_id(), next_sequence_number_, frame_time,
       frame_time + default_interval, default_interval, BeginFrameArgs::NORMAL);
   next_sequence_number_++;
 
@@ -179,8 +178,7 @@ BeginFrameArgs DelayBasedBeginFrameSource::CreateBeginFrameArgs(
     BeginFrameArgs::BeginFrameArgsType type) {
   uint64_t sequence_number = next_sequence_number_++;
   return BeginFrameArgs::Create(
-      BEGINFRAME_FROM_HERE, source_id(), sequence_number,
-      BeginFrameArgs::kDefaultSourceFrameNumber, frame_time,
+      BEGINFRAME_FROM_HERE, source_id(), sequence_number, frame_time,
       time_source_->NextTickTime(), time_source_->Interval(), type);
 }
 

cc/scheduler/scheduler.cc

@@ -82,18 +82,9 @@ void Scheduler::SetCanDraw(bool can_draw) {
   ProcessScheduledActions();
 }
 
-void Scheduler::NotifyReadyToActivate(int source_frame_number) {
+void Scheduler::NotifyReadyToActivate() {
   compositor_timing_history_->ReadyToActivate();
   state_machine_.NotifyReadyToActivate();
-  if (source_frame_number != -1) {
-    if (ready_to_activate_time_.empty() ||
-        ready_to_activate_time_.back().first !=
-            static_cast<unsigned>(source_frame_number)) {
-      // Note the activate source-frame and timestamp
-      ready_to_activate_time_.emplace_back(source_frame_number,
-                                           base::TimeTicks::Now());
-    }
-  }
   ProcessScheduledActions();
 }
 
@@ -616,6 +607,7 @@ void Scheduler::ProcessScheduledActions() {
   // The top-level call will iteratively execute the next action for us anyway.
   if (inside_process_scheduled_actions_)
     return;
+
   base::AutoReset<bool> mark_inside(&inside_process_scheduled_actions_, true);
 
   SchedulerStateMachine::Action action;
@@ -634,9 +626,6 @@ void Scheduler::ProcessScheduledActions() {
             begin_main_frame_args_.frame_time);
         state_machine_.WillSendBeginMainFrame();
         // TODO(brianderson): Pass begin_main_frame_args_ directly to client.
-        begin_main_frame_args_.ready_to_activate_time =
-            std::move(ready_to_activate_time_);
-        ready_to_activate_time_.clear();
         client_->ScheduledActionSendBeginMainFrame(begin_main_frame_args_);
         break;
       case SchedulerStateMachine::ACTION_COMMIT: {

cc/scheduler/scheduler.h

@@ -77,7 +77,7 @@ class CC_EXPORT Scheduler : public BeginFrameObserverBase {
   void SetVisible(bool visible);
   bool visible() { return state_machine_.visible(); }
   void SetCanDraw(bool can_draw);
-  void NotifyReadyToActivate(int source_frame_number);
+  void NotifyReadyToActivate();
   void NotifyReadyToDraw();
   void SetBeginFrameSource(BeginFrameSource* source);
 
@@ -191,13 +191,6 @@ class CC_EXPORT Scheduler : public BeginFrameObserverBase {
 
   bool stopped_ = false;
 
-  // This is to report Activation (ReadyToActivate signal) time back to Main.
-  // A vector of pairs of source_frame_number and timestamp is captured, and
-  // sent back in BeginFrameArgs with the next ProxyMain::BeginMainFrame. A
-  // vector is needed as there can be multiple ReadyToActivate signals before
-  // the next BeginMainFrame.
-  std::vector<std::pair<uint32_t, base::TimeTicks>> ready_to_activate_time_;
-
  private:
   void ScheduleBeginImplFrameDeadline();
   void ScheduleBeginImplFrameDeadlineIfNeeded();

cc/test/begin_frame_args_test.cc

@@ -25,8 +25,7 @@ BeginFrameArgs CreateBeginFrameArgsForTesting(
     uint64_t sequence_number,
     base::TimeTicks frame_time) {
   return BeginFrameArgs::Create(
-      location, source_id, sequence_number,
-      BeginFrameArgs::kDefaultSourceFrameNumber, frame_time,
+      location, source_id, sequence_number, frame_time,
       frame_time + BeginFrameArgs::DefaultInterval() -
           BeginFrameArgs::DefaultEstimatedParentDrawTime(),
       BeginFrameArgs::DefaultInterval(), BeginFrameArgs::NORMAL);
@@ -40,7 +39,6 @@ BeginFrameArgs CreateBeginFrameArgsForTesting(
     int64_t deadline,
     int64_t interval) {
   return BeginFrameArgs::Create(location, source_id, sequence_number,
-                                BeginFrameArgs::kDefaultSourceFrameNumber,
                                 base::TimeTicks::FromInternalValue(frame_time),
                                 base::TimeTicks::FromInternalValue(deadline),
                                 base::TimeDelta::FromInternalValue(interval),
@@ -56,7 +54,6 @@ BeginFrameArgs CreateBeginFrameArgsForTesting(
     int64_t interval,
     BeginFrameArgs::BeginFrameArgsType type) {
   return BeginFrameArgs::Create(location, source_id, sequence_number,
-                                BeginFrameArgs::kDefaultSourceFrameNumber,
                                 base::TimeTicks::FromInternalValue(frame_time),
                                 base::TimeTicks::FromInternalValue(deadline),
                                 base::TimeDelta::FromInternalValue(interval),
@@ -70,8 +67,7 @@ BeginFrameArgs CreateBeginFrameArgsForTesting(
     base::SimpleTestTickClock* now_src) {
   base::TimeTicks now = now_src->NowTicks();
   return BeginFrameArgs::Create(
-      location, source_id, sequence_number,
-      BeginFrameArgs::kDefaultSourceFrameNumber, now,
+      location, source_id, sequence_number, now,
       now + BeginFrameArgs::DefaultInterval() -
           BeginFrameArgs::DefaultEstimatedParentDrawTime(),
       BeginFrameArgs::DefaultInterval(), BeginFrameArgs::NORMAL);

cc/trees/layer_tree_host_unittest_proxy.cc

@@ -420,79 +420,4 @@ class LayerTreeHostProxyTestCommitWaitsForActivationMFBA
 
 MULTI_THREAD_TEST_F(LayerTreeHostProxyTestCommitWaitsForActivationMFBA);
 
-// Test for activation time passed in BeginFrameArgs.
-class LayerTreeHostProxyTestActivationTime : public LayerTreeHostProxyTest {
- protected:
-  LayerTreeHostProxyTestActivationTime() = default;
-
-  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
-
-  void InitializeSettings(LayerTreeSettings* settings) override {
-    settings->main_frame_before_activation_enabled = true;
-    LayerTreeHostProxyTest::InitializeSettings(settings);
-  }
-
-  void DidSendBeginMainFrameOnThread(LayerTreeHostImpl* impl) override {
-    if (activate_blocked_) {
-      impl->BlockNotifyReadyToActivateForTesting(false);
-      {
-        base::AutoLock hold(activate_blocked_lock_);
-        activate_blocked_ = false;
-      }
-    }
-  }
-
-  void BeginMainFrame(const BeginFrameArgs& args) override {
-    switch (args.source_frame_number) {
-      case 0:
-        // Initial frame, no activation yet.
-        EXPECT_EQ(0UL, args.ready_to_activate_time.size());
-        break;
-      case 1:
-        // Frame #1, activation for Frame #0.
-        EXPECT_EQ(1UL, args.ready_to_activate_time.size());
-        EXPECT_EQ(0UL, args.ready_to_activate_time[0].first);
-        break;
-      case 2:
-        // Frame #2, activation is delayed in BeginCommitOnThread.
-        EXPECT_EQ(0UL, args.ready_to_activate_time.size());
-        break;
-      case 3:
-        // Frame #3, received activation for Frame #1 & #2.
-        EXPECT_EQ(2UL, args.ready_to_activate_time.size());
-        EXPECT_EQ(1UL, args.ready_to_activate_time[0].first);
-        EXPECT_EQ(2UL, args.ready_to_activate_time[1].first);
-        EndTest();
-    }
-  }
-
-  void DidActivateTreeOnThread(LayerTreeHostImpl* impl) override {
-    if (impl->sync_tree()->source_frame_number() == 0 ||
-        impl->sync_tree()->source_frame_number() == 2)
-      PostSetNeedsCommitToMainThread();  // invoke for Frame #3
-  }
-
-  void CommitCompleteOnThread(LayerTreeHostImpl* impl) override {
-    // Block activation for Frame #1
-    if (impl->sync_tree()->source_frame_number() == 1) {
-      impl->BlockNotifyReadyToActivateForTesting(true);
-      PostSetNeedsCommitToMainThread();
-      {
-        base::AutoLock hold(activate_blocked_lock_);
-        activate_blocked_ = true;
-      }
-    }
-  }
-
-  void AfterTest() override {}
-
- private:
-  base::Lock activate_blocked_lock_;
-  bool activate_blocked_ = false;
-
-  DISALLOW_COPY_AND_ASSIGN(LayerTreeHostProxyTestActivationTime);
-};
-
-MULTI_THREAD_TEST_F(LayerTreeHostProxyTestActivationTime);
-
 }  // namespace cc

cc/trees/proxy_impl.cc

@@ -329,10 +329,7 @@ void ProxyImpl::OnCanDrawStateChanged(bool can_draw) {
 void ProxyImpl::NotifyReadyToActivate() {
   TRACE_EVENT0("cc", "ProxyImpl::NotifyReadyToActivate");
   DCHECK(IsImplThread());
-  scheduler_->NotifyReadyToActivate(
-      layer_tree_host_impl_->pending_tree()
-          ? layer_tree_host_impl_->pending_tree()->source_frame_number()
-          : -1);
+  scheduler_->NotifyReadyToActivate();
 }
 
 void ProxyImpl::NotifyReadyToDraw() {

cc/trees/proxy_main.cc

@@ -119,9 +119,6 @@ void ProxyMain::BeginMainFrame(
       benchmark_instrumentation::kDoBeginFrame,
       begin_main_frame_state->begin_frame_id);
 
-  begin_main_frame_state->begin_frame_args.source_frame_number =
-      layer_tree_host_->SourceFrameNumber();
-
   base::TimeTicks begin_main_frame_start_time = base::TimeTicks::Now();
 
   TRACE_EVENT_SYNTHETIC_DELAY_BEGIN("cc.BeginMainFrame");

cc/trees/single_thread_proxy.cc

@@ -322,11 +322,8 @@ void SingleThreadProxy::OnCanDrawStateChanged(bool can_draw) {
 void SingleThreadProxy::NotifyReadyToActivate() {
   TRACE_EVENT0("cc", "SingleThreadProxy::NotifyReadyToActivate");
   DebugScopedSetImplThread impl(task_runner_provider_);
-  if (scheduler_on_impl_thread_) {
-    // The argument to NotifyReadyToActivate is the pending-tree for computing
-    // Activation time in ProxyImpl. However this is not applicable here.
-    scheduler_on_impl_thread_->NotifyReadyToActivate(-1);
-  }
+  if (scheduler_on_impl_thread_)
+    scheduler_on_impl_thread_->NotifyReadyToActivate();
 }
 
 void SingleThreadProxy::NotifyReadyToDraw() {
@@ -467,8 +464,7 @@ void SingleThreadProxy::CompositeImmediately(base::TimeTicks frame_begin_time) {
 
   BeginFrameArgs begin_frame_args(BeginFrameArgs::Create(
       BEGINFRAME_FROM_HERE, BeginFrameArgs::kManualSourceId, 1,
-      BeginFrameArgs::kDefaultSourceFrameNumber, frame_begin_time,
-      base::TimeTicks(), BeginFrameArgs::DefaultInterval(),
+      frame_begin_time, base::TimeTicks(), BeginFrameArgs::DefaultInterval(),
       BeginFrameArgs::NORMAL));
 
   // Start the impl frame.

content/browser/compositor/gpu_vsync_begin_frame_source.cc

@@ -30,9 +30,8 @@ void GpuVSyncBeginFrameSource::OnVSync(base::TimeTicks timestamp,
 
   next_sequence_number_++;
   OnBeginFrame(cc::BeginFrameArgs::Create(
-      BEGINFRAME_FROM_HERE, source_id(), next_sequence_number_,
-      cc::BeginFrameArgs::kDefaultSourceFrameNumber, timestamp, deadline,
-      interval, cc::BeginFrameArgs::NORMAL));
+      BEGINFRAME_FROM_HERE, source_id(), next_sequence_number_, timestamp,
+      deadline, interval, cc::BeginFrameArgs::NORMAL));
 }
 
 void GpuVSyncBeginFrameSource::OnNeedsBeginFrames(bool needs_begin_frames) {

ui/android/window_android.cc

@@ -102,9 +102,8 @@ void WindowAndroid::WindowBeginFrameSource::OnVSync(
   // frame time is in the past, so give the next vsync period as the deadline.
   base::TimeTicks deadline = frame_time + vsync_period;
   last_begin_frame_args_ = cc::BeginFrameArgs::Create(
-      BEGINFRAME_FROM_HERE, source_id(), next_sequence_number_,
-      cc::BeginFrameArgs::kDefaultSourceFrameNumber, frame_time, deadline,
-      vsync_period, cc::BeginFrameArgs::NORMAL);
+      BEGINFRAME_FROM_HERE, source_id(), next_sequence_number_, frame_time,
+      deadline, vsync_period, cc::BeginFrameArgs::NORMAL);
   DCHECK(last_begin_frame_args_.IsValid());
   next_sequence_number_++;