Defer ARC file system operations while ARC is booting.

chrome/browser/chromeos/arc/fileapi/arc_deferred_file_system_operation_runner_unittest.cc

Index: chrome/browser/chromeos/arc/fileapi/arc_deferred_file_system_operation_runner_unittest.cc
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_deferred_file_system_operation_runner_unittest.cc b/chrome/browser/chromeos/arc/fileapi/arc_deferred_file_system_operation_runner_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3d7c2b9164524f4924429430cc6573a6401f3f76
--- /dev/null
+++ b/chrome/browser/chromeos/arc/fileapi/arc_deferred_file_system_operation_runner_unittest.cc
@@ -0,0 +1,175 @@
+// 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 <string>
+
+#include "base/bind.h"
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/optional.h"
+#include "chrome/browser/chromeos/arc/fileapi/arc_deferred_file_system_operation_runner.h"
+#include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_service_manager.h"
+#include "components/arc/common/file_system.mojom.h"
+#include "components/arc/file_system/arc_file_system_operation_runner.h"
+#include "components/arc/test/fake_file_system_instance.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "mojo/public/cpp/system/handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace arc {
+
+namespace {
+
+constexpr char kAuthority[] = "authority";
+constexpr char kDocumentId[] = "document_id";
+constexpr char kUrl[] = "content://test";
+
+class FileSystemInstanceForTest : public FakeFileSystemInstance {

[hidehiko, 2017/01/20 08:01:49]

Could you merge this implementation to FakeFileSystemInstance, instead of
inheriting and introducing it?

In general, "fake" is introduced to maintain in one place (the implementation
dir), rather than test code.
It is beneficial for the maintainers, because they just can edit one fake class,
rather than client test code
in distributed dirs one by one.

If just merging does not work in your case, addressing it in a following CL with
leaving TODO here is fine to me.

[Shuhei Takahashi, 2017/01/20 09:13:04]

Yup, in this case the fake implementation is fairly simple so I think we can
merge it to FakeFileSystemInstance. Done.

+ public:
+  FileSystemInstanceForTest() = default;
+  ~FileSystemInstanceForTest() override = default;
+
+  void GetChildDocuments(const std::string& authority,
+                         const std::string& document_id,
+                         const GetChildDocumentsCallback& callback) override {
+    EXPECT_EQ(kAuthority, authority);
+    EXPECT_EQ(kDocumentId, document_id);
+    callback.Run(base::nullopt);
+  }
+
+  void GetDocument(const std::string& authority,
+                   const std::string& document_id,
+                   const GetDocumentCallback& callback) override {
+    EXPECT_EQ(kAuthority, authority);
+    EXPECT_EQ(kDocumentId, document_id);
+    callback.Run(mojom::DocumentPtr());
+  }
+
+  void GetFileSize(const std::string& url,
+                   const GetFileSizeCallback& callback) override {
+    EXPECT_EQ(kUrl, url);
+    callback.Run(0);
+  }
+
+  void OpenFileToRead(const std::string& url,
+                      const OpenFileToReadCallback& callback) override {
+    EXPECT_EQ(kUrl, url);
+    callback.Run(mojo::ScopedHandle());
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FileSystemInstanceForTest);
+};
+
+}  // namespace
+
+class ArcDeferredFileSystemOperationRunnerTest : public testing::Test {
+ public:
+  ArcDeferredFileSystemOperationRunnerTest()
+      : arc_service_manager_(base::MakeUnique<ArcServiceManager>(nullptr)) {
+    arc_service_manager_->arc_bridge_service()->file_system()->SetInstance(

[hidehiko, 2017/01/20 08:01:49]

For consistency, could you do this setup in SetUp() function, instead?
AFAIK, most ARC code does it there.

[Shuhei Takahashi, 2017/01/20 09:13:04]

Done.

+        &file_system_instance_);
+    // We can not use base::MakeUnique() for friend constructors.
+    arc_service_manager_->AddService(
+        base::WrapUnique(new ArcDeferredFileSystemOperationRunner(
+            arc_service_manager_->arc_bridge_service(),
+            false /* observe_events */)));
+    deferred_runner_ = static_cast<ArcDeferredFileSystemOperationRunner*>(
+        arc_service_manager_->GetService<ArcFileSystemOperationRunner>());
+  }
+  ~ArcDeferredFileSystemOperationRunnerTest() override = default;
+
+ protected:
+  content::TestBrowserThreadBundle thread_bundle_;
+  FileSystemInstanceForTest file_system_instance_;
+  std::unique_ptr<ArcServiceManager> arc_service_manager_;
+  // Owned by |arc_service_manager_|.
+  ArcDeferredFileSystemOperationRunner* deferred_runner_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ArcDeferredFileSystemOperationRunnerTest);
+};
+
+TEST_F(ArcDeferredFileSystemOperationRunnerTest, RunImmediately) {
+  int counter = 0;
+  deferred_runner_->SetShouldDefer(false);
+  deferred_runner_->GetChildDocuments(
+      kAuthority, kDocumentId,
+      base::Bind(
+          [](int* counter,
+             base::Optional<std::vector<mojom::DocumentPtr>> documents) {
+            ++*counter;
+          },
+          &counter));
+  deferred_runner_->GetDocument(
+      kAuthority, kDocumentId,
+      base::Bind([](int* counter, mojom::DocumentPtr document) { ++*counter; },
+                 &counter));
+  deferred_runner_->GetFileSize(
+      kUrl,
+      base::Bind([](int* counter, int64_t size) { ++*counter; }, &counter));
+  deferred_runner_->OpenFileToRead(
+      kUrl,
+      base::Bind([](int* counter, mojo::ScopedHandle handle) { ++*counter; },
+                 &counter));
+  EXPECT_EQ(4, counter);
+}
+
+TEST_F(ArcDeferredFileSystemOperationRunnerTest, DeferAndRun) {
+  int counter = 0;
+  deferred_runner_->SetShouldDefer(true);
+  deferred_runner_->GetChildDocuments(
+      kAuthority, kDocumentId,
+      base::Bind(
+          [](int* counter,
+             base::Optional<std::vector<mojom::DocumentPtr>> documents) {
+            ++*counter;
+          },
+          &counter));
+  deferred_runner_->GetDocument(
+      kAuthority, kDocumentId,
+      base::Bind([](int* counter, mojom::DocumentPtr document) { ++*counter; },
+                 &counter));
+  deferred_runner_->GetFileSize(
+      kUrl,
+      base::Bind([](int* counter, int64_t size) { ++*counter; }, &counter));
+  deferred_runner_->OpenFileToRead(
+      kUrl,
+      base::Bind([](int* counter, mojo::ScopedHandle handle) { ++*counter; },
+                 &counter));
+  EXPECT_EQ(0, counter);
+  deferred_runner_->SetShouldDefer(false);
+  EXPECT_EQ(4, counter);
+}
+
+TEST_F(ArcDeferredFileSystemOperationRunnerTest, DeferAndDiscard) {
+  int counter = 0;
+  deferred_runner_->SetShouldDefer(true);
+  deferred_runner_->GetChildDocuments(
+      kAuthority, kDocumentId,
+      base::Bind(
+          [](int* counter,
+             base::Optional<std::vector<mojom::DocumentPtr>> documents) {
+            ++*counter;
+          },
+          &counter));
+  deferred_runner_->GetDocument(
+      kAuthority, kDocumentId,
+      base::Bind([](int* counter, mojom::DocumentPtr document) { ++*counter; },
+                 &counter));
+  deferred_runner_->GetFileSize(
+      kUrl,
+      base::Bind([](int* counter, int64_t size) { ++*counter; }, &counter));
+  deferred_runner_->OpenFileToRead(
+      kUrl,
+      base::Bind([](int* counter, mojo::ScopedHandle handle) { ++*counter; },
+                 &counter));
+  EXPECT_EQ(0, counter);
+  arc_service_manager_.reset();
+  EXPECT_EQ(0, counter);
+}
+
+}  // namespace arc