Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(929)

Unified Diff: extensions/renderer/api_request_handler.cc

Issue 2894923003: [Extensions Bindings] Include request id in a custom callback response (Closed)
Patch Set: Add comment Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: extensions/renderer/api_request_handler.cc
diff --git a/extensions/renderer/api_request_handler.cc b/extensions/renderer/api_request_handler.cc
index 369cfcb1495bafd50dfb8f4d3e697e1e5a409205..52b123d7d0853a6516ac8ee137261c66631990d9 100644
--- a/extensions/renderer/api_request_handler.cc
+++ b/extensions/renderer/api_request_handler.cc
@@ -10,6 +10,7 @@
#include "base/values.h"
#include "content/public/child/v8_value_converter.h"
#include "gin/converter.h"
+#include "gin/data_object_builder.h"
#include "third_party/WebKit/public/web/WebScopedUserGesture.h"
#include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
@@ -57,6 +58,13 @@ int APIRequestHandler::StartRequest(v8::Local<v8::Context> context,
binding::RequestThread thread) {
auto request = base::MakeUnique<Request>();
+ // The request id is primarily used in the renderer to associate an API
+ // request with the associated callback, but it's also used in the browser as
+ // an identifier for the extension function (e.g. by the pageCapture API).
+ // TODO(devlin): We should probably fix this, since the request id is only
+ // unique per-isolate, rather than globally.
+ int request_id = next_request_id_++;
+
if (!custom_callback.IsEmpty() || !callback.IsEmpty()) {
v8::Isolate* isolate = context->GetIsolate();
// In the JS bindings, custom callbacks are called with the arguments of
@@ -69,9 +77,10 @@ int APIRequestHandler::StartRequest(v8::Local<v8::Context> context,
// properties for callback, callbackSchema, args, stack, id, and
// customCallback. Of those, it appears that we only use stack, args, and
// id (since callback is curried in separately). We may be able to update
- // bindings to get away from some of those. For now, just pass in an empty
- // object (most APIs don't rely on it).
- v8::Local<v8::Object> request = v8::Object::New(isolate);
+ // bindings to get away from some of those. For now, just pass in an
+ // object with the request id.
+ v8::Local<v8::Object> request =
+ gin::DataObjectBuilder(isolate).Set("id", request_id).Build();
v8::Local<v8::Value> callback_to_pass = callback;
if (callback_to_pass.IsEmpty())
callback_to_pass = v8::Undefined(isolate);
@@ -85,7 +94,7 @@ int APIRequestHandler::StartRequest(v8::Local<v8::Context> context,
// we use that instead? It means updating the IPC
// (ExtensionHostMsg_Request).
// base::UnguessableToken is another good option.
- request->request_id = next_request_id_++;
+ request->request_id = request_id;
request->has_callback = true;
pending_requests_.insert(std::make_pair(
request->request_id,

Powered by Google App Engine
This is Rietveld 408576698