[Extensions Bindings] Add a GetStringPropertyFromObject test method

Common test practice: compare a stringified property on an object to an
expected value (we use strings for easy readability if something is
unexpected). Add a method to streamline the process a bit.

BUG=653596

Review-Url: https://codereview.chromium.org/2601143002
Cr-Commit-Position: refs/heads/master@{#440904}

extensions/renderer/api_binding_test_util.cc

@@ -175,4 +175,21 @@ std::unique_ptr<base::Value> GetBaseValuePropertyFromObject(
   return V8ToBaseValue(GetPropertyFromObject(object, context, key), context);
 }
 
+std::string GetStringPropertyFromObject(v8::Local<v8::Object> object,
+                                        v8::Local<v8::Context> context,
+                                        base::StringPiece key) {
+  v8::Local<v8::Value> v8_val = GetPropertyFromObject(object, context, key);
+  if (v8_val.IsEmpty())
+    return "empty";
+  if (v8_val->IsNull())
+    return "null";
+  if (v8_val->IsUndefined())
+    return "undefined";
+  if (v8_val->IsFunction())
+    return "function";
+  std::unique_ptr<base::Value> json_prop = V8ToBaseValue(v8_val, context);
+  DCHECK(json_prop) << key;
+  return ValueToString(*json_prop);
+}
+
 }  // namespace extensions

extensions/renderer/api_binding_test_util.h

@@ -106,6 +106,12 @@ std::unique_ptr<base::Value> GetBaseValuePropertyFromObject(
     v8::Local<v8::Context> context,
     base::StringPiece key);
 
+// As above, but returns a JSON-serialized version of the value, or
+// "undefined", "null", "function", or "empty".
+std::string GetStringPropertyFromObject(v8::Local<v8::Object> object,
+                                        v8::Local<v8::Context> context,
+                                        base::StringPiece key);
+
 }  // extensions
 
 #endif  // EXTENSIONS_RENDERER_API_BINDING_TEST_UTIL_H_

extensions/renderer/api_bindings_system_unittest.cc

@@ -232,10 +232,9 @@ TEST_F(APIBindingsSystemTest, TestInitializationAndCallbacks) {
     bindings_system()->CompleteRequest(last_request()->request_id,
                                        *expected_args);
 
-    std::unique_ptr<base::Value> result = GetBaseValuePropertyFromObject(
-        context->Global(), context, "callbackArguments");
-    ASSERT_TRUE(result);
-    EXPECT_EQ(ReplaceSingleQuotes(kResponseArgsJson), ValueToString(*result));
+    EXPECT_EQ(ReplaceSingleQuotes(kResponseArgsJson),
+              GetStringPropertyFromObject(context->Global(), context,
+                                          "callbackArguments"));
     reset_last_request();
   }
 
@@ -255,10 +254,8 @@ TEST_F(APIBindingsSystemTest, TestInitializationAndCallbacks) {
     bindings_system()->CompleteRequest(last_request()->request_id,
                                        base::ListValue());
 
-    std::unique_ptr<base::Value> result = GetBaseValuePropertyFromObject(
-        context->Global(), context, "callbackArguments");
-    ASSERT_TRUE(result);
-    EXPECT_EQ("[]", ValueToString(*result));
+    EXPECT_EQ("[]", GetStringPropertyFromObject(context->Global(), context,
+                                                "callbackArguments"));
     reset_last_request();
   }
 
@@ -276,10 +273,9 @@ TEST_F(APIBindingsSystemTest, TestInitializationAndCallbacks) {
     bindings_system()->FireEventInContext("alpha.alphaEvent", context,
                                           *expected_args);
 
-    std::unique_ptr<base::Value> result = GetBaseValuePropertyFromObject(
-        context->Global(), context, "eventArguments");
-    ASSERT_TRUE(result);
-    EXPECT_EQ(ReplaceSingleQuotes(kResponseArgsJson), ValueToString(*result));
+    EXPECT_EQ(ReplaceSingleQuotes(kResponseArgsJson),
+              GetStringPropertyFromObject(context->Global(), context,
+                                          "eventArguments"));
   }
 
   {
@@ -339,10 +335,9 @@ TEST_F(APIBindingsSystemTest, TestCustomHooks) {
     CallFunctionOnObject(context, alpha_api, kTestCall);
     EXPECT_TRUE(did_call);
 
-    std::unique_ptr<base::Value> result = GetBaseValuePropertyFromObject(
-        context->Global(), context, "callbackArguments");
-    ASSERT_TRUE(result);
-    EXPECT_EQ("[\"bar\"]", ValueToString(*result));
+    EXPECT_EQ("[\"bar\"]",
+              GetStringPropertyFromObject(context->Global(), context,
+                                          "callbackArguments"));
   }
 }
 
@@ -485,16 +480,13 @@ TEST_F(APIBindingsSystemTestWithRealAPI, RealAPIs) {
         "  this.idleState = state;\n"
         "});\n";
     ExecuteScript(context, kTestCall);
-    v8::Local<v8::Value> v8_result =
-        GetPropertyFromObject(context->Global(), context, "idleState");
-    EXPECT_TRUE(v8_result->IsUndefined());
+    EXPECT_EQ("undefined", GetStringPropertyFromObject(context->Global(),
+                                                       context, "idleState"));
     bindings_system()->FireEventInContext("idle.onStateChanged", context,
                                           *ListValueFromString("['active']"));
 
-    std::unique_ptr<base::Value> result =
-        GetBaseValuePropertyFromObject(context->Global(), context, "idleState");
-    ASSERT_TRUE(result);
-    EXPECT_EQ("\"active\"", ValueToString(*result));
+    EXPECT_EQ("\"active\"", GetStringPropertyFromObject(context->Global(),
+                                                        context, "idleState"));
   }
 }
 

extensions/renderer/api_event_handler_unittest.cc

@@ -265,10 +265,9 @@ TEST_F(APIEventHandlerTest, EventArguments) {
   ASSERT_TRUE(event_args);
   handler.FireEventInContext(kEventName, context, *event_args);
 
-  std::unique_ptr<base::Value> result =
-      GetBaseValuePropertyFromObject(context->Global(), context, "eventArgs");
-  ASSERT_TRUE(result);
-  EXPECT_EQ(ReplaceSingleQuotes(kArguments), ValueToString(*result));
+  EXPECT_EQ(
+      ReplaceSingleQuotes(kArguments),
+      GetStringPropertyFromObject(context->Global(), context, "eventArgs"));
 }
 
 // Test dispatching events to multiple contexts.
@@ -328,16 +327,13 @@ TEST_F(APIEventHandlerTest, MultipleContexts) {
 
   handler.FireEventInContext(kEventName, context_a, *arguments_a);
   {
-    std::unique_ptr<base::Value> result_a = GetBaseValuePropertyFromObject(
-        context_a->Global(), context_a, "eventArgs");
-    ASSERT_TRUE(result_a);
-    EXPECT_EQ("\"result_a:alpha\"", ValueToString(*result_a));
+    EXPECT_EQ("\"result_a:alpha\"",
+              GetStringPropertyFromObject(context_a->Global(), context_a,
+                                          "eventArgs"));
   }
   {
-    v8::Local<v8::Value> result_b =
-        GetPropertyFromObject(context_b->Global(), context_b, "eventArgs");
-    ASSERT_FALSE(result_b.IsEmpty());
-    EXPECT_TRUE(result_b->IsUndefined());
+    EXPECT_EQ("undefined", GetStringPropertyFromObject(context_b->Global(),
+                                                       context_b, "eventArgs"));
   }
 
   // Dispatch the event in context_b - the listener in context_a should not be
@@ -347,16 +343,14 @@ TEST_F(APIEventHandlerTest, MultipleContexts) {
   ASSERT_TRUE(arguments_b);
   handler.FireEventInContext(kEventName, context_b, *arguments_b);
   {
-    std::unique_ptr<base::Value> result_a = GetBaseValuePropertyFromObject(
-        context_a->Global(), context_a, "eventArgs");
-    ASSERT_TRUE(result_a);
-    EXPECT_EQ("\"result_a:alpha\"", ValueToString(*result_a));
+    EXPECT_EQ("\"result_a:alpha\"",
+              GetStringPropertyFromObject(context_a->Global(), context_a,
+                                          "eventArgs"));
   }
   {
-    std::unique_ptr<base::Value> result_b = GetBaseValuePropertyFromObject(
-        context_b->Global(), context_b, "eventArgs");
-    ASSERT_TRUE(result_b);
-    EXPECT_EQ("\"result_b:beta\"", ValueToString(*result_b));
+    EXPECT_EQ("\"result_b:beta\"",
+              GetStringPropertyFromObject(context_b->Global(), context_b,
+                                          "eventArgs"));
   }
 }
 

extensions/renderer/api_request_handler_unittest.cc

@@ -78,10 +78,8 @@ TEST_F(APIRequestHandlerTest, AddRequestAndCompleteRequestTest) {
   request_handler.CompleteRequest(request_id, *response_arguments);
 
   EXPECT_TRUE(did_run_js());
-  std::unique_ptr<base::Value> result_value =
-      GetBaseValuePropertyFromObject(context->Global(), context, "result");
-  ASSERT_TRUE(result_value);
-  EXPECT_EQ(ReplaceSingleQuotes(kArguments), ValueToString(*result_value));
+  EXPECT_EQ(ReplaceSingleQuotes(kArguments),
+            GetStringPropertyFromObject(context->Global(), context, "result"));
 
   EXPECT_TRUE(request_handler.GetPendingRequestIdsForTesting().empty());
 }
@@ -154,11 +152,9 @@ TEST_F(APIRequestHandlerTest, MultipleRequestsAndContexts) {
   EXPECT_THAT(request_handler.GetPendingRequestIdsForTesting(),
               testing::UnorderedElementsAre(request_b));
 
-  std::unique_ptr<base::Value> result_a =
-      GetBaseValuePropertyFromObject(context_a->Global(), context_a, "result");
-  ASSERT_TRUE(result_a);
-  EXPECT_EQ(ReplaceSingleQuotes("'response_a:alpha'"),
-            ValueToString(*result_a));
+  EXPECT_EQ(
+      ReplaceSingleQuotes("'response_a:alpha'"),
+      GetStringPropertyFromObject(context_a->Global(), context_a, "result"));
 
   std::unique_ptr<base::ListValue> response_b =
       ListValueFromString("['response_b:']");
@@ -167,10 +163,9 @@ TEST_F(APIRequestHandlerTest, MultipleRequestsAndContexts) {
   request_handler.CompleteRequest(request_b, *response_b);
   EXPECT_TRUE(request_handler.GetPendingRequestIdsForTesting().empty());
 
-  std::unique_ptr<base::Value> result_b =
-      GetBaseValuePropertyFromObject(context_b->Global(), context_b, "result");
-  ASSERT_TRUE(result_b);
-  EXPECT_EQ(ReplaceSingleQuotes("'response_b:beta'"), ValueToString(*result_b));
+  EXPECT_EQ(
+      ReplaceSingleQuotes("'response_b:beta'"),
+      GetStringPropertyFromObject(context_b->Global(), context_b, "result"));
 }
 
 }  // namespace extensions

extensions/renderer/native_extension_bindings_system_unittest.cc

@@ -318,37 +318,29 @@ TEST_F(NativeExtensionBindingsSystemUnittest, TestBridgingToJSCustomBindings) {
     RunFunctionOnGlobal(call_idle_query_state, context, 0, nullptr);
   }
 
-  auto get_property_as_string = [&context](v8::Local<v8::Object> object,
-                                           base::StringPiece property_name) {
-    std::unique_ptr<base::Value> property =
-        GetBaseValuePropertyFromObject(object, context, property_name);
-    if (!property)
-      return std::string();
-    return ValueToString(*property);
-  };
-
   // To start, check that the properties we set when running the hooks are
   // correct. We do this after calling the function because the API objects (and
   // thus the hooks) are set up lazily.
   v8::Local<v8::Object> global = context->Global();
   EXPECT_EQ(base::StringPrintf("\"%s\"", extension->id().c_str()),
-            get_property_as_string(global, "hookedExtensionId"));
+            GetStringPropertyFromObject(global, context, "hookedExtensionId"));
   EXPECT_EQ("\"BLESSED_EXTENSION\"",
-            get_property_as_string(global, "hookedContextType"));
+            GetStringPropertyFromObject(global, context, "hookedContextType"));
   v8::Local<v8::Value> idle_api =
       V8ValueFromScriptSource(context, "chrome.idle");
   ASSERT_FALSE(idle_api.IsEmpty());
   ASSERT_TRUE(idle_api->IsObject());
   EXPECT_EQ("\"someProperty\"",
-            get_property_as_string(idle_api.As<v8::Object>(),
-                                   "hookedApiProperty"));
+            GetStringPropertyFromObject(idle_api.As<v8::Object>(), context,
+                                        "hookedApiProperty"));
 
   // Next, we need to check two pieces: first, that the custom handler was
   // called with the proper arguments....
-  EXPECT_EQ("30", get_property_as_string(global, "timeArg"));
+  EXPECT_EQ("30", GetStringPropertyFromObject(global, context, "timeArg"));
 
   // ...and second, that the callback was called with the proper result.
-  EXPECT_EQ("\"active\"", get_property_as_string(global, "responseState"));
+  EXPECT_EQ("\"active\"",
+            GetStringPropertyFromObject(global, context, "responseState"));
 }
 
 }  // namespace extensions