cc: Fix overwriting of commit completion event in MFBA mode.

cc/trees/proxy_impl.cc

 // Copyright 2015 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 "cc/trees/proxy_impl.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 33 matching lines @@
       new ProxyImpl(channel_impl, layer_tree_host, task_runner_provider,
                     std::move(external_begin_frame_source)));
 }
 
 ProxyImpl::ProxyImpl(
     ChannelImpl* channel_impl,
     LayerTreeHost* layer_tree_host,
     TaskRunnerProvider* task_runner_provider,
     std::unique_ptr<BeginFrameSource> external_begin_frame_source)
     : layer_tree_host_id_(layer_tree_host->id()),
-      next_commit_waits_for_activation_(false),
+      commit_completion_waits_for_activation_(false),
       commit_completion_event_(nullptr),
+      activation_completion_event_(nullptr),
       next_frame_is_newly_committed_frame_(false),
       inside_draw_(false),
       input_throttled_until_commit_(false),
       task_runner_provider_(task_runner_provider),
       smoothness_priority_expiration_notifier_(
           task_runner_provider->ImplThreadTaskRunner(),
           base::Bind(&ProxyImpl::RenewTreePriority, base::Unretained(this)),
           base::TimeDelta::FromSecondsD(
               kSmoothnessTakesPriorityExpirationDelay)),
       external_begin_frame_source_(std::move(external_begin_frame_source)),
@@ skipping 168 matching lines @@
     bool* main_frame_will_happen) {
   DCHECK(IsImplThread());
   if (layer_tree_host_impl_->output_surface()) {
     *main_frame_will_happen = scheduler_->MainFrameForTestingWillHappen();
   } else {
     *main_frame_will_happen = false;
   }
   completion->Signal();
 }
 
+void ProxyImpl::BlockNotifyReadyToActivateForTesting(bool block) {
+  DCHECK(IsImplThread());
+  layer_tree_host_impl_->BlockNotifyReadyToActivateForTesting(block);
+}
+
 void ProxyImpl::StartCommitOnImpl(CompletionEvent* completion,
                                   LayerTreeHost* layer_tree_host,
                                   base::TimeTicks main_thread_start_time,
                                   bool hold_commit_for_activation) {
   TRACE_EVENT0("cc", "ProxyImpl::StartCommitOnImplThread");
   DCHECK(!commit_completion_event_);
   DCHECK(IsImplThread() && IsMainThreadBlocked());
   DCHECK(scheduler_);
   DCHECK(scheduler_->CommitPending());
 
-  if (hold_commit_for_activation) {
-    // This commit may be aborted. Store the value for
-    // hold_commit_for_activation so that whenever the next commit is started,
-    // the main thread will be unblocked only after pending tree activation.
-    next_commit_waits_for_activation_ = hold_commit_for_activation;
-  }
-
   if (!layer_tree_host_impl_) {
     TRACE_EVENT_INSTANT0("cc", "EarlyOut_NoLayerTree",
                          TRACE_EVENT_SCOPE_THREAD);
     completion->Signal();
     return;
   }
 
   // Ideally, we should inform to impl thread when BeginMainFrame is started.
   // But, we can avoid a PostTask in here.
   scheduler_->NotifyBeginMainFrameStarted(main_thread_start_time);
+
   commit_completion_event_ = completion;
+  // This commit may be aborted. Store the value for hold_commit_for_activation

[enne (OOO), 2016/05/20 18:26:12]

Is this comment true? It seems like one you get past that EarlyOut_NoLayerTree,
you're going to get to ScheduledActionCommit? Maybe the comment can just go
away?

[sunnyps, 2016/05/20 20:49:36]

Done.

+  // so that whenever the next commit is started, the main thread will be
+  // unblocked only after pending tree activation.
+  commit_completion_waits_for_activation_ = hold_commit_for_activation;
+
   DCHECK(!blocked_main_commit().layer_tree_host);
   blocked_main_commit().layer_tree_host = layer_tree_host;
   scheduler_->NotifyReadyToCommit();
 }
 
 void ProxyImpl::UpdateRendererCapabilitiesOnImplThread() {
   DCHECK(IsImplThread());
   channel_impl_->SetRendererCapabilitiesMainCopy(
       layer_tree_host_impl_->GetRendererCapabilities()
           .MainThreadCapabilities());
@@ skipping 157 matching lines @@
                                                      base::TimeDelta delay) {
   DCHECK(IsImplThread());
   task_runner_provider_->ImplThreadTaskRunner()->PostDelayedTask(FROM_HERE,
                                                                  task, delay);
 }
 
 void ProxyImpl::DidActivateSyncTree() {
   TRACE_EVENT0("cc", "ProxyImpl::DidActivateSyncTreeOnImplThread");
   DCHECK(IsImplThread());
 
-  if (next_commit_waits_for_activation_) {
+  if (activation_completion_event_) {
     TRACE_EVENT_INSTANT0("cc", "ReleaseCommitbyActivation",
                          TRACE_EVENT_SCOPE_THREAD);
-    DCHECK(commit_completion_event_);
-    commit_completion_event_->Signal();
-    commit_completion_event_ = nullptr;
-    next_commit_waits_for_activation_ = false;
+    activation_completion_event_->Signal();
+    activation_completion_event_ = nullptr;
   }
 }
 
 void ProxyImpl::WillPrepareTiles() {
   DCHECK(IsImplThread());
   scheduler_->WillPrepareTiles();
 }
 
 void ProxyImpl::DidPrepareTiles() {
   DCHECK(IsImplThread());
@@ skipping 76 matching lines @@
   layer_tree_host_impl_->BeginCommit();
   blocked_main_commit().layer_tree_host->FinishCommitOnImplThread(
       layer_tree_host_impl_.get());
 
   // Remove the LayerTreeHost reference before the completion event is signaled
   // and cleared. This is necessary since blocked_main_commit() allows access
   // only while we have the completion event to ensure the main thread is
   // blocked for a commit.
   blocked_main_commit().layer_tree_host = nullptr;
 
-  if (next_commit_waits_for_activation_) {
-    // For some layer types in impl-side painting, the commit is held until
-    // the sync tree is activated.  It's also possible that the
-    // sync tree has already activated if there was no work to be done.
+  if (commit_completion_waits_for_activation_) {
+    // For some layer types in impl-side painting, the commit is held until the
+    // sync tree is activated.  It's also possible that the sync tree has
+    // already activated if there was no work to be done.
     TRACE_EVENT_INSTANT0("cc", "HoldCommit", TRACE_EVENT_SCOPE_THREAD);
+    commit_completion_waits_for_activation_ = false;
+    activation_completion_event_ = commit_completion_event_;
   } else {
     commit_completion_event_->Signal();
-    commit_completion_event_ = nullptr;
   }
+  commit_completion_event_ = nullptr;
 
   scheduler_->DidCommit();
 
   // Delay this step until afer the main thread has been released as it's
   // often a good bit of work to update the tree and prepare the new frame.
   layer_tree_host_impl_->CommitComplete();
 
   SetInputThrottledUntilCommitOnImpl(false);
 
   next_frame_is_newly_committed_frame_ = true;
@@ skipping 96 matching lines @@
 bool ProxyImpl::IsMainThreadBlocked() const {
   return task_runner_provider_->IsMainThreadBlocked();
 }
 
 ProxyImpl::BlockedMainCommitOnly& ProxyImpl::blocked_main_commit() {
   DCHECK(IsMainThreadBlocked() && commit_completion_event_);
   return main_thread_blocked_commit_vars_unsafe_;
 }
 
 }  // namespace cc