Defer ARC file system operations while ARC is booting.

components/arc/file_system/arc_file_system_operation_runner.h

Index: components/arc/file_system/arc_file_system_operation_runner.h
diff --git a/components/arc/file_system/arc_file_system_operation_runner.h b/components/arc/file_system/arc_file_system_operation_runner.h
new file mode 100644
index 0000000000000000000000000000000000000000..6015140485c64e45b2ede8e55149ed609ba4c2d3
--- /dev/null
+++ b/components/arc/file_system/arc_file_system_operation_runner.h
@@ -0,0 +1,52 @@
+// 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.
+
+#ifndef COMPONENTS_ARC_FILE_SYSTEM_ARC_FILE_SYSTEM_OPERATION_RUNNER_H_
+#define COMPONENTS_ARC_FILE_SYSTEM_ARC_FILE_SYSTEM_OPERATION_RUNNER_H_
+
+#include <string>
+
+#include "components/arc/arc_service.h"
+#include "components/arc/common/file_system.mojom.h"
+
+namespace arc {
+
+// An interface to run ARC file system operations.
+//
+// This is an abstraction layer on top of mojom::FileSystemInstance.
+// An implementation of this interface is ArcDeferredFileSystemOperationRunner

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

In ArcServiceManager, ArcFileSystemOperationRunner instance must be unique,
otherwise named service getter won't work.
So, practically, ArcDeferredFileSystemOperationRunner should be the unique
subclass.
Could you explicitly document it? Also, maybe mentioning you introduced this
interface
for faking for testing will be valuable, I think.

(Optional) In the sense, how about renaming ArcDeferredFileSystemOperationRunner
to ArcFileSystemOperationRunnerImpl to imply it is the only (real)
implementation?

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

Sure, updated the comments.
Hmm, I'm wondering if which naming is good. It seems to me slightly better to
name this "deferred" because it more clearly conveys what it does.

+// which defers file system operations while ARC is booting.
+//
+// All member functions must be called on the UI thread.
+class ArcFileSystemOperationRunner : public ArcService {
+ public:
+  explicit ArcFileSystemOperationRunner(ArcBridgeService* bridge_service);
+  ~ArcFileSystemOperationRunner() override;
+
+  using GetFileSizeCallback = mojom::FileSystemInstance::GetFileSizeCallback;
+  using OpenFileToReadCallback =
+      mojom::FileSystemInstance::OpenFileToReadCallback;
+  using GetDocumentCallback = mojom::FileSystemInstance::GetDocumentCallback;
+  using GetChildDocumentsCallback =
+      mojom::FileSystemInstance::GetChildDocumentsCallback;
+
+  // Runs file system operations. See file_system.mojom for documentation.
+  virtual void GetFileSize(const std::string& url,
+                           const GetFileSizeCallback& callback) = 0;
+  virtual void OpenFileToRead(const std::string& url,
+                              const OpenFileToReadCallback& callback) = 0;
+  virtual void GetDocument(const std::string& authority,
+                           const std::string& document_id,
+                           const GetDocumentCallback& callback) = 0;
+  virtual void GetChildDocuments(const std::string& authority,
+                                 const std::string& parent_document_id,
+                                 const GetChildDocumentsCallback& callback) = 0;
+
+  // For supporting ArcServiceManager::GetService<T>().
+  static const char kArcServiceName[];

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

Style: Could you move up (maybe on top)?

https://google.github.io/styleguide/cppguide.html#Declaration_Order

Note: "using" there looks fine to me because these are close to virtual void
foo(...) = 0 declarations conceptually.

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

Done.

+};
+
+}  // namespace arc
+
+#endif  // COMPONENTS_ARC_FILE_SYSTEM_ARC_FILE_SYSTEM_OPERATION_RUNNER_H_