Incorporate BlobChannel into Blimp image encode/decode pipeline.

blimp/client/feature/compositor/client_image_serialization_processor.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_CLIENT_FEATURE_COMPOSITOR_CLIENT_IMAGE_SERIALIZATION_PROCESSOR_H_
 #define BLIMP_CLIENT_FEATURE_COMPOSITOR_CLIENT_IMAGE_SERIALIZATION_PROCESSOR_H_
 
+#include <memory>
+
 #include "base/macros.h"
+#include "base/memory/singleton.h"
 #include "cc/proto/image_serialization_processor.h"
 #include "third_party/skia/include/core/SkPicture.h"
 
 class SkPixelSerializer;
 
 namespace blimp {
+
+class BlobChannelReceiver;
+
 namespace client {
 
-// ClientImageSerializationProcessor provides functionality to deserialize Skia
-// images.
+// Adds BlobChannel image retrieval support to the Skia image decoding process.
 class ClientImageSerializationProcessor

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

nit: Should this be BlobImageSerializationProcessor now?

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

I know they're in different namespaces, but it seems like that name could be
ambiguous with its serialization counterpart in Engine?

[Wez, 2016/06/07 01:18:31]

My point is that names ssociating them w/ Client & Engine are not necessarily a
good thing; we could in future pass Blobs from Client->Engine, for example -
this class' function isn't intrinsically tied to the Client, it just enables
Skia in the calling process to fetch imagery that has been pushed into our local
BlobCache, right?

[Kevin M, 2016/06/08 00:21:09]

Done.

     : public cc::ImageSerializationProcessor {
  public:
+  virtual ~ClientImageSerializationProcessor();

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

Why is the destructor public if this is a singleton?

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

Done.

+
+  static ClientImageSerializationProcessor* GetInstance();

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

Do we actually need this to be a singleton? Is it the case that we expect
multiple decoders to all need to reference it, or will we only have a single
decoder, in practice, at the client?

If the latter then consider moving to an owned object with checks to ensure that
there is at-most-one of it, rather than a lazy singleton.

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

The Skia decode callback is a raw function pointer which can't be associated
with any object instance. I think that means we'll need a singleton, or
something that's like a singleton, to connect the function with a stateful
object.

[Wez, 2016/06/07 01:18:31]

Right; I get that we need a function to
get-the-current-process-wide-serialization-processor but does it need to be a
lazily-constructed singleton, as it is here?

Could it instead follow a pattern like MessageLoop, where the caller manages the
lifetime, and internally it sets itself as the "current" processor, for the
decode callback to use?

[Kevin M, 2016/06/08 00:21:09]

Interesting, done.

+
+  void set_blob_receiver(BlobChannelReceiver* blob_receiver) {

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

nit: Clarify ownership/lifetime expectations of blob_receiver.

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

Done.

+    blob_receiver_ = blob_receiver;
+  }
+
+  // Retrieves a bitmap with ID |input| from |blob_receiver_| and decodes it
+  // to |bitmap|.
+  bool GetAndDecodeBlob(const void* input, size_t input_size, SkBitmap* bitmap);
+
+ private:
   ClientImageSerializationProcessor();

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

nit: Move this below the friend.

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

Done.

-  ~ClientImageSerializationProcessor();
+  friend struct base::DefaultSingletonTraits<ClientImageSerializationProcessor>;
 
-  // cc::ImageSerializationProcessor implementation.
+  // Adapts a bare function pointer call to a singleton call to
+  // GetAndDecodeBlob().
+  static bool InstallPixelRefProc(const void* input,
+                                  size_t input_size,
+                                  SkBitmap* bitmap);
+
+  // cc:ImageSerializationProcessor implementation.
   SkPixelSerializer* GetPixelSerializer() override;
   SkPicture::InstallPixelRefProc GetPixelDeserializer() override;
 
- private:
-  SkPicture::InstallPixelRefProc pixel_deserializer_;
+  BlobChannelReceiver* blob_receiver_ = nullptr;

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

nit: Comment to explain this member - how is it used?

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

Done.

 
   DISALLOW_COPY_AND_ASSIGN(ClientImageSerializationProcessor);
 };
 
 }  // namespace client
 }  // namespace blimp
 
 #endif  // BLIMP_CLIENT_FEATURE_COMPOSITOR_CLIENT_IMAGE_SERIALIZATION_PROCESSOR_H_