cc: Avoid deadlock with the UI thread and NPAPI in more cases

cc/scheduler/scheduler_unittest.cc

Index: cc/scheduler/scheduler_unittest.cc
diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
index 2f878fdcdbfa9a22a3d101934c3200fb3c30b96d..6c629213245f76a3cb6bc0523766e3c49b493935 100644
--- a/cc/scheduler/scheduler_unittest.cc
+++ b/cc/scheduler/scheduler_unittest.cc
@@ -1378,7 +1378,14 @@ TEST_F(SchedulerTest, NotSkipMainFrameInPreferImplLatencyMode) {
   EXPECT_SCOPED(MainFrameInHighLatencyMode(1, 1, true, true));
 }
 
-TEST_F(SchedulerTest, PollForCommitCompletion) {
+TEST_F(SchedulerTest,
+       Deadlock_NotifyReadyToCommitMakesProgressWhileSwapTrottled) {

[sunnyps, 2015/05/14 00:02:02]

Does this allow us to remove commit polling?

[brianderson, 2015/05/14 01:12:03]

We can remove commit polling once TextureUploader is removed or NPAPI is
completely gone in September.

Now that I think about it, I think we can already get rid of commit polling -
ever since I moved the BeginFrame suppression logic from the OutputSurface to
the Scheduler swap acks won't suppress BeginFrames. Unfortunately, this problem
will come back when we have unified BeginFrame, so we shouldn't actually remove
it yet.

+  // NPAPI plugins on Windows block the Browser UI thread on the Renderer main
+  // thread. This prevents the scheduler from receiving any pending swap acks.
+  // This test makes sure that we keep updating the TextureUploader with
+  // DidAnticipatedDrawTimeChange's so that it can make forward progress and
+  // upload all the textures needed for the commit to complete.
+
   // Since we are simulating a long commit, set up a client with draw duration
   // estimates that prevent skipping main frames to get to low latency mode.
   SchedulerClientWithFixedEstimates* client =
@@ -1439,6 +1446,284 @@ TEST_F(SchedulerTest, PollForCommitCompletion) {
   }
 }
 
+TEST_F(
+    SchedulerTest,
+    Deadlock_CommitMakesProgressWhileSwapTrottledAndActiveTreeNeedsFirstDraw) {
+  // NPAPI plugins on Windows block the Browser UI thread on the Renderer main
+  // thread. This prevents the scheduler from receiving any pending swap acks.
+  // The scheduler normally prioritizes draws over the commits, but doing so
+  // unconditionally will result in deadlocks.
+
+  // Since we are simulating a long commit, set up a client with draw duration
+  // estimates that prevent skipping main frames to get to low latency mode.
+  SchedulerClientWithFixedEstimates* client =
+      new SchedulerClientWithFixedEstimates(
+          base::TimeDelta::FromMilliseconds(1),
+          base::TimeDelta::FromMilliseconds(32),
+          base::TimeDelta::FromMilliseconds(32));
+  scheduler_settings_.use_external_begin_frame_source = true;
+  scheduler_settings_.main_frame_while_swap_throttled_enabled = true;
+  scheduler_settings_.impl_side_painting = true;
+  SetUpScheduler(make_scoped_ptr(client).Pass(), true);
+
+  // Disables automatic swap acks so this test can force swap ack throttling
+  // to simulate a blocked Browser ui thread.
+  scheduler_->SetMaxSwapsPending(1);
+  client_->SetAutomaticSwapAck(false);
+
+  // Get a new active tree in main-thread high latency mode and put us
+  // in a swap throttled state.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  scheduler_->SetNeedsRedraw();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_TRUE(scheduler_->CommitPending());
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  task_runner().RunPendingTasks();  // Run posted deadlines.

[sunnyps, 2015/05/14 00:02:02]

RunTasksWhile(DeadlinePending).

+  EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  scheduler_->NotifyReadyToActivate();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  EXPECT_TRUE(client->HasAction("ScheduledActionSendBeginMainFrame"));

[sunnyps, 2015/05/14 00:02:01]

EXPECT_ACTION?

+  EXPECT_TRUE(client->HasAction("ScheduledActionDrawAndSwapIfPossible"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionCommit"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionActivateSyncTree"));
+
+  // Make sure that we can finish the next commit even while swap throttled.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  EXPECT_SCOPED(AdvanceFrame());
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  scheduler_->NotifyReadyToActivate();
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  task_runner().RunPendingTasks();  // Run posted deadlines.
+  EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
+  EXPECT_TRUE(client->HasAction("ScheduledActionSendBeginMainFrame"));

[sunnyps, 2015/05/14 00:02:02]

EXPECT_ACTION?

+  EXPECT_TRUE(client->HasAction("ScheduledActionCommit"));
+  EXPECT_FALSE(client->HasAction("ScheduledActionActivateSyncTree"));
+
+  // Make sure we do not send a BeginMainFrame while swap throttled and
+  // we have both a pending tree and an active tree.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_FALSE(scheduler_->CommitPending());
+  task_runner().RunPendingTasks();  // Run posted deadline.
+  EXPECT_FALSE(client->HasAction("ScheduledActionSendBeginMainFrame"));
+}
+
+TEST_F(SchedulerTest,
+       Deadlock_CommitMakesProgressWhenIdleAndActiveTreeNeedsFirstDraw) {

[sunnyps, 2015/05/14 00:02:02]

I think this test is for the "HavePendingTree" case.

[brianderson, 2015/05/14 01:12:03]

It's also both. Will rename.

+  // NPAPI plugins on Windows block the Browser UI thread on the Renderer main
+  // thread. This prevents the scheduler from receiving any pending swap acks.
+  // The scheduler normally prioritizes draws over the commits, but doing so
+  // unconditionally will result in deadlocks.
+
+  // Since we are simulating a long commit, set up a client with draw duration
+  // estimates that prevent skipping main frames to get to low latency mode.
+  SchedulerClientWithFixedEstimates* client =
+      new SchedulerClientWithFixedEstimates(
+          base::TimeDelta::FromMilliseconds(1),
+          base::TimeDelta::FromMilliseconds(32),
+          base::TimeDelta::FromMilliseconds(32));
+  scheduler_settings_.use_external_begin_frame_source = true;
+  scheduler_settings_.main_frame_while_swap_throttled_enabled = true;
+  scheduler_settings_.main_frame_before_activation_enabled = true;
+  scheduler_settings_.impl_side_painting = true;
+  SetUpScheduler(make_scoped_ptr(client).Pass(), true);
+
+  // Disables automatic swap acks so this test can force swap ack throttling
+  // to simulate a blocked Browser ui thread.
+  scheduler_->SetMaxSwapsPending(1);
+  client_->SetAutomaticSwapAck(false);
+
+  // Start a new commit in main-thread high latency mode and hold off on
+  // activation.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  scheduler_->SetNeedsRedraw();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_TRUE(scheduler_->CommitPending());
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  task_runner().RunPendingTasks();  // Run posted deadlines.

[sunnyps, 2015/05/14 00:02:02]

RunTasksWhile(DeadlinePending).

+  EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
+  scheduler_->DidSwapBuffersComplete();
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  EXPECT_TRUE(client->HasAction("ScheduledActionSendBeginMainFrame"));

[sunnyps, 2015/05/14 00:02:01]

EXPECT_ACTION?

+  EXPECT_TRUE(client->HasAction("ScheduledActionDrawAndSwapIfPossible"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionCommit"));
+  EXPECT_FALSE(client->HasAction("ScheduledActionActivateSyncTree"));
+
+  // Start another commit while we still have an active tree.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  scheduler_->SetNeedsRedraw();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_TRUE(client->HasAction("ScheduledActionSendBeginMainFrame"));
+  EXPECT_TRUE(scheduler_->CommitPending());
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  task_runner().RunPendingTasks();  // Run posted deadlines.
+  EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
+  EXPECT_TRUE(client->HasAction("ScheduledActionDrawAndSwapIfPossible"));
+  scheduler_->DidSwapBuffersComplete();
+  scheduler_->NotifyBeginMainFrameStarted();
+
+  // Can't commit yet because there's still a pending tree.
+  scheduler_->NotifyReadyToCommit();
+  EXPECT_FALSE(client->HasAction("ScheduledActionActivateSyncTree"));
+  EXPECT_FALSE(client->HasAction("ScheduledActionCommit"));
+
+  // Activate the pending tree, which also unblocks the commit immediately.
+  scheduler_->NotifyReadyToActivate();
+  EXPECT_TRUE(client->HasAction("ScheduledActionActivateSyncTree"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionCommit"));
+}
+
+TEST_F(SchedulerTest,
+       Deadlock_NoBeginMainFrameWhileSwapTrottledAndHavePendingTree) {

[sunnyps, 2015/05/14 00:02:02]

This is the "ActiveTreeNeedsFirstDraw" case.

[brianderson, 2015/05/14 01:12:03]

It's both, I will change the name to:
Deadlock_NoBeginMainFrameWhileSwapTrottledAndHavePendingTreeAndActiveTreeNeedsFirstDraw

+  // NPAPI plugins on Windows block the Browser UI thread on the Renderer main
+  // thread. This prevents the scheduler from receiving any pending swap acks.
+  // The scheduler normally prioritizes draws over the commits, but doing so
+  // unconditionally will result in deadlocks.
+
+  // Since we are simulating a long commit, set up a client with draw duration
+  // estimates that prevent skipping main frames to get to low latency mode.
+  SchedulerClientWithFixedEstimates* client =
+      new SchedulerClientWithFixedEstimates(
+          base::TimeDelta::FromMilliseconds(1),
+          base::TimeDelta::FromMilliseconds(32),
+          base::TimeDelta::FromMilliseconds(32));
+  scheduler_settings_.use_external_begin_frame_source = true;
+  scheduler_settings_.main_frame_while_swap_throttled_enabled = true;
+  scheduler_settings_.main_frame_before_activation_enabled = true;
+  scheduler_settings_.impl_side_painting = true;
+  SetUpScheduler(make_scoped_ptr(client).Pass(), true);
+
+  // Disables automatic swap acks so this test can force swap ack throttling
+  // to simulate a blocked Browser ui thread.
+  scheduler_->SetMaxSwapsPending(1);
+  client_->SetAutomaticSwapAck(false);
+
+  // Start a new commit in main-thread high latency mode and hold off on
+  // activation.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  scheduler_->SetNeedsRedraw();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_TRUE(scheduler_->CommitPending());
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  task_runner().RunPendingTasks();  // Run posted deadlines.
+  EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
+  scheduler_->DidSwapBuffersComplete();
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  EXPECT_TRUE(client->HasAction("ScheduledActionSendBeginMainFrame"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionDrawAndSwapIfPossible"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionCommit"));
+  EXPECT_FALSE(client->HasAction("ScheduledActionActivateSyncTree"));
+
+  // Start another commit while we still have an active tree.
+  // Enter a swap throttled state.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  scheduler_->SetNeedsRedraw();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_TRUE(client->HasAction("ScheduledActionSendBeginMainFrame"));
+  EXPECT_TRUE(scheduler_->CommitPending());
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  task_runner().RunPendingTasks();  // Run posted deadlines.
+  EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
+  EXPECT_TRUE(client->HasAction("ScheduledActionDrawAndSwapIfPossible"));
+  scheduler_->NotifyBeginMainFrameStarted();
+
+  // Can't commit yet because there's still a pending tree.
+  scheduler_->NotifyReadyToCommit();
+  EXPECT_FALSE(client->HasAction("ScheduledActionActivateSyncTree"));
+  EXPECT_FALSE(client->HasAction("ScheduledActionCommit"));
+
+  // Activate the pending tree, which also unblocks the commit immediately.
+  scheduler_->NotifyReadyToActivate();
+  EXPECT_TRUE(client->HasAction("ScheduledActionActivateSyncTree"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionCommit"));
+
+  // Make sure we do not send a BeginMainFrame while swap throttled and
+  // we have both a pending tree and an active tree that still needs
+  // it's first draw.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_FALSE(scheduler_->CommitPending());
+  task_runner().RunPendingTasks();  // Run posted deadline.
+  EXPECT_FALSE(client->HasAction("ScheduledActionSendBeginMainFrame"));
+}
+
+TEST_F(SchedulerTest,
+       Deadlock_NoBeginMainFrameWhileSwapTrottledAndActiveTreeNeedsFirstDraw) {

[sunnyps, 2015/05/14 00:02:02]

In this case BeginMainFrame is blocked because of a pending tree and not because
of active_tree_needs_first_draw.

[brianderson, 2015/05/14 01:12:03]

Sorry, my comments in the test were wrong. This test is for impl_side_painting =
false, where we wouldn't have a pending tree. I guess this isn't a supported
configuration for the Renderer anymore - so maybe I should just delete this
test.

+  // This test only applies with non-impl-side painting, where we do not have a
+  // pending tree and might not be able to commit if we send a BeginMainFrame.
+
+  // NPAPI plugins on Windows block the Browser UI thread on the Renderer main
+  // thread. This prevents the scheduler from receiving any pending swap acks.
+  // The scheduler normally prioritizes draws over the commits, but doing so
+  // unconditionally will result in deadlocks.
+
+  // Since we are simulating a long commit, set up a client with draw duration
+  // estimates that prevent skipping main frames to get to low latency mode.
+  SchedulerClientWithFixedEstimates* client =
+      new SchedulerClientWithFixedEstimates(
+          base::TimeDelta::FromMilliseconds(1),
+          base::TimeDelta::FromMilliseconds(32),
+          base::TimeDelta::FromMilliseconds(32));
+  scheduler_settings_.use_external_begin_frame_source = true;
+  scheduler_settings_.impl_side_painting = false;
+  SetUpScheduler(make_scoped_ptr(client).Pass(), true);
+
+  // Disables automatic swap acks so this test can force swap ack throttling
+  // to simulate a blocked Browser ui thread.
+  scheduler_->SetMaxSwapsPending(1);
+  client_->SetAutomaticSwapAck(false);
+
+  // Get a new active tree in main-thread high latency mode.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  scheduler_->SetNeedsRedraw();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_TRUE(scheduler_->CommitPending());
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+  task_runner().RunPendingTasks();  // Run posted deadlines.
+  EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  EXPECT_TRUE(client->HasAction("ScheduledActionSendBeginMainFrame"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionDrawAndSwapIfPossible"));
+  EXPECT_TRUE(client->HasAction("ScheduledActionCommit"));
+
+  // Make sure we do not send a BeginMainFrame while swap throttled and
+  // we have both a pending tree and an active tree.
+  client_->Reset();
+  EXPECT_FALSE(scheduler_->CommitPending());
+  scheduler_->SetNeedsCommit();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_FALSE(scheduler_->CommitPending());
+  task_runner().RunPendingTasks();  // Run posted deadline.
+  EXPECT_FALSE(client->HasAction("ScheduledActionSendBeginMainFrame"));
+}
+
 TEST_F(SchedulerTest, BeginRetroFrame) {
   scheduler_settings_.use_external_begin_frame_source = true;
   SetUpScheduler(true);