[Extensions] Apply WARN_UNUSED_RESULT and final keyword to ExtensionFunctions.

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 <list>
 #include <string>
 
@@ skipping 303 matching lines @@
   // It shouldn't be possible to have both an error *and* some arguments.
   // Some legacy APIs do rely on it though, like webstorePrivate.
   ResponseValue ErrorWithArguments(scoped_ptr<base::ListValue> args,
                                    const std::string& error);
   // Bad message. A ResponseValue equivalent to EXTENSION_FUNCTION_VALIDATE(),
   // so this will actually kill the renderer and not respond at all.
   ResponseValue BadMessage();
 
   // ResponseActions.
   //
+  // These are exclusively used as return values from Run(). Call Respond(...)
+  // to respond at any other time - but as described below, only after Run()
+  // has already executed, and only if it returned RespondLater().
+  //

[Devlin, 2015/02/25 16:41:58]

More as a note-to-self than anything, but we should enforce this with a CHECK
somewhere (thinking something as simple as a "said_respond_later_" private bool
of ExtensionFunction that can be set when RespondLater()ing).

[not at google - send to devlin, 2015/02/25 16:53:57]

There is a reason I didn't do this - the most logical place to put this check
was in SendResponse, but the old code uses this too - and the old code has bugs.
I'd need to introduce some extra complexity to bypass sanity checks in some
cases. Not to say it's not worthwhile, I just didn't want the churn at the time.

   // Respond to the extension immediately with |result|.
-  ResponseAction RespondNow(ResponseValue result);
+  ResponseAction RespondNow(ResponseValue result) WARN_UNUSED_RESULT;
   // Don't respond now, but promise to call Respond(...) later.
-  ResponseAction RespondLater();
+  ResponseAction RespondLater() WARN_UNUSED_RESULT;
 
   // This is the return value of the EXTENSION_FUNCTION_VALIDATE macro, which
   // needs to work from Run(), RunAsync(), and RunSync(). The former of those
   // has a different return type (ResponseAction) than the latter two (bool).
-  static ResponseAction ValidationFailure(ExtensionFunction* function);
+  static ResponseAction ValidationFailure(ExtensionFunction* function)
+      WARN_UNUSED_RESULT;
 
-  // If RespondLater() was used, functions must at some point call Respond()
-  // with |result| as their result.
+  // If RespondLater() was returned from Run(), functions must at some point
+  // call Respond() with |result| as their result.

[Devlin, 2015/02/25 16:41:58]

Same - I kind of wonder if we can CHECK on destruction, but shutdown/other cases
where we destroy this without finishing might make it complicated.

[not at google - send to devlin, 2015/02/25 16:53:57]

Indeed. Even more complex.

+  //
+  // More specifically: call this iff Run() has already executed, it returned
+  // RespondLater(), and Respond(...) hasn't already been called.
   void Respond(ResponseValue result);
 
   virtual ~ExtensionFunction();
 
   // Helper method for ExtensionFunctionDeleteTraits. Deletes this object.
   virtual void Destruct() const = 0;
 
   // Do not call this function directly, return the appropriate ResponseAction
   // from Run() instead. If using RespondLater then call Respond().
   //
@@ skipping 235 matching lines @@
   //
   // 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);
 
  private:
-  ResponseAction Run() override;
+  // 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;
 };
 
 // A SyncExtensionFunction is an ExtensionFunction that runs synchronously
 // *relative to the browser's UI thread*. Note that this has nothing to do with
 // running synchronously relative to the extension process. From the extension
 // process's point of view, the function is still asynchronous.
 //
 // This kind of function is convenient for implementing simple APIs that just
 // need to interact with things on the browser UI thread.
 class SyncExtensionFunction : public UIThreadExtensionFunction {
  public:
   SyncExtensionFunction();
 
  protected:
   ~SyncExtensionFunction() override;
 
   // Deprecated: Override UIThreadExtensionFunction and implement Run() instead.
   //
   // SyncExtensionFunctions implement this method. Return true to respond
   // immediately with success, false to respond immediately with an error.
   virtual bool RunSync() = 0;
 
   // ValidationFailure override to match RunSync().
   static bool ValidationFailure(SyncExtensionFunction* function);
 
  private:
-  ResponseAction Run() override;
+  // 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 SyncExtensionFunction.
+  ResponseAction Run() final;
 };
 
 class SyncIOThreadExtensionFunction : public IOThreadExtensionFunction {
  public:
   SyncIOThreadExtensionFunction();
 
  protected:
   ~SyncIOThreadExtensionFunction() override;
 
   // Deprecated: Override IOThreadExtensionFunction and implement Run() instead.
   //
   // SyncIOThreadExtensionFunctions implement this method. Return true to
   // respond immediately with success, false to respond immediately with an
   // error.
   virtual bool RunSync() = 0;
 
   // ValidationFailure override to match RunSync().
   static bool ValidationFailure(SyncIOThreadExtensionFunction* function);
 
  private:
-  ResponseAction Run() override;
+  // If you're hitting a compile error here due to "final" - great! You're
+  // doing the right thing, you just need to extend IOThreadExtensionFunction
+  // instead of SyncIOExtensionFunction.
+  ResponseAction Run() final;
 };
 
 #endif  // EXTENSIONS_BROWSER_EXTENSION_FUNCTION_H_