[Extensions Bindings] Move request dispatch to APIRequestHandler

extensions/renderer/api_request_handler_unittest.cc

Index: extensions/renderer/api_request_handler_unittest.cc
diff --git a/extensions/renderer/api_request_handler_unittest.cc b/extensions/renderer/api_request_handler_unittest.cc
index 915c19718a03d58fdb74d3bf27db7794029e45ab..a54445301c0a6759474b35d19ce9a9a7e8cee754 100644
--- a/extensions/renderer/api_request_handler_unittest.cc
+++ b/extensions/renderer/api_request_handler_unittest.cc
@@ -2,12 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "extensions/renderer/api_request_handler.h"
 #include "base/bind.h"
+#include "base/memory/ptr_util.h"
 #include "base/optional.h"
 #include "base/values.h"
 #include "extensions/renderer/api_binding_test.h"
 #include "extensions/renderer/api_binding_test_util.h"
-#include "extensions/renderer/api_request_handler.h"
 #include "gin/converter.h"
 #include "gin/function_template.h"
 #include "gin/public/context_holder.h"
@@ -23,9 +24,15 @@ namespace {
 const char kEchoArgs[] =
     "(function() { this.result = Array.from(arguments); })";
 
+const char kMethod[] = "method";
+
 // TODO(devlin): We should probably hoist this up to e.g. api_binding_types.h.
 using ArgumentList = std::vector<v8::Local<v8::Value>>;
 
+// TODO(devlin): Should we move some parts of api_binding_unittest.cc to here?
+void DoNothingWithRequest(std::unique_ptr<APIRequestHandler::Request> request,
+                          v8::Local<v8::Context> context) {}
+
 }  // namespace
 
 class APIRequestHandlerTest : public APIBindingTest {
@@ -58,6 +65,7 @@ TEST_F(APIRequestHandlerTest, AddRequestAndCompleteRequestTest) {
   v8::Local<v8::Context> context = ContextLocal();
 
   APIRequestHandler request_handler(
+      base::Bind(&DoNothingWithRequest),
       base::Bind(&APIRequestHandlerTest::RunJS, base::Unretained(this)),
       APILastError(APILastError::GetParent()));
 
@@ -66,8 +74,9 @@ TEST_F(APIRequestHandlerTest, AddRequestAndCompleteRequestTest) {
   v8::Local<v8::Function> function = FunctionFromString(context, kEchoArgs);
   ASSERT_FALSE(function.IsEmpty());
 
-  int request_id = request_handler.AddPendingRequest(isolate(), function,
-                                                     context, ArgumentList());
+  int request_id = request_handler.StartRequest(
+      context, kMethod, base::MakeUnique<base::ListValue>(), function,
+      v8::Local<v8::Function>());
   EXPECT_THAT(request_handler.GetPendingRequestIdsForTesting(),
               testing::UnorderedElementsAre(request_id));
 
@@ -91,14 +100,16 @@ TEST_F(APIRequestHandlerTest, InvalidRequestsTest) {
   v8::Local<v8::Context> context = ContextLocal();
 
   APIRequestHandler request_handler(
+      base::Bind(&DoNothingWithRequest),
       base::Bind(&APIRequestHandlerTest::RunJS, base::Unretained(this)),
       APILastError(APILastError::GetParent()));
 
   v8::Local<v8::Function> function = FunctionFromString(context, kEchoArgs);
   ASSERT_FALSE(function.IsEmpty());
 
-  int request_id = request_handler.AddPendingRequest(isolate(), function,
-                                                     context, ArgumentList());
+  int request_id = request_handler.StartRequest(
+      context, kMethod, base::MakeUnique<base::ListValue>(), function,
+      v8::Local<v8::Function>());
   EXPECT_THAT(request_handler.GetPendingRequestIdsForTesting(),
               testing::UnorderedElementsAre(request_id));
 
@@ -127,6 +138,7 @@ TEST_F(APIRequestHandlerTest, MultipleRequestsAndContexts) {
   holder_b.SetContext(context_b);
 
   APIRequestHandler request_handler(
+      base::Bind(&DoNothingWithRequest),
       base::Bind(&APIRequestHandlerTest::RunJS, base::Unretained(this)),
       APILastError(APILastError::GetParent()));
 
@@ -138,10 +150,12 @@ TEST_F(APIRequestHandlerTest, MultipleRequestsAndContexts) {
   v8::Local<v8::Function> function_b = FunctionFromString(
       context_b, "(function(res) { this.result = res + 'beta'; })");
 
-  int request_a = request_handler.AddPendingRequest(isolate(), function_a,
-                                                    context_a, ArgumentList());
-  int request_b = request_handler.AddPendingRequest(isolate(), function_b,
-                                                    context_b, ArgumentList());
+  int request_a = request_handler.StartRequest(
+      context_a, kMethod, base::MakeUnique<base::ListValue>(), function_a,
+      v8::Local<v8::Function>());
+  int request_b = request_handler.StartRequest(
+      context_b, kMethod, base::MakeUnique<base::ListValue>(), function_b,
+      v8::Local<v8::Function>());
 
   EXPECT_THAT(request_handler.GetPendingRequestIdsForTesting(),
               testing::UnorderedElementsAre(request_a, request_b));
@@ -176,30 +190,42 @@ TEST_F(APIRequestHandlerTest, CustomCallbackArguments) {
   v8::Local<v8::Context> context = ContextLocal();
 
   APIRequestHandler request_handler(
+      base::Bind(&DoNothingWithRequest),
       base::Bind(&APIRequestHandlerTest::RunJS, base::Unretained(this)),
       APILastError(APILastError::GetParent()));
 
-  ArgumentList custom_callback_args = {
-      gin::StringToV8(isolate(), "to"), gin::StringToV8(isolate(), "be"),
-  };
+  v8::Local<v8::Function> custom_callback =
+      FunctionFromString(context, kEchoArgs);
+  v8::Local<v8::Function> callback =
+      FunctionFromString(context, "(function() {})");
+  ASSERT_FALSE(callback.IsEmpty());
+  ASSERT_FALSE(custom_callback.IsEmpty());
 
-  v8::Local<v8::Function> function = FunctionFromString(context, kEchoArgs);
-  ASSERT_FALSE(function.IsEmpty());
-
-  int request_id = request_handler.AddPendingRequest(
-      isolate(), function, context, custom_callback_args);
+  int request_id = request_handler.StartRequest(
+      context, "method", base::MakeUnique<base::ListValue>(), callback,
+      custom_callback);
   EXPECT_THAT(request_handler.GetPendingRequestIdsForTesting(),
               testing::UnorderedElementsAre(request_id));
 
   std::unique_ptr<base::ListValue> response_arguments =
-      ListValueFromString("['or','not','to','be']");
+      ListValueFromString("['response', 'arguments']");
   ASSERT_TRUE(response_arguments);
   request_handler.CompleteRequest(request_id, *response_arguments,
                                   std::string());
 
   EXPECT_TRUE(did_run_js());
-  EXPECT_EQ(ReplaceSingleQuotes("['to','be','or','not','to','be']"),
-            GetStringPropertyFromObject(context->Global(), context, "result"));
+  v8::Local<v8::Value> result =
+      GetPropertyFromObject(context->Global(), context, "result");
+  ASSERT_FALSE(result.IsEmpty());
+  ASSERT_TRUE(result->IsArray());
+  ArgumentList args;
+  ASSERT_TRUE(gin::Converter<ArgumentList>::FromV8(isolate(), result, &args));
+  ASSERT_EQ(5u, args.size());
+  EXPECT_EQ("\"method\"", V8ToString(args[0], context));
+  EXPECT_EQ("{}", V8ToString(args[1], context));
+  EXPECT_EQ(callback, args[2]);
+  EXPECT_EQ("\"response\"", V8ToString(args[3], context));
+  EXPECT_EQ("\"arguments\"", V8ToString(args[4], context));
 
   EXPECT_TRUE(request_handler.GetPendingRequestIdsForTesting().empty());
 }
@@ -210,6 +236,7 @@ TEST_F(APIRequestHandlerTest, UserGestureTest) {
   v8::Local<v8::Context> context = ContextLocal();
 
   APIRequestHandler request_handler(
+      base::Bind(&DoNothingWithRequest),
       base::Bind(&APIRequestHandlerTest::RunJS, base::Unretained(this)),
       APILastError(APILastError::GetParent()));
 
@@ -228,8 +255,9 @@ TEST_F(APIRequestHandlerTest, UserGestureTest) {
       function_template->GetFunction(context).ToLocalChecked();
 
   // Try first without a user gesture.
-  int request_id = request_handler.AddPendingRequest(isolate(), v8_callback,
-                                                     context, ArgumentList());
+  int request_id = request_handler.StartRequest(
+      context, kMethod, base::MakeUnique<base::ListValue>(), v8_callback,
+      v8::Local<v8::Function>());
   request_handler.CompleteRequest(request_id, *ListValueFromString("[]"),
                                   std::string());
 
@@ -243,8 +271,9 @@ TEST_F(APIRequestHandlerTest, UserGestureTest) {
     blink::WebScopedUserGesture user_gesture(nullptr);
     EXPECT_TRUE(
         blink::WebUserGestureIndicator::isProcessingUserGestureThreadSafe());
-    request_id = request_handler.AddPendingRequest(isolate(), v8_callback,
-                                                   context, ArgumentList());
+    request_id = request_handler.StartRequest(
+        context, kMethod, base::MakeUnique<base::ListValue>(), v8_callback,
+        v8::Local<v8::Function>());
   }
   EXPECT_FALSE(
       blink::WebUserGestureIndicator::isProcessingUserGestureThreadSafe());