Added ImagePipe

mojo/services/gfx/images/cpp/image_pipe_apptest.cc

+// 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.
+
+#include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/public/cpp/application/application_test_base.h"
+#include "mojo/public/cpp/environment/logging.h"
+#include "mojo/public/cpp/system/macros.h"
+#include "mojo/public/cpp/utility/run_loop.h"
+#include "mojo/services/gfx/images/cpp/image_pipe_consumer_endpoint.h"
+#include "mojo/services/gfx/images/cpp/image_pipe_producer_endpoint.h"
+
+namespace mojo {
+namespace {
+
+using namespace image_pipe;

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

this is banned by the style guide: 
http://google.github.io/styleguide/cppguide.html#Namespaces.  you can have using
statements for specific types you want to bring into this namespace

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

Done.

+
+class ImagePipeApplicationTest : public test::ApplicationTestBase {

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

what's this class for? it doesn't seem to do anything

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

Its used to do some kind of wicked crazy test macro magic in the TEST_F calls
below. I'm not going to pretend how it works, but it wont build without it and
all the other app tests have it, so unless you can tell me how to make app tests
work without it I'm gonna leave it there.

+ public:
+  ImagePipeApplicationTest() : ApplicationTestBase() {}
+  ~ImagePipeApplicationTest() override {}
+
+ private:
+  MOJO_DISALLOW_COPY_AND_ASSIGN(ImagePipeApplicationTest);
+};
+
+class TestImagePipe : private ImagePipeConsumerDelegate {

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

private inheritance is banned by the style guide (and generally not a good idea)

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

Acknowledged.

+ public:
+  TestImagePipe(bool producer_checked, bool consumer_checked);
+  ~TestImagePipe() override;
+  mojo::gfx::ImagePtr CreateTestImage();
+
+  ImagePipeProducerEndpoint* producer_endpoint;

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

if it's a class members are private

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

Acknowledged.

+  ImagePipeConsumerEndpoint* consumer_endpoint;
+  mojo::gfx::SupportedImagePropertiesPtr supported_properties_;

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

this is a public member with private naming, double confusing

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

Acknowledged.

+
+  bool HasExperiencedError() { return has_error_; }
+
+  std::function<void(mojo::gfx::ImagePtr image, uint32_t id)>

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

no public members

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

Acknowledged.

+      add_image_override;
+  std::function<void(uint32_t id)> remove_image_override;
+  std::function<void(uint32_t id)> present_image_override;
+
+ private:
+  // Inherited from ImagePipeConsumerDelegate //

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

odd trailing //

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

Done.

+  void AddImage(mojo::gfx::ImagePtr image, uint32_t id) override;
+  void RemoveImage(uint32_t id) override;
+  void PresentImage(uint32_t id) override;
+  void HandleEndpointClosed() override {
+    has_error_ = true;
+    mojo::RunLoop::current()->Quit();
+  }
+  bool has_error_;
+};
+
+void TestImagePipe::AddImage(mojo::gfx::ImagePtr image, uint32_t id) {
+  add_image_override(image.Pass(), id);
+}
+void TestImagePipe::RemoveImage(uint32_t id) {
+  remove_image_override(id);
+}
+void TestImagePipe::PresentImage(uint32_t id) {
+  present_image_override(id);
+}
+
+TestImagePipe::TestImagePipe(bool producer_checked, bool consumer_checked) {
+  present_image_override = [](uint32_t id) {};

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

seems silly - wouldn't it be cleaner to leave it default constructed (aka empty)
and just test if the function is empty before calling?

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

I mean in general I prefer initializing the functions in the constructor so that
way you dont have to check them at every single call site. In this case theres
only one call site so sure, I guess that makes sense, but in a more complex case
I would definitely favor having the functions be initialized empty so if theres
an unchecked call in some untested edge case it wont just bring the whole
program down.

+  remove_image_override = [](uint32_t id) {};
+  add_image_override = [](mojo::gfx::ImagePtr image, uint32_t id) {};
+
+  has_error_ = false;
+  supported_properties_ = mojo::gfx::SupportedImageProperties::New();
+  supported_properties_->size = mojo::Size::New();
+  supported_properties_->size->width = 256;
+  supported_properties_->size->height = 256;
+
+  supported_properties_->formats =
+      mojo::Array<mojo::gfx::ColorFormatPtr>::New(0);
+  mojo::gfx::ColorFormatPtr format = mojo::gfx::ColorFormat::New();
+  format->layout = mojo::gfx::PixelLayout::BGRA_8888;
+  format->color_space = mojo::gfx::ColorSpace::SRGB;
+  supported_properties_->formats.push_back(format.Pass());
+
+  mojo::gfx::ImagePipePtr image_pipe_ptr;
+  consumer_endpoint = new ImagePipeConsumerEndpoint(
+      GetProxy(&image_pipe_ptr), supported_properties_.Clone(), this);
+  if (!consumer_checked) {
+    consumer_endpoint->DisableFatalErrors();
+  }
+
+  producer_endpoint =
+      new ImagePipeProducerEndpoint(image_pipe_ptr.Pass(), [this] {
+        has_error_ = true;
+        mojo::RunLoop::current()->Quit();
+      });
+  if (!producer_checked) {
+    producer_endpoint->DisableFatalErrors();
+  }
+}
+
+TestImagePipe::~TestImagePipe() {
+  delete producer_endpoint;

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

std::unique_ptr<> instead?

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

Done.

+  delete consumer_endpoint;
+}
+
+mojo::gfx::ImagePtr TestImagePipe::CreateTestImage() {
+  mojo::MessagePipe pipe;
+
+  uint32_t bytes_per_pixel = 32;
+
+  mojo::gfx::ImageBufferPtr image_buffer = mojo::gfx::ImageBuffer::New();
+  image_buffer->size = supported_properties_->size->width *
+                       supported_properties_->size->width * bytes_per_pixel;
+  image_buffer->data = mojo::ScopedHandle(pipe.handle0.Pass());
+
+  mojo::gfx::ImagePtr image = mojo::gfx::Image::New();
+  image->buffer = image_buffer.Pass();
+  image->format = supported_properties_->formats[0].Clone();
+  image->size = supported_properties_->size.Clone();
+  image->pitch = supported_properties_->size->width;
+  image->stride = image->pitch * bytes_per_pixel;
+
+  return image;
+}
+
+// Tests that the usual flow for creating, adding, presenting, and removing
+// an image doesnt crash/break/cause-errors etc
+TEST_F(ImagePipeApplicationTest, NormalImageLifeCycle) {
+  TestImagePipe image_pipe(true, true);
+
+  image_pipe.present_image_override = [&image_pipe](uint32_t id) {
+    uint32_t acquired_id;
+    MOJO_CHECK(image_pipe.consumer_endpoint->AcquireNextImage(acquired_id));
+    MOJO_CHECK(acquired_id == id);
+    image_pipe.consumer_endpoint->ReleaseImage(
+        id, mojo::gfx::PresentationStatus::PRESENTED);
+  };
+
+  uint32_t id = 0, acquired_id = UINT32_MAX;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  MOJO_CHECK(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  MOJO_CHECK(acquired_id == id);
+  image_pipe.producer_endpoint->PresentImage(
+      id, [&image_pipe](unsigned int id, mojo::gfx::PresentationStatus status) {
+        EXPECT_EQ(mojo::gfx::PresentationStatus::PRESENTED, status);
+        mojo::RunLoop::current()->Quit();
+      });
+
+  mojo::RunLoop::current()->Run();
+  acquired_id = UINT32_MAX;
+  EXPECT_TRUE(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  MOJO_CHECK(acquired_id == id);
+  image_pipe.producer_endpoint->RemoveImage(acquired_id);
+
+  EXPECT_FALSE(image_pipe.HasExperiencedError());
+}
+
+// Tests that flushing returns images to the producer with NOT_PRESENTED_FLUSHED
+TEST_F(ImagePipeApplicationTest, FlushImages) {
+  TestImagePipe image_pipe(true, true);
+
+  uint32_t id = 0, acquired_id = UINT32_MAX;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  MOJO_CHECK(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  MOJO_CHECK(acquired_id == id);
+  image_pipe.producer_endpoint->PresentImage(
+      id,
+      [&id](unsigned int presented_id, mojo::gfx::PresentationStatus status) {
+        EXPECT_EQ(mojo::gfx::PresentationStatus::NOT_PRESENTED_FLUSHED, status);
+        EXPECT_TRUE(presented_id == id);
+        mojo::RunLoop::current()->Quit();
+      });
+  image_pipe.producer_endpoint->FlushImages();
+
+  mojo::RunLoop::current()->Run();
+  acquired_id = UINT32_MAX;
+  EXPECT_TRUE(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  EXPECT_TRUE(acquired_id == id);
+
+  EXPECT_FALSE(image_pipe.HasExperiencedError());
+}
+
+// Tests that you can safely try to acquire an image when none are available,
+// and that you will safely fail
+TEST_F(ImagePipeApplicationTest, AcquireImageFromEmptyPool) {
+  TestImagePipe image_pipe(true, true);
+
+  uint32_t id = 0xDEADBEEF, acquired_id = id;
+  EXPECT_FALSE(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  EXPECT_TRUE(acquired_id == id);
+  EXPECT_FALSE(image_pipe.HasExperiencedError());
+}
+
+// Tests that adding an image with an existing ID causes the pipe to error
+TEST_F(ImagePipeApplicationTest, ProducerError_AddImageWithReusedID) {
+  TestImagePipe image_pipe(false, true);
+
+  uint32_t id = 0;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+// Tests that removing an image that hasnt been added causes the pipe to error
+TEST_F(ImagePipeApplicationTest, ProducerError_RemoveImageBeforeAdded) {
+  TestImagePipe image_pipe(false, true);
+
+  uint32_t id = 0;
+  image_pipe.producer_endpoint->RemoveImage(id);
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+// Tests that removing an image that has already been removed causes the pipe to
+// error (essentially that removing and image takes it out of the pool)
+TEST_F(ImagePipeApplicationTest, ProducerError_AddImageThenRemoveTwice) {
+  TestImagePipe image_pipe(false, true);
+
+  uint32_t id = 0;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  image_pipe.producer_endpoint->RemoveImage(id);
+  image_pipe.producer_endpoint->RemoveImage(id);
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+// Tests that removing an image owned by the consumer causes the pipe to error
+TEST_F(ImagePipeApplicationTest, ProducerError_RemoveImageOwnedByConsumer) {
+  TestImagePipe image_pipe(false, true);
+
+  uint32_t id = 0, acquired_id = UINT32_MAX;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  MOJO_CHECK(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  MOJO_CHECK(acquired_id == id);
+  image_pipe.producer_endpoint->PresentImage(
+      id, [](unsigned int, mojo::gfx::PresentationStatus) {});
+  image_pipe.producer_endpoint->RemoveImage(id);
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+// Tests that presenting an image that hasnt been added causes the pipe to error
+TEST_F(ImagePipeApplicationTest, ProducerError_PresentImageNotAdded) {
+  TestImagePipe image_pipe(false, true);
+
+  uint32_t id = 0;
+  image_pipe.producer_endpoint->PresentImage(
+      id, [](unsigned int, mojo::gfx::PresentationStatus) {});
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+// Tests that presenting an image that has already been presented causes the
+// pipe to error
+TEST_F(ImagePipeApplicationTest, ProducerError_PresentImageTwice) {
+  TestImagePipe image_pipe(false, true);
+
+  uint32_t id = 0, acquired_id = UINT32_MAX;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  MOJO_CHECK(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  MOJO_CHECK(acquired_id == id);
+  image_pipe.producer_endpoint->PresentImage(
+      id, [](unsigned int, mojo::gfx::PresentationStatus) {});
+  image_pipe.producer_endpoint->PresentImage(
+      id, [](unsigned int, mojo::gfx::PresentationStatus) {});
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+// Tests that releasing an image that hasnt been added causes the pipe to error
+TEST_F(ImagePipeApplicationTest, ConsumerError_ReleaseImageNotInPool) {
+  TestImagePipe image_pipe(true, false);
+
+  image_pipe.present_image_override = [&image_pipe](uint32_t id) {
+    image_pipe.consumer_endpoint->ReleaseImage(
+        id + 1, mojo::gfx::PresentationStatus::PRESENTED);
+  };
+
+  uint32_t id = 0, acquired_id = UINT32_MAX;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  MOJO_CHECK(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  MOJO_CHECK(acquired_id == id);
+  image_pipe.producer_endpoint->PresentImage(
+      id, [](unsigned int, mojo::gfx::PresentationStatus) {});
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+// Tests that releasing an image before acquiring it causes the pipe to error
+TEST_F(ImagePipeApplicationTest, ConsumerError_ReleaseBeforeAcquire) {
+  TestImagePipe image_pipe(true, false);
+
+  image_pipe.present_image_override = [&image_pipe](uint32_t id) {
+    image_pipe.consumer_endpoint->ReleaseImage(
+        id, mojo::gfx::PresentationStatus::PRESENTED);
+  };
+
+  uint32_t id = 0, acquired_id = UINT32_MAX;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  MOJO_CHECK(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  MOJO_CHECK(acquired_id == id);
+  image_pipe.producer_endpoint->PresentImage(
+      id, [](unsigned int, mojo::gfx::PresentationStatus) {});
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+// Tests that releasing an image before its presented causes the pipe to error
+TEST_F(ImagePipeApplicationTest, ConsumerError_ReleaseImageNotPresented) {
+  TestImagePipe image_pipe(true, false);
+
+  image_pipe.present_image_override = [&image_pipe](uint32_t id) {
+    uint32_t acquired_id;
+    MOJO_CHECK(image_pipe.consumer_endpoint->AcquireNextImage(acquired_id));
+    MOJO_CHECK(acquired_id == id);
+    image_pipe.consumer_endpoint->ReleaseImage(
+        id + 1, mojo::gfx::PresentationStatus::PRESENTED);
+  };
+
+  uint32_t id = 0, acquired_id = UINT32_MAX;
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id);
+  image_pipe.producer_endpoint->AddImage(image_pipe.CreateTestImage(), id + 1);
+  MOJO_CHECK(image_pipe.producer_endpoint->AcquireImage(acquired_id));
+  MOJO_CHECK(acquired_id == id);
+  image_pipe.producer_endpoint->PresentImage(
+      id, [](unsigned int, mojo::gfx::PresentationStatus) {});
+
+  mojo::RunLoop::current()->Run();
+  EXPECT_TRUE(image_pipe.HasExperiencedError());
+}
+
+}  // namespace image_pipe
+}  // namespace mojo