Add error handling and unit test for pdf compositor service

components/printing/service/pdf_compositor_service_unittest.cc

+// Copyright 2017 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 <memory>
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/memory/shared_memory.h"
+#include "base/path_service.h"
+#include "components/printing/service/public/interfaces/pdf_compositor.mojom.h"
+#include "mojo/public/cpp/system/platform_handle.h"
+#include "services/service_manager/public/cpp/service_test.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace printing {
+
+class PdfCompositorServiceTest : public service_manager::test::ServiceTest {
+ public:
+  PdfCompositorServiceTest() : ServiceTest("pdf_compositor_service_unittest") {}
+  ~PdfCompositorServiceTest() override {}
+
+  MOCK_METHOD1(CallbackOnSuccess, void(mojo::SharedBufferHandle));
+  MOCK_METHOD1(CallbackOnError, void(mojom::PdfCompositor::Status));
+  void OnCallback(mojom::PdfCompositor::Status status,
+                  mojo::ScopedSharedBufferHandle handle) {
+    if (status == mojom::PdfCompositor::Status::SUCCESS)
+      CallbackOnSuccess(handle.get());
+    else
+      CallbackOnError(status);
+    run_loop_->Quit();
+  }
+
+  MOCK_METHOD0(ConnectionClosed, void());
+
+ protected:
+  // service_manager::test::ServiceTest:
+  void SetUp() override {
+    ServiceTest::SetUp();
+
+    ASSERT_FALSE(compositor_);
+    connector()->BindInterface(mojom::kServiceName, &compositor_);
+    ASSERT_TRUE(compositor_);
+
+    run_loop_.reset(new base::RunLoop());

[Lei Zhang, 2017/06/02 21:54:09]

run_loop_ = base::MakeUnique<base::RunLoop>();

[Wei Li, 2017/07/07 21:23:57]

Done.

+  }
+
+  void TearDown() override {
+    // Clean up
+    compositor_.reset();
+  }
+
+  base::SharedMemoryHandle LoadFileInSharedMemory(size_t* len) {

[Lei Zhang, 2017/06/02 21:54:09]

base::SharedMemoryHandle has a GetSize() method. Do we need |len| as an out
param?

[Wei Li, 2017/07/07 21:23:57]

Added a check to make sure these two are same so we can use either one.

+    base::FilePath path;
+    PathService::Get(base::DIR_SOURCE_ROOT, &path);
+    base::FilePath test_file =
+        path.AppendASCII("components/test/data/printing/google.mskp");
+    std::string content;
+    base::ReadFileToString(test_file, &content);

[Lei Zhang, 2017/06/02 21:54:09]

Do we want to return early if this fails? Same for Create() and Map() below.

[Wei Li, 2017/07/07 21:23:57]

Checks added.

+    *len = content.size();
+    base::SharedMemoryCreateOptions options;
+    options.size = *len;
+    options.share_read_only = true;
+    base::SharedMemory shared_memory;
+    shared_memory.Create(options);
+    shared_memory.Map(*len);
+    memcpy(shared_memory.memory(), content.data(), *len);
+    return base::SharedMemory::DuplicateHandle(shared_memory.handle());
+  }
+
+  void CallCompositorWithSuccess(mojom::PdfCompositorPtr ptr) {
+    size_t len;
+    mojo::ScopedSharedBufferHandle buffer_handle =
+        mojo::WrapSharedMemoryHandle(LoadFileInSharedMemory(&len), len, true);
+    ASSERT_TRUE(buffer_handle->is_valid());
+    EXPECT_CALL(*this, CallbackOnSuccess(testing::_)).Times(1);
+    ptr->CompositePdf(std::move(buffer_handle),
+                      base::BindOnce(&PdfCompositorServiceTest::OnCallback,
+                                     base::Unretained(this)));
+    run_loop_->Run();
+  }
+
+  std::unique_ptr<base::RunLoop> run_loop_;
+  mojom::PdfCompositorPtr compositor_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PdfCompositorServiceTest);
+};
+
+// Test callback is called on error conditions in service.
+TEST_F(PdfCompositorServiceTest, InvokeCallbackOnContentError) {
+  size_t len;
+  mojo::ScopedSharedBufferHandle buffer_handle =
+      mojo::WrapSharedMemoryHandle(LoadFileInSharedMemory(&len), 10, true);
+  // The size of mapped area is not equal to the original buffer,
+  // so the content is invalid.
+  ASSERT_NE(10u, len);

[Lei Zhang, 2017/06/02 21:54:09]

Can we check for the expected size of |len| instead?

[Wei Li, 2017/07/07 21:23:57]

The len is still file content size. But we truncated the shared memory to size
10 (smaller than needed) to simulate an erroneous buffer.  

+  ASSERT_TRUE(buffer_handle->is_valid());
+  EXPECT_CALL(*this, CallbackOnError(
+                         mojom::PdfCompositor::Status::CONTENT_FORMAT_ERROR))
+      .Times(1);
+  compositor_->CompositePdf(
+      std::move(buffer_handle),
+      base::BindOnce(&PdfCompositorServiceTest::OnCallback,
+                     base::Unretained(this)));
+  run_loop_->Run();
+}
+
+TEST_F(PdfCompositorServiceTest, InvokeCallbackOnSuccess) {
+  CallCompositorWithSuccess(std::move(compositor_));
+}
+
+TEST_F(PdfCompositorServiceTest, ServiceInstances) {
+  // One service can bind multiple interfaces.
+  mojom::PdfCompositorPtr another_compositor;
+  ASSERT_FALSE(another_compositor);
+  connector()->BindInterface(mojom::kServiceName, &another_compositor);
+  ASSERT_TRUE(another_compositor);
+  ASSERT_NE(compositor_.get(), another_compositor.get());
+
+  // Terminating one interface won't affect another.
+  compositor_.reset();
+  CallCompositorWithSuccess(std::move(another_compositor));
+}
+
+}  // namespace printing