cc/blimp: Add a LayerTreeHostRemote implementation.

cc/blimp/remote_compositor_bridge.h

Index: cc/blimp/remote_compositor_bridge.h
diff --git a/cc/blimp/remote_compositor_bridge.h b/cc/blimp/remote_compositor_bridge.h
new file mode 100644
index 0000000000000000000000000000000000000000..147f408dd53b406705cc9abb6a191693c8c0e54e
--- /dev/null
+++ b/cc/blimp/remote_compositor_bridge.h
@@ -0,0 +1,55 @@
+// 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.
+
+#ifndef CC_BLIMP_REMOTE_COMPOSITOR_BRIDGE_H_
+#define CC_BLIMP_REMOTE_COMPOSITOR_BRIDGE_H_
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "cc/base/cc_export.h"
+
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
+namespace cc {
+class CompositorProtoState;
+class RemoteCompositorBridgeClient;
+
+// The RemoteCompositorBridge is the compositor's communication bridge to the

[danakj, 2016/09/30 22:37:13]

It's not the compositor's communication bridge. It's the remote LTH
implementation's interface to talk to the blimp client which has a compositor in
it, right?

I think this is mostly about framing but here's what I have in my head where
this code is going architecturally, and I would like comments and dependencies
to match that if it seems right:

- LayerTreeHost is no longer a compositor, it is a public API that is exposed to
blink and content/renderer/.
- LayerTreeHostInProcess is an implementation of LTH that is a compositor.
- LayerTreeHostRemote is an implementation of LTH that talks to a blimp client.
- Owners of LayerTreeHostInProcess continue to be embedders of a compositor,
these are ui::Compositor, android ui, blimp client, unit tests.
- There are a few clear layers here though: blink <-> blimp engine impl
(LTHRemote) <-> blimp client (cc embedder) <-> cc layer compositor
(LTHInProcess)
- Nothing skips past these layers, so the LTHRemote talks to blimp client, not
to a compositor. Moving between these layers should normally go through an
abstract interface so we can replace it in tests and test a single layer on its
own.

Does that make sense? This relates to thinking about how to test things too. We
don't test LTHRemote by attaching it to a LTHInProcess. We test its interactions
with blimp client. We can test blimp client interactions with cc but adding each
layer to the same test increases the amount of work to maintain the test over
time and the likelyhood of it becoming flaky/failing due to unrelated code
change.

[Khushal, 2016/10/01 00:53:48]

Yup, the class structure is going to be that blimp will have an impl of this
class to take the protos (then diff/cache or whatever optimizations it wants to
do). On the client side the blimp code will have a compositor which owns a
LTHInProcess, and a CompositorStateDeserializer to deserialize the state into
the LayerTree.
This sounds mostly like what I had in mind, each layer has an interface to talk
to the next layer in the pipeline. At the same time, each layer has a
responsibility so that so long as you have an implementation that faithfully
does that, you can replace the layer in between 2 components and test the
behaviour of the system together. So the layer between LTHRemote and
CompositorStateDeserializer on the client's job is to correctly transport the
data provided between these 2 ends. There can be optimizations in the transport
but these 2 ends don't need to aware of it.

One of the primary reasons for having this code in cc is that it is strongly
tied to the cc APIs and data. Sure, we have an impl of LTHRemote, which keeps
blimp specific quirks outside rest of cc, but for instance, if a callback is
added to the LTHClient that blink now depends on, we have to make sure it gets
updated in both places. Similarly, the data we are transporting is cc data
structures and any updates to these have to be made together. Basically cc/blimp
is saying here is a way you can record cc state for deferred use and later if
you give it to back to me, you'll get the same result. The best parallel here
would be an SkPicture. Now if skia changes anything, its their responsibility to
make sure that the recording API works correctly.

The only way I could think of getting some testing to do this with cc is to mock
out the layer between LTHRemote and LTHInProcess (assuming its a dumb transport)
and run all the existing tests, and any future tests added here. If a new
property is added, I hope we have tests to cover their use through the cc
pipeline.

I understand if you think that is going overboard, but I'm not sure how I can
convince myself that regressions won't creep in from this transition layer
between blink and cc not being maintained. If there is another testing strategy
you have in mind, I'm all open to hear that.

+// compositor on the client that consumes the CompositorProtoState updates and
+// performs the actual compositing work. It is responsible for scheduling
+// when a state update should be sent to the client and providing back
+// mutations made to the state on the client.
+class CC_EXPORT RemoteCompositorBridge {
+ public:
+  RemoteCompositorBridge(
+      scoped_refptr<base::SingleThreadTaskRunner> compositor_main_task_runner);
+
+  virtual ~RemoteCompositorBridge();
+
+  // Must be called exactly once before the RemoteCompositorBridge can be
+  // used. Once bound, the client is expected to outlive this class.
+  virtual void BindToClient(RemoteCompositorBridgeClient* client) = 0;
+
+  // Notifies the bridge that a main frame update is required.
+  virtual void ScheduleMainFrame() = 0;
+
+  // If a main frame update results in any mutations to the compositor state,
+  // the serialized compositor state is provided to the
+  // RemoteCompositorBridge.
+  virtual void ProcessCompositorStateUpdate(
+      std::unique_ptr<CompositorProtoState> compositor_proto_state) = 0;
+
+ protected:
+  // The task runner for the compositor's main thread. The
+  // RemoteCompositorBridgeClient must be called on this task runner.
+  scoped_refptr<base::SingleThreadTaskRunner> compositor_main_task_runner_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(RemoteCompositorBridge);
+};
+
+}  // namespace cc
+#endif  // CC_BLIMP_REMOTE_COMPOSITOR_BRIDGE_H_