Separate validation failures from other failures in ExecuteCodeFunction.

extensions/browser/api/execute_code_function.h

Index: extensions/browser/api/execute_code_function.h
diff --git a/extensions/browser/api/execute_code_function.h b/extensions/browser/api/execute_code_function.h
index dbf8639e9ff06378db37fc269e8c8c7b1364b5f7..a44527aa1038c232040416487175cd9c7ea42515 100644
--- a/extensions/browser/api/execute_code_function.h
+++ b/extensions/browser/api/execute_code_function.h
@@ -6,6 +6,7 @@
 #define EXTENSIONS_BROWSER_API_EXECUTE_CODE_FUNCTION_H_
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "extensions/browser/extension_function.h"
 #include "extensions/browser/script_executor.h"
 #include "extensions/common/api/extension_types.h"
@@ -27,8 +28,18 @@ class ExecuteCodeFunction : public AsyncExtensionFunction {
   bool HasPermission() override;
   bool RunAsync() override;
 
-  // Initialize |details_| if it hasn't already been.
-  virtual bool Init() = 0;
+  enum InitResult {
+    // ExtensionFunction validation failure.

[Devlin, 2017/01/17 18:52:09]

We should describe the importance of these.  In particular, validation failures
kill the renderer and should only be used for this that should never, ever
happen; failures return an error and should be used when it could happen in
certain circumstances.

[lazyboy, 2017/01/18 02:29:10]

Done.

+    VALIDATION_FAILURE,
+    // Failure other than validation.
+    FAILURE,
+    SUCCESS
+  };
+
+  // Initializes |details_| and other variables if they haven't already been.
+  // Returns whether or not it succeeded. Failure can be tolerable (FAILURE), or
+  // fatal (VALIDATION_FAILURE).
+  virtual InitResult Init() = 0;
   virtual bool ShouldInsertCSS() const = 0;
   virtual bool CanExecuteScriptOnPage() = 0;
   virtual ScriptExecutor* GetScriptExecutor() = 0;
@@ -48,8 +59,20 @@ class ExecuteCodeFunction : public AsyncExtensionFunction {
   const HostID& host_id() const { return host_id_; }
   void set_host_id(const HostID& host_id) { host_id_ = host_id; }
 
+  InitResult set_init_result(InitResult init_result) {
+    init_result_ = init_result;
+    return init_result_.value();
+  }
+  InitResult set_init_result(InitResult init_result, const std::string& error) {

[Devlin, 2017/01/17 18:52:09]

This can only be called with FAILURE, right?  Can we assign that directly, or
DCHECK if you want to continue passing it in?

[lazyboy, 2017/01/18 02:29:10]

Assigned directly via renamed method set_init_result_failure.

+    init_error_ = error;
+    return set_init_result(init_result);
+  }
+
   // The injection details.
   std::unique_ptr<api::extension_types::InjectDetails> details_;
+  base::Optional<InitResult> init_result_;
+  // Set iff |init_result_| == FAILURE, holds the error string.
+  base::Optional<std::string> init_error_;
 
  private:
   // Retrieves the file url for the given |extension_path| and optionally