WebstorePrivate extension API cleanup, part 1

extensions/browser/extension_function.h

Index: extensions/browser/extension_function.h
diff --git a/extensions/browser/extension_function.h b/extensions/browser/extension_function.h
index 0fccce5b50ea12008452f6f079ba180a305977b0..b16157c52f24ff5c860f2babd466f12883ff57ba 100644
--- a/extensions/browser/extension_function.h
+++ b/extensions/browser/extension_function.h
@@ -308,6 +308,12 @@ class ExtensionFunction
                       const std::string& s1,
                       const std::string& s2,
                       const std::string& s3);
+  // Error with a list of arguments |results| to pass to caller. TAKES OWNERSHIP
+  // - a scoped_ptr<> for convenience, since callers usually get this from the
+  // result of a Create(...) call on the generated Results struct, for example,
+  // alarms::Get::Results::Create(alarm).

[not at google - send to devlin, 2015/02/05 17:16:17]

It's funny actually - this ResponseAction/ResponseValue was a big refactor I did
a few months ago to avoid this exact scenario - APIs specifying both an error
and a result. It's supposed to be illegal, but some APIs (notably
webstorePrivate) still do it.

One big motivating factor is that if we convert to Promise APIs we won't have a
chance to pass errors and values in, since they're 2 separate callbacks. In that
situation we'd extend Error() to also take a Value instead of these format
strings.

However, given webstorePrivate does actually rely on this behavior, I suppose we
need to account for it.

I'd write it a little differently, in the spirit of better composition with
NoArguments/OneArgument/etc family -

// Using this ResponseValue indicates something is wrong with the API.
// 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(ResponseValue error, ResponseValue args)

[Marc Treib, 2015/02/06 10:26:15]

Done.

+  ResponseValue ArgumentListWithError(scoped_ptr<base::ListValue> results,
+                                      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();