Added ImagePipe

mojo/services/gfx/images/cpp/image_pipe_endpoint.h

Index: mojo/services/gfx/images/cpp/image_pipe_endpoint.h
diff --git a/mojo/services/gfx/images/cpp/image_pipe_endpoint.h b/mojo/services/gfx/images/cpp/image_pipe_endpoint.h
new file mode 100644
index 0000000000000000000000000000000000000000..f6c3ae2571762e9955e41972db9f4da1a99bbad6
--- /dev/null
+++ b/mojo/services/gfx/images/cpp/image_pipe_endpoint.h
@@ -0,0 +1,79 @@
+// Copyright 2015 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 MOJO_SERVICES_GFX_IMAGES_CPP_IMAGE_PIPE_ENDPOINT_H_
+#define MOJO_SERVICES_GFX_IMAGES_CPP_IMAGE_PIPE_ENDPOINT_H_
+
+#include <deque>
+#include <map>
+#include <vector>
+
+#include "mojo/public/c/system/macros.h"
+#include "mojo/services/gfx/images/interfaces/image_pipe.mojom.h"
+
+namespace image_pipe {
+
+class ImagePipeEndpoint {
+ public:
+  ImagePipeEndpoint(bool is_producer,
+                    std::function<void()> fatal_error_handler);
+  virtual ~ImagePipeEndpoint();
+
+  void ProducerFatalError(const char* message, uint32_t id);

[jamesr, 2016/02/18 20:40:59]

why const char*? it's very rare in c++ code to really want to use c-style
strings

[Forrest Reiling, 2016/02/25 00:35:14]

Done.

+  void ConsumerFatalError(const char* message, uint32_t id);
+
+  bool AcquireNextImage(uint32_t& id);

[jamesr, 2016/02/18 20:40:59]

ditto re non-const ref param

[Forrest Reiling, 2016/02/25 00:35:14]

Done.

+
+  void ProducerAdd(uint32_t id);
+  void ProducerRemove(uint32_t id);
+  void ProducerPresent(uint32_t id,
+                       mojo::gfx::ImagePipe::PresentImageCallback callback);
+  void ConsumerRelease(uint32_t id, mojo::gfx::PresentationStatus status);
+  void ProducerFlush();
+
+  // For testing only, makes fatal errors not quite fatal, which allows tests
+  // to cause a fatal error and check that it was caught correctly without
+  // dying a horrible death in the process. If you are using this for something
+  // other than testing you are probably doing something very wrong.
+  void DisableFatalErrors();

[jamesr, 2016/02/18 20:40:59]

make "for testing" part of the function name - you can't see this comment at
callsites. in other code we suffix "ForTesting" on functions that should only be
used in tests

[Forrest Reiling, 2016/02/25 00:35:14]

Done.

+
+ private:
+  bool IsInPool(uint32_t id) const;
+  bool IsConsumerOwned(uint32_t id) const;
+  bool IsConsumerAcquirable(uint32_t id) const;
+  bool IsProducerOwned(uint32_t id) const;
+  bool IsProducerAcquirable(uint32_t id) const;
+  void CallPresentCallback(uint32_t id, mojo::gfx::PresentationStatus status);
+  void ReleaseInternal(uint32_t id, bool released_by_producer);
+
+  static void ImagePipeLogError(const char* entity,

[jamesr, 2016/02/18 20:40:59]

why const char*?

[Forrest Reiling, 2016/02/25 00:35:14]

Done.

+                                const char* message,
+                                uint32_t id);
+
+  // Used for internal state tracking and validation
+  std::map<uint32_t, mojo::gfx::ImagePipe::PresentImageCallback>

[jamesr, 2016/02/18 20:40:59]

unordered_map?

[Forrest Reiling, 2016/02/25 00:35:14]

I mean theres not likely going to be more than a few elements in this map at a
time, so i doubt it will matter.. But sure, why not?

+      present_callback_map_;
+
+  // ids that have been added to the pipe's image pool and have not been removed
+  std::vector<uint32_t> image_pool_ids_;

[jamesr, 2016/02/18 20:40:59]

does the order of these matter? if it's just keeping track of a set then it
seems like std::set<> or std::unordered_set<> would be better

[Forrest Reiling, 2016/02/25 00:35:14]

yeah i guess. The ones that are deques I think should probably stay deques since
the ordering is important on those in some cases, but I guess for this guy it
doesnt matter. For the consumer_owned_ids_ and producer_owned_ids_ its a little
trickier because the consumer/producer can release the ids it owns in any order,
though some orders will be more likely in practice. For the common use case
these containers will probably only have a few elements each, so hash table
based containers might actually perform worse than array based containers, but I
guess I agree with this on the grounds that unordered_set more accurately
represents the intended use of the container. 

+
+  // images that have been aquired by the consumer
+  std::vector<uint32_t> consumer_owned_ids_;
+  // images that have been presented by the producer but have not been aquired
+  // by the consumer
+  std::deque<uint32_t> consumer_acquirable_ids_;
+
+  // images that have been aquired by the producer
+  std::vector<uint32_t> producer_owned_ids_;
+  // images that have been released by the producer but have not been aquired
+  // by the consumer
+  std::deque<uint32_t> producer_acquirable_ids_;
+
+  bool is_producer_;
+  bool is_checked_;
+  std::function<void()> fatal_error_handler_;
+};
+}

[jamesr, 2016/02/18 20:40:59]

say what this bracket is closing (i.e. "}  // namespace image_pipe")

[Forrest Reiling, 2016/02/25 00:35:14]

Done.

+
+#endif  // MOJO_SERVICES_GFX_IMAGES_CPP_IMAGE_PIPE_ENDPOINT_H_