content/blimp: Set up hooks for enabling LTHRemote in the renderer.

blimp/engine/renderer/frame_scheduler.cc

+// Copyright 2016 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 "base/logging.h"
+#include "blimp/engine/renderer/frame_scheduler.h"
+
+namespace blimp {
+namespace engine {
+namespace {
+// This is the temporary frame delay to keep pages which make animation requests
+// but don't mutate the state on the engine from running main frames
+// back-to-back. We need smarter throttling of engine updates. See
+// crbug.com/597829.
+constexpr base::TimeDelta kDefaultFrameDelay =
+    base::TimeDelta::FromMilliseconds(30);
+}  // namespace
+
+FrameScheduler::FrameScheduler(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    FrameSchedulerClient* client)
+    : FrameScheduler(kDefaultFrameDelay, std::move(task_runner), client) {}
+
+FrameScheduler::FrameScheduler(
+    base::TimeDelta frame_delay,
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    FrameSchedulerClient* client)
+    : next_frame_time_(base::TimeTicks::Now()),
+      frame_delay_(frame_delay),
+      client_(client) {
+  DCHECK(client_);
+  frame_tick_timer_.SetTaskRunner(std::move(task_runner));
+}
+
+FrameScheduler::~FrameScheduler() = default;
+
+void FrameScheduler::ScheduleFrameUpdate() {
+  if (needs_frame_update_)
+    return;
+
+  needs_frame_update_ = true;
+  ScheduleFrameUpdateIfNecessary();
+}
+
+void FrameScheduler::DidSendFrameUpdateToClient() {
+  DCHECK(!frame_ack_pending_) << "We can have only frame update in flight";
+
+  frame_ack_pending_ = true;
+  ScheduleFrameUpdateIfNecessary();

[Wez, 2016/10/21 01:40:35]

Wait... this call will never have any effect, surely? :P

[Khushal, 2016/10/21 03:13:56]

Woops. :P Will remove.

[Khushal, 2016/10/24 22:49:09]

Done.

+}
+
+void FrameScheduler::DidReceiveFrameUpdateAck() {
+  DCHECK(frame_ack_pending_);
+
+  frame_ack_pending_ = false;
+  ScheduleFrameUpdateIfNecessary();
+}
+
+void FrameScheduler::ScheduleFrameUpdateIfNecessary() {
+  // If we can't produce main frame updates right now, don't schedule a task.
+  if (!ShouldProduceFrameUpdates()) {
+    return;
+  }
+
+  // If a task has already been scheduled, we don't need to schedule again.
+  if (frame_tick_timer_.IsRunning())
+    return;
+
+  base::TimeDelta delay = next_frame_time_ - base::TimeTicks::Now();
+  frame_tick_timer_.Start(
+      FROM_HERE, delay,
+      base::Bind(&FrameScheduler::StartFrameUpdate, base::Unretained(this)));
+}
+
+bool FrameScheduler::ShouldProduceFrameUpdates() const {
+  return needs_frame_update_ && !frame_ack_pending_;
+}
+
+void FrameScheduler::StartFrameUpdate() {
+  DCHECK(needs_frame_update_);
+
+  // If an update was sent to the client since this request, we can not start
+  // another frame. Early out here, we'll come back when an Ack is received from
+  // the client.

[Wez, 2016/10/20 23:53:27]

It's really not clear from this comment under which conditions it is even
possible for this to occur - IIUC it's basically because of the separation
between StartFrameUpdate and DidSendFrameUpdateToClient?  Is there a reason why
we can't set |frame_ack_pending_| to true here, and eliminate
DidSendFrameUpdateToClient(), to simplify this state-machine?

[Khushal, 2016/10/21 00:03:11]

Hmm, so StartFrameUpdate won't necessarily result in DidSendFrameUpdateToClient.
For instance, if it does not result in any content changes. Basically if we
reach here,
https://cs.chromium.org/chromium/src/cc/blimp/layer_tree_host_remote.cc?sq=pa...

So we can have the state where we run StartFrameUpdate, running this causes
ScheduleFrameUpdate to be called, because there can be a request to schedule the
next frame while we are updating for the current one, which will queue a task
with the |frame_tick_timer_|, and we then have a DidSendFrameUpdateToClient.

Though what we can probably do is, if we get a DidSendFrameUpdateToClient, we
can reset the frame_tick_timer_ and remove this early out here instead. Does
that sound cleaner?

[Wez, 2016/10/21 01:40:35]

Hmmm. Resetting the timer in DidSendFrameUpdateToClient will lead to clock-churn
in some cases; we'll stop the timer (leaving a no-op delayed task in the queue)
only to likely create an identical timer task when the ACK arrives. But it'll
only be one wasted timer task, I suppose, so probably not worth worrying
about...

However, the lack of a 1:1 correspondence between StartFrameUpdate and
DidSendFrameUpdateToClient is really what creates the confusion. With the
current impl, if creating the frame update is super slow then we could
conceivably call StartFrameUpdate multiple times before
DidSendFrameUpdateToClient is even called (Though obviously not if it's
synchronous). Is that actually possible, though?

Really the required semantics of StartFrameUpdate and the Did* APIs needs to be
spelled-out in the header (e.g. that StartFrameUpdate MAY result in DidSend*,
that DidSend* MUST be followed by DidReceive*Ack eventually, etc).

[Khushal, 2016/10/21 03:13:56]

Nope, that's not possible. DidSendFrameUpdateToClient should be called
synchronously when we are within a frame update. I'll DCHECK that here to make
it obvious too.
Sure, let me write some more detailed comments here to clarify the state
expectations.

[Khushal, 2016/10/24 22:49:09]

Done.

+  if (frame_ack_pending_)
+    return;
+
+  needs_frame_update_ = false;
+  next_frame_time_ = base::TimeTicks::Now() + frame_delay_;
+  client_->StartFrameUpdate();
+}
+
+}  // namespace engine
+}  // namespace blimp