Incorporate BlobChannel into Blimp image encode/decode pipeline.

blimp/engine/renderer/blob_channel_sender_host.h

+// 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 BLIMP_ENGINE_RENDERER_BLOB_CHANNEL_SENDER_HOST_H_
+#define BLIMP_ENGINE_RENDERER_BLOB_CHANNEL_SENDER_HOST_H_
+
+#include <memory>
+#include <string>
+
+#include "base/containers/hash_tables.h"
+#include "blimp/common/blimp_common_export.h"
+#include "blimp/engine/mojo/blob_channel.mojom.h"
+
+namespace blimp {
+namespace engine {
+
+// Host object for communicating with the BlobChannel service across the
+// renderer/browser IPC boundary.

[Wez, 2016/05/21 01:08:04]

Communicating with the BlobChannel service doesn't tell me what this does - what
is this host object actually for?  Looks like it is basically the actual
BlobChannel send-side implementation, which renderer processes use to describe
blobs that need sharing with the client.

BTW, how do we ensure that requests to this service to place blobs are actually
processed before compositor traffic from the renderer, given that the two go
over different IPC channels?

Looking at the implementation this class just seems to be a trivial wrapper
around the Mojo channel, plus a replication-state table, and some annoying
DCHECKs. Could we have a single class presenting the BlobChannelSender interface
and have that take care of the send-if-necessary logic instead?

[Kevin M, 2016/05/27 22:35:30]

Description updated.
The IPC channel is the same. Mojo MessagePipe writes are synchronous and
unbuffered, so they should arrive on the browser side before subsequent writes
to the channel.
OK, I moved it to a BlobChannelSender, but it does mean that we are passing very
short lived refcounted BlobDataPtrs for the payload.

+class BLIMP_COMMON_EXPORT BlobChannelSenderHost {

[nyquist, 2016/05/24 00:05:41]

This is a little bit confusing naming to me. In //content a '*Host' suffix
usually implies that this is the part that runs in the browser process.

[Kevin M, 2016/05/27 22:35:30]

Done, it's now "proxy".

+ public:
+  // Represents the known cached state of a given blob.
+  // Flags can be combined with boolean AND.

[Wez, 2016/05/21 01:08:04]

Give a little context as to what these actually mean, e.g. does ENGINE mean
"this blob exists in the engine cache, but not in the client's, yet" or
something else?

[Kevin M, 2016/05/27 22:35:31]

Done.

+  enum ReplicationState { NOT_STORED = 0, ENGINE = 1 << 1, CLIENT = 1 << 2 };
+
+  BlobChannelSenderHost();
+  ~BlobChannelSenderHost();
+
+  // Indicates whether a given blob is stored on the engine, the client,
+  // or both.
+  // The return value is an integer composed of AND'd ReplicationState flags.
+  // TODO(kmarshall): Synchronize state with data on the Browser side.
+  int GetReplicationState(const std::string& id) const;

[Wez, 2016/05/21 01:08:04]

Would it be cleaner to provide separate IsInClientCache() and IsInEngineCache()
methods?

What is this actually used for?

[Kevin M, 2016/05/27 22:35:30]

Done.

+
+  // Conditionally stores a blob in the local portion of BlobChannel, if it
+  // doesn't already contain the blob.

[Wez, 2016/05/21 01:08:04]

It's not clear what "local portion" means here.

[Kevin M, 2016/05/27 22:35:31]

Comments for implementation have been removed.

+  // See the documentation for BlobChannel::Put().
+  void Put(const std::string& id, const std::string& data);
+
+  // Conditionally sends a blob to the remote side of the BlobChannel, if it
+  // doesn't already have the blob
+  // See the documentation for BlobChannel::Push().
+  void Push(const std::string& id);

[Wez, 2016/05/21 01:08:04]

Put & Push are just mirroring the BlobChannelSender interface, right? So why not
actually provide that interface, so the caller just uses it and the blob is
delivered to the browser process only if it's not already there?

[Kevin M, 2016/05/27 22:35:30]

Done.

+
+ private:
+  // BlobChannel Mojo IPC stub.

[Wez, 2016/05/21 01:08:04]

Which is used for...? Receiving the BlobChannel IPCs from the renderer?

[Kevin M, 2016/05/27 22:35:31]

Done.

+  mojom::BlobChannelPtr blob_channel_;
+
+  // Local copy of the cache state for the local and remote BlobChannel.
+  // TODO(kmarshall): Synchronize with browser side copy.
+  base::hash_map<std::string, int> replication_state_;

[Wez, 2016/05/21 01:08:04]

Wait... so this class is used in the _renderer_?  Why is it called "...Host",
then?  Host is usually the suffix for browser-side classes that handle renderer
requests for some purpose.

[Kevin M, 2016/05/27 22:35:30]

Acknowledged.

+
+  DISALLOW_COPY_AND_ASSIGN(BlobChannelSenderHost);
+};
+
+}  // namespace engine
+}  // namespace blimp
+
+#endif  // BLIMP_ENGINE_RENDERER_BLOB_CHANNEL_SENDER_HOST_H_