[fsp] Extract common code to FileSystemProvidedInternalFunction.

chrome/browser/chromeos/extensions/file_system_provider/provider_function.h

Index: chrome/browser/chromeos/extensions/file_system_provider/provider_function.h
diff --git a/chrome/browser/chromeos/extensions/file_system_provider/provider_function.h b/chrome/browser/chromeos/extensions/file_system_provider/provider_function.h
new file mode 100644
index 0000000000000000000000000000000000000000..e29db06ae5a7a6e1d17f7c43a2025b6836a42cf9
--- /dev/null
+++ b/chrome/browser/chromeos/extensions/file_system_provider/provider_function.h
@@ -0,0 +1,83 @@
+// Copyright 2013 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 CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_SYSTEM_PROVIDER_PROVIDER_FUNCTION_H_
+#define CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_SYSTEM_PROVIDER_PROVIDER_FUNCTION_H_
+#include <string>
+
+#include "base/files/file.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"

[hashimoto, 2014/05/01 09:20:12]

nit: "class DictionaryValue;" is enough?

[mtomasz, 2014/05/02 01:26:22]

Done.

+#include "chrome/browser/extensions/chrome_extension_function.h"
+#include "chrome/common/extensions/api/file_system_provider.h"
+
+namespace chromeos {
+namespace file_system_provider {
+class RequestValue;
+}  // file_system_provider
+}  // chromeos
+
+namespace extensions {
+
+// Error names from
+// http://www.w3.org/TR/file-system-api/#errors-and-exceptions
+extern const char kNotFoundErrorName[];
+extern const char kSecurityErrorName[];
+
+// Error messages.
+extern const char kEmptyNameErrorMessage[];
+extern const char kMountFailedErrorMessage[];
+extern const char kUnmountFailedErrorMessage[];
+extern const char kResponseFailedErrorMessage[];
+
+// Creates a dictionary, which looks like a DOMError. The returned dictionary
+// will be converted to a real DOMError object in
+// file_system_provier_custom_bindings.js.
+base::DictionaryValue* CreateError(const std::string& name,
+                                   const std::string& message);
+
+// Converts ProviderError to base::File::Error. This could be redundant, if it
+// was possible to create DOMError instances in Javascript easily.
+base::File::Error ProviderErrorToFileError(
+    api::file_system_provider::ProviderError error);
+
+class FileSystemProviderInternalFunction : public ChromeSyncExtensionFunction {

[hashimoto, 2014/05/01 09:20:12]

Please add a class comment.

[mtomasz, 2014/05/02 01:26:22]

Done.

+ public:
+  FileSystemProviderInternalFunction();
+
+ protected:
+  virtual ~FileSystemProviderInternalFunction() {}
+
+  // Rejects the request. If failed, then sets an error and returns false. The
+  // caller should also return false in such case.

[hashimoto, 2014/05/01 09:20:12]

Since there are many parties (provider extension, user extension, extension
function inheriting this class, and this class) involved here, could you make
this comment a bit clearer?
Please note that there can be two different meanings for "error" (i.e. errors
returned to the user extension and errors returned to the provider extension)
and three meanings for "return" here. (returning to the provider extension JS
code, returning to the user extension JS code, and returning to the extension
function)

IIUC, an extremely lengthy version of this comment would be:
(The extension function called by the provider extension uses this function to)
reject the request (made by the user extension and sends the error to the user).
If failed, (i.e. the reject message cannot be delivered to the user) sets an
error (as the result of the extension function) and returns false (to the
extension function). The caller (i.e. the extension function) should also return
false (to let the provider extension know about the failure) in such case.

[mtomasz, 2014/05/02 01:26:22]

I reworked the code to make it simpler. Now the methods are much shorter, and do
not return any value. WDYT?

+  bool RejectRequest(base::File::Error error);
+
+  // Fulfills the request. If failed, then sets an error and returns false. The
+  // caller should also return false then.

[hashimoto, 2014/05/01 09:20:12]

ditto.

Also, please briefly describe what |value| and |has_net| are for.

[mtomasz, 2014/05/02 01:26:22]

Done.

+  bool FulfillRequest(
+      scoped_ptr<chromeos::file_system_provider::RequestValue> value,
+      bool has_next);
+
+  bool is_valid() { return is_valid_; }

[hashimoto, 2014/05/01 09:20:12]

Please add a comment about what "valid" means.

[mtomasz, 2014/05/02 01:26:22]

Removed.

+
+  // ChromeSyncExtensionFunction overrides.
+  virtual bool RunImpl() OVERRIDE;
+
+ private:
+  // Parses the function args in order to acquire the file system id and the
+  // request id. Also, checks if the arguments are correct, and if there is a
+  // valid corresponding provided file system. On error, returns false.
+  bool Parse();
+
+  // Sets an error message in case of a failure.
+  void SetErrorMessage(const std::string& name, const std::string& message);
+
+  int file_system_id_;
+  int request_id_;
+  bool is_valid_;
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_SYSTEM_PROVIDER_PROVIDER_FUNCTION_H_