Don't crash browser process when OK passed as error code in FSP API.

chrome/renderer/resources/extensions/file_system_provider_custom_bindings.js

Index: chrome/renderer/resources/extensions/file_system_provider_custom_bindings.js
diff --git a/chrome/renderer/resources/extensions/file_system_provider_custom_bindings.js b/chrome/renderer/resources/extensions/file_system_provider_custom_bindings.js
index 91a271876da952bcdd39f7c0fc4dbf99db29c44d..b09f52621dcd41d7b298d0e122e112462b30bcd5 100644
--- a/chrome/renderer/resources/extensions/file_system_provider_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/file_system_provider_custom_bindings.js
@@ -120,6 +120,20 @@ function verifyMetadata(options, metadata) {
 }
 
 /**
+ * Verifies if the passed error code is valid when used to indicate
+ * a failure.
+ * @param {!string} error
+ * @return {boolean} True if valid, false if invalid.
+ */
+function verifyErrorForFailure(error) {
+  if (error === 'OK') {
+    console.error('Error code cannot be OK in case of failures.');
+    return false;
+  }
+  return true;
+}
+
+/**
  * Annotates an entry metadata by serializing its modifiedTime value.
  * @param {EntryMetadata} metadata Input metadata.
  * @return {EntryMetadata} metadata Annotated metadata, which can be passed
@@ -156,6 +170,8 @@ function massageArgumentsDefault(args, dispatch) {
         options.fileSystemId, options.requestId, Date.now() - executionStart);
   };
   var onErrorCallback = function(error) {
+    if (!verifyErrorForFailure(error))
+      return;
     fileSystemProviderInternal.operationRequestedError(
         options.fileSystemId, options.requestId, error,
         Date.now() - executionStart);
@@ -188,6 +204,8 @@ eventBindings.registerArgumentMassager(
       };
 
       var onErrorCallback = function(error) {
+        if (!verifyErrorForFailure(error))
+          return;
         fileSystemProviderInternal.operationRequestedError(
             options.fileSystemId, options.requestId, error,
             Date.now() - executionStart);
@@ -210,6 +228,8 @@ eventBindings.registerArgumentMassager(
       };
 
       var onErrorCallback = function(error) {
+        if (!verifyErrorForFailure(error))
+          return;
         fileSystemProviderInternal.operationRequestedError(
             options.fileSystemId, options.requestId, error,
             Date.now() - executionStart);
@@ -246,6 +266,8 @@ eventBindings.registerArgumentMassager(
       };
 
       var onErrorCallback = function(error) {
+        if (!verifyErrorForFailure(error))
+          return;
         fileSystemProviderInternal.operationRequestedError(
             options.fileSystemId, options.requestId, error,
             Date.now() - executionStart);
@@ -272,6 +294,8 @@ eventBindings.registerArgumentMassager(
             Date.now() - executionStart);
       };
       var onErrorCallback = function(error) {
+        if (!verifyErrorForFailure(error))
+          return;
         fileSystemProviderInternal.operationRequestedError(
             options.fileSystemId, options.requestId, error,
             Date.now() - executionStart);
@@ -342,6 +366,7 @@ eventBindings.registerArgumentMassager(
         // TODO(mtomasz): To be implemented.
       };
       var onErrorCallback = function(error) {
+        verifyErrorForFailure(error);

[benwells, 2017/01/30 01:50:50]

Nit: should this bail on failure like the other onErrorCallbacks?

[mtomasz, 2017/02/21 05:25:59]

Do you mean return if returned false? It would be a no-op here. If your question
is why we don't call fileSystemProviderInternal.operationRequestError here like
in other onErrorCallbacks, then the answer is I don't remember :(.

It's quite parallel to the issue this CL is addressing, so would you mind if I
look at it separately?

[benwells, 2017/02/21 05:35:11]

That is what I meant. It would be a no-op, but would be clear to whoever has to
come and implement the TODO.

If your question

[mtomasz, 2017/02/21 05:56:43]

I took a look at the original CL adding this TODO, and there is nothing to
implement here until we have more clients dispatching this event. It's unlikely
to happen, so removed the TODO and replaced with a comment.

         // TODO(mtomasz): To be implemented.
       }
       dispatch([onSuccessCallback, onErrorCallback]);