[Extensions] Isolate ExtensionFunction results_ and error_

extensions/browser/extension_function.h

 // 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 EXTENSIONS_BROWSER_EXTENSION_FUNCTION_H_
 #define EXTENSIONS_BROWSER_EXTENSION_FUNCTION_H_
 
 #include <stddef.h>
 
 #include <list>
@@ skipping 38 matching lines @@
 }
 
 namespace IPC {
 class Sender;
 }
 
 #ifdef NDEBUG
 #define EXTENSION_FUNCTION_VALIDATE(test) \
   do {                                    \
     if (!(test)) {                        \
-      this->bad_message_ = true;          \
+      this->set_bad_message(true);        \
       return ValidationFailure(this);     \
     }                                     \
   } while (0)
 #else   // NDEBUG
 #define EXTENSION_FUNCTION_VALIDATE(test) CHECK(test)
 #endif  // NDEBUG
 
 #ifdef NDEBUG
 #define EXTENSION_FUNCTION_PRERUN_VALIDATE(test) \
   do {                                           \
     if (!(test)) {                               \
-      this->bad_message_ = true;                 \
+      this->set_bad_message(true);               \
       return false;                              \
     }                                            \
   } while (0)
 #else  // NDEBUG
 #define EXTENSION_FUNCTION_PRERUN_VALIDATE(test) CHECK(test)
 #endif  // NDEBUG
 
 #define EXTENSION_FUNCTION_ERROR(error) \
   do {                                  \
     error_ = error;                     \
-    this->bad_message_ = true;          \
+    this->set_bad_message(true);        \
     return ValidationFailure(this);     \
   } while (0)
 
 // Declares a callable extension function with the given |name|. You must also
 // supply a unique |histogramvalue| used for histograms of extension function
 // invocation (add new ones at the end of the enum in
 // extension_function_histogram_value.h).
 #define DECLARE_EXTENSION_FUNCTION(name, histogramvalue) \
   public: static const char* function_name() { return name; } \
   public: static extensions::functions::HistogramValue histogram_value() \
@@ skipping 45 matching lines @@
   // The result of a function call.
   //
   // Use NoArguments(), OneArgument(), ArgumentList(), or Error()
   // rather than this class directly.
   class ResponseValueObject {
    public:
     virtual ~ResponseValueObject() {}
 
     // Returns true for success, false for failure.
     virtual bool Apply() = 0;
+
+   protected:
+    void SetFunctionResults(ExtensionFunction* function,
+                            std::unique_ptr<base::ListValue> results);
+    void SetFunctionError(ExtensionFunction* function,
+                          const std::string& error);
   };
   typedef std::unique_ptr<ResponseValueObject> ResponseValue;
 
   // The action to use when returning from RunAsync.
   //
   // Use RespondNow() or RespondLater() rather than this class directly.
   class ResponseActionObject {
    public:
     virtual ~ResponseActionObject() {}
 
@@ skipping 66 matching lines @@
       extensions::QuotaLimitHeuristics* heuristics) const {}
 
   // Called when the quota limit has been exceeded. The default implementation
   // returns an error.
   virtual void OnQuotaExceeded(const std::string& violation_error);
 
   // Specifies the raw arguments to the function, as a JSON value.
   // TODO(dcheng): This should take a const ref.
   virtual void SetArgs(const base::ListValue* args);
 
-  // Sets a single Value as the results of the function.
-  void SetResult(std::unique_ptr<base::Value> result);
-
-  // Sets multiple Values as the results of the function.
-  void SetResultList(std::unique_ptr<base::ListValue> results);
-
   // Retrieves the results of the function as a ListValue.
   const base::ListValue* GetResultList() const;
 
   // Retrieves any error string from the function.
-  virtual std::string GetError() const;
+  virtual const std::string& GetError() const;
 
   // Sets the function's error string.
+  // TODO(devlin): This should be handled exclusively through the responses.
   virtual void SetError(const std::string& error);
 
-  // Sets the function's bad message state.
+  bool bad_message() const { return bad_message_; }
   void set_bad_message(bool bad_message) { bad_message_ = bad_message; }
 
   // Specifies the name of the function. A long-lived string (such as a string
   // literal) must be provided.
   void set_name(const char* name) { name_ = name; }
   const char* name() const { return name_; }
 
   void set_profile_id(void* profile_id) { profile_id_ = profile_id; }
   void* profile_id() const { return profile_id_; }
 
@@ skipping 50 matching lines @@
 
   void set_source_process_id(int source_process_id) {
     source_process_id_ = source_process_id;
   }
   int source_process_id() const {
     return source_process_id_;
   }
 
   ResponseType* response_type() const { return response_type_.get(); }
 
+  bool did_respond() const { return did_respond_; }
+
   // Sets did_respond_ to true so that the function won't DCHECK if it never
   // sends a response. Typically, this shouldn't be used, even in testing. It's
   // only for when you want to test functionality that doesn't exercise the
   // Run() aspect of an extension function.
   void ignore_did_respond_for_testing() { did_respond_ = true; }
   // Same as above, but global. Yuck. Do not add any more uses of this.
   static bool ignore_all_did_respond_for_testing_do_not_use;
 
  protected:
   friend struct ExtensionFunctionDeleteTraits;
@@ skipping 68 matching lines @@
   virtual void Destruct() const = 0;
 
   // Called after the response is sent, allowing the function to perform any
   // additional work or cleanup.
   virtual void OnResponded() {}
 
   // Return true if the argument to this function at |index| was provided and
   // is non-null.
   bool HasOptionalArgument(size_t index);
 
+  // The extension that called this function.
+  scoped_refptr<const extensions::Extension> extension_;
+
+  // The arguments to the API. Only non-null if argument were specified.
+  std::unique_ptr<base::ListValue> args_;
+
+ private:
+  friend class ResponseValueObject;
+
+  // Call with true to indicate success, false to indicate failure. If this
+  // failed, |error_| should be set.
+  void SendResponseImpl(bool success);
+
+  base::ElapsedTimer timer_;
+
+  // The results of the API. This should be populated through the Respond()/
+  // RespondNow() methods. In legacy implementations, this is set directly, and
+  // should be set before calling SendResponse().
+  std::unique_ptr<base::ListValue> results_;
+
+  // Any detailed error from the API. This should be populated by the derived
+  // class before Run() returns.
+  std::string error_;
+
+  // The callback to run once the function has done execution.
+  ResponseCallback response_callback_;
+
   // Id of this request, used to map the response back to the caller.
   int request_id_;
 
   // The id of the profile of this function's extension.
   void* profile_id_;
 
-  // The extension that called this function.
-  scoped_refptr<const extensions::Extension> extension_;
-
   // The name of this function.
   const char* name_;
 
   // The URL of the frame which is making this request
   GURL source_url_;
 
   // True if the js caller provides a callback function to receive the response
   // of this call.
   bool has_callback_;
 
   // True if this callback should include information from incognito contexts
   // even if our profile_ is non-incognito. Note that in the case of a "split"
   // mode extension, this will always be false, and we will limit access to
   // data from within the same profile_ (either incognito or not).
   bool include_incognito_;
 
   // True if the call was made in response of user gesture.
   bool user_gesture_;
 
-  // The arguments to the API. Only non-null if argument were specified.
-  std::unique_ptr<base::ListValue> args_;
-
-  // The results of the API. This should be populated through the Respond()/
-  // RespondNow() methods. In legacy implementations, this is set directly, and
-  // should be set before calling SendResponse().
-  std::unique_ptr<base::ListValue> results_;
-
-  // Any detailed error from the API. This should be populated by the derived
-  // class before Run() returns.
-  std::string error_;
-
   // Any class that gets a malformed message should set this to true before
   // returning.  Usually we want to kill the message sending process.
   bool bad_message_;
 
   // The sample value to record with the histogram API when the function
   // is invoked.
   extensions::functions::HistogramValue histogram_value_;
 
-  // The callback to run once the function has done execution.
-  ResponseCallback response_callback_;
-
   // The ID of the tab triggered this function call, or -1 if there is no tab.
   int source_tab_id_;
 
   // The type of the JavaScript context where this call originated.
   extensions::Feature::Context source_context_type_;
 
   // The process ID of the page that triggered this function call, or -1
   // if unknown.
   int source_process_id_;
 
-  // Whether this function has responded.
-  bool did_respond_;
-
- private:
-  // Call with true to indicate success, false to indicate failure. If this
-  // failed, |error_| should be set.
-  void SendResponseImpl(bool success);
-
-  base::ElapsedTimer timer_;
-
   // The response type of the function, if the response has been sent.
   std::unique_ptr<ResponseType> response_type_;
 
+  // Whether this function has responded.
+  // TODO(devlin): Replace this with response_type_ != null.
+  bool did_respond_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionFunction);
 };
 
 // Extension functions that run on the UI thread. Most functions fall into
 // this category.
 class UIThreadExtensionFunction : public ExtensionFunction {
  public:
   UIThreadExtensionFunction();
 
   UIThreadExtensionFunction* AsUIThreadExtensionFunction() override;
@@ skipping 131 matching lines @@
 
   DISALLOW_COPY_AND_ASSIGN(IOThreadExtensionFunction);
 };
 
 // Base class for an extension function that runs asynchronously *relative to
 // the browser's UI thread*.
 class AsyncExtensionFunction : public UIThreadExtensionFunction {
  public:
   AsyncExtensionFunction();
 
+  // ExtensionFunction:
+  void SetError(const std::string& error) override;
+  const std::string& GetError() const override;
+
  protected:
   ~AsyncExtensionFunction() override;
 
+  // Sets a single Value as the results of the function.
+  void SetResult(std::unique_ptr<base::Value> result);
+
+  // Sets multiple Values as the results of the function.
+  void SetResultList(std::unique_ptr<base::ListValue> results);
+
   // Deprecated: Override UIThreadExtensionFunction and implement Run() instead.
   //
   // AsyncExtensionFunctions implement this method. Return true to indicate that
   // nothing has gone wrong yet; SendResponse must be called later. Return false
   // to respond immediately with an error.
   virtual bool RunAsync() = 0;
 
   // ValidationFailure override to match RunAsync().
   static bool ValidationFailure(AsyncExtensionFunction* function);
 
   // Responds with success/failure. |results_| or |error_| should be set
   // accordingly.
   void SendResponse(bool success);
 
+  // Exposed versions of |results_| and |error_| which are curried into the
+  // ExtensionFunction response.
+  // These need to keep the same name to avoid breaking existing
+  // implementations, but this should be temporary with crbug.com/648275
+  // and crbug.com/634140.
+  std::unique_ptr<base::ListValue> results_;
+  std::string error_;
+
  private:
   // If you're hitting a compile error here due to "final" - great! You're
   // doing the right thing, you just need to extend UIThreadExtensionFunction
   // instead of AsyncExtensionFunction.
   ResponseAction Run() final;
 
   DISALLOW_COPY_AND_ASSIGN(AsyncExtensionFunction);
 };
 
 #endif  // EXTENSIONS_BROWSER_EXTENSION_FUNCTION_H_