diff options
Diffstat (limited to 'chromium/extensions/renderer/bindings')
7 files changed, 88 insertions, 27 deletions
diff --git a/chromium/extensions/renderer/bindings/api_binding.cc b/chromium/extensions/renderer/bindings/api_binding.cc index 16c3ef8b279..322d32c7b88 100644 --- a/chromium/extensions/renderer/bindings/api_binding.cc +++ b/chromium/extensions/renderer/bindings/api_binding.cc @@ -65,24 +65,32 @@ struct SignaturePair { std::unique_ptr<APISignature> callback_signature; }; -SignaturePair GetAPISignatureFromDictionary(const base::DictionaryValue* dict) { - const base::ListValue* params = nullptr; - CHECK(dict->GetList("parameters", ¶ms)); +SignaturePair GetAPISignatureFromDictionary(const base::Value* dict) { + const base::Value* params = + dict->FindKeyOfType("parameters", base::Value::Type::LIST); + CHECK(params); - bool supports_promises = false; - dict->GetBoolean("supportsPromises", &supports_promises); + // The inclusion of the "returns_async" property indicates that an API + // supports promises. + const base::Value* returns_async = + dict->FindKeyOfType("returns_async", base::Value::Type::DICTIONARY); SignaturePair result; - result.method_signature = std::make_unique<APISignature>(*params); + result.method_signature = + std::make_unique<APISignature>(*params, returns_async); result.method_signature->set_promise_support( - supports_promises ? binding::PromiseSupport::kAllowed - : binding::PromiseSupport::kDisallowed); + returns_async && APIBinding::enable_promise_support_for_testing + ? binding::PromiseSupport::kAllowed + : binding::PromiseSupport::kDisallowed); // If response validation is enabled, parse the callback signature. Otherwise, // there's no reason to, so don't bother. if (result.method_signature->has_callback() && binding::IsResponseValidationEnabled()) { - const base::Value* callback_params = params->GetList().back().FindKeyOfType( - "parameters", base::Value::Type::LIST); + const base::Value* callback_params = + returns_async ? returns_async->FindKeyOfType("parameters", + base::Value::Type::LIST) + : params->GetList().back().FindKeyOfType( + "parameters", base::Value::Type::LIST); if (callback_params) { const base::ListValue* params_as_list = nullptr; callback_params->GetAsList(¶ms_as_list); @@ -532,6 +540,9 @@ void APIBinding::DecorateTemplateWithProperties( } // static +bool APIBinding::enable_promise_support_for_testing = false; + +// static void APIBinding::GetEventObject( v8::Local<v8::Name> property, const v8::PropertyCallbackInfo<v8::Value>& info) { diff --git a/chromium/extensions/renderer/bindings/api_binding.h b/chromium/extensions/renderer/bindings/api_binding.h index e62af331e4d..63b27c47f3f 100644 --- a/chromium/extensions/renderer/bindings/api_binding.h +++ b/chromium/extensions/renderer/bindings/api_binding.h @@ -83,6 +83,11 @@ class APIBinding { APIBindingHooks* hooks() { return binding_hooks_.get(); } + // Global bool to allow for testing of promise support. + // TODO(tjudkins): Replace this with a runtime determined condition gated on + // MV3. + static bool enable_promise_support_for_testing; + private: // Initializes the object_template_ for this API. Called lazily when the // first instance is created. diff --git a/chromium/extensions/renderer/bindings/api_binding_unittest.cc b/chromium/extensions/renderer/bindings/api_binding_unittest.cc index b2a29346cff..b56bb87e81b 100644 --- a/chromium/extensions/renderer/bindings/api_binding_unittest.cc +++ b/chromium/extensions/renderer/bindings/api_binding_unittest.cc @@ -4,6 +4,7 @@ #include "extensions/renderer/bindings/api_binding.h" +#include "base/auto_reset.h" #include "base/bind.h" #include "base/bind_helpers.h" #include "base/macros.h" @@ -1672,21 +1673,22 @@ TEST_F(APIBindingUnittest, // Tests promise-based APIs exposed on bindings. TEST_F(APIBindingUnittest, PromiseBasedAPIs) { + // TODO(tjudkins): Remove this once promise support is fully integrated into + // the API calling flow. + base::AutoReset<bool> auto_reset( + &APIBinding::enable_promise_support_for_testing, true); + constexpr char kFunctions[] = R"([{ 'name': 'supportsPromises', - 'supportsPromises': true, 'parameters': [{ 'name': 'int', 'type': 'integer' - }, { - 'name': 'callback', - 'type': 'function', - 'parameters': [{ - 'name': 'strResult', - 'type': 'string' - }] - }] + }], + "returns_async": { + 'name': 'strResult', + 'type': 'string' + } }])"; SetFunctions(kFunctions); @@ -1701,7 +1703,7 @@ TEST_F(APIBindingUnittest, PromiseBasedAPIs) { R"((function(api) { this.apiResult = api.supportsPromises(3); this.apiResult.then((strResult) => { - this.strResult = strResult; + this.promiseResult = strResult; }); }))"; v8::Local<v8::Function> promise_api_call = @@ -1723,8 +1725,29 @@ TEST_F(APIBindingUnittest, PromiseBasedAPIs) { EXPECT_EQ(v8::Promise::kFulfilled, promise->State()); EXPECT_EQ(R"("foo")", V8ToString(promise->Result(), context)); - EXPECT_EQ(R"("foo")", GetStringPropertyFromObject(context->Global(), - context, "strResult")); + EXPECT_EQ(R"("foo")", GetStringPropertyFromObject( + context->Global(), context, "promiseResult")); + } + // Also test that promise-based APIs still support passing a callback. + { + constexpr char kFunctionCall[] = + R"((function(api) { + api.supportsPromises(3, (strResult) => { + this.callbackResult = strResult + }); + }))"; + v8::Local<v8::Function> promise_api_call = + FunctionFromString(context, kFunctionCall); + v8::Local<v8::Value> args[] = {binding_object}; + RunFunctionOnGlobal(promise_api_call, context, base::size(args), args); + + ASSERT_TRUE(last_request()); + request_handler()->CompleteRequest(last_request()->request_id, + *ListValueFromString(R"(["bar"])"), + std::string()); + + EXPECT_EQ(R"("bar")", GetStringPropertyFromObject( + context->Global(), context, "callbackResult")); } } diff --git a/chromium/extensions/renderer/bindings/api_signature.cc b/chromium/extensions/renderer/bindings/api_signature.cc index 03b34af7ad7..e617533e508 100644 --- a/chromium/extensions/renderer/bindings/api_signature.cc +++ b/chromium/extensions/renderer/bindings/api_signature.cc @@ -19,10 +19,9 @@ namespace extensions { namespace { bool HasCallback(const std::vector<std::unique_ptr<ArgumentSpec>>& signature) { - // TODO(devlin): This is how extension APIs have always determined if a - // function has a callback, but it seems a little silly. In the long run (once - // signatures are generated), it probably makes sense to indicate this - // differently. + // TODO(tjudkins): Once we change the APISignature to represent the whole + // signature including any asynchronous return, we should replace this with a + // method on the APISignature object itself. return !signature.empty() && signature.back()->type() == ArgumentType::FUNCTION; } @@ -409,6 +408,27 @@ APISignature::APISignature(const base::ListValue& specification) { has_callback_ = HasCallback(signature_); } +APISignature::APISignature(const base::Value& specification_list, + bool supports_promises) { + auto size = specification_list.GetList().size() + (supports_promises ? 1 : 0); + signature_.reserve(size); + for (const auto& value : specification_list.GetList()) { + CHECK(value.is_dict()); + signature_.push_back(std::make_unique<ArgumentSpec>(value)); + } + // To allow promise supporting APIs to instead take a callback, we add an + // allowed function to the end of the signature. + if (supports_promises) { + auto callback = std::make_unique<ArgumentSpec>(ArgumentType::FUNCTION); + callback->set_name("callback"); + signature_.push_back(std::move(callback)); + } + + has_callback_ = HasCallback(signature_); + DCHECK(!supports_promises || has_callback_) + << "If an API supports promises, it must also support callbacks"; +} + APISignature::APISignature(std::vector<std::unique_ptr<ArgumentSpec>> signature) : signature_(std::move(signature)), has_callback_(HasCallback(signature_)) {} diff --git a/chromium/extensions/renderer/bindings/api_signature.h b/chromium/extensions/renderer/bindings/api_signature.h index 6cca11efd3e..814bb273976 100644 --- a/chromium/extensions/renderer/bindings/api_signature.h +++ b/chromium/extensions/renderer/bindings/api_signature.h @@ -28,6 +28,7 @@ class ArgumentSpec; class APISignature { public: explicit APISignature(const base::ListValue& specification); + APISignature(const base::Value& specification_list, bool supports_promises); explicit APISignature(std::vector<std::unique_ptr<ArgumentSpec>> signature); ~APISignature(); diff --git a/chromium/extensions/renderer/bindings/test_interaction_provider.cc b/chromium/extensions/renderer/bindings/test_interaction_provider.cc index f2d09c6bd9e..b4aa7415fe5 100644 --- a/chromium/extensions/renderer/bindings/test_interaction_provider.cc +++ b/chromium/extensions/renderer/bindings/test_interaction_provider.cc @@ -4,6 +4,8 @@ #include "extensions/renderer/bindings/test_interaction_provider.h" +#include "base/check.h" + namespace extensions { namespace { diff --git a/chromium/extensions/renderer/bindings/test_interaction_provider.h b/chromium/extensions/renderer/bindings/test_interaction_provider.h index 9fc794efabe..f3c28ea764c 100644 --- a/chromium/extensions/renderer/bindings/test_interaction_provider.h +++ b/chromium/extensions/renderer/bindings/test_interaction_provider.h @@ -7,7 +7,6 @@ #include "extensions/renderer/bindings/interaction_provider.h" -#include "base/logging.h" #include "base/macros.h" #include "v8/include/v8.h" |