Commit 7ab1596c authored by rdevlin.cronin's avatar rdevlin.cronin Committed by Commit Bot

[Extensions Bindings] Add activity logging of custom handling

If a method is handled by a custom hook, we won't notify the browser
(where we normally would with an API request), and so the activity log
won't be updated. Add support for activity logging with native binding.

If a request is handled by a custom hook and a request is not sent to
the browser, notify the activity logger (which will send the information
along to the browser process).  This matches the logic we currently have
in the JS bindings.

Add unit tests for the same.  In addition to the unit tests, this fixes
ActivityLogApiTest.TriggerEvent with native bindings enabled.

BUG=653596

Review-Url: https://codereview.chromium.org/2962093002
Cr-Commit-Position: refs/heads/master@{#485457}
parent 7e469cba
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chrome/renderer/extensions/app_hooks_delegate.h" #include "chrome/renderer/extensions/app_hooks_delegate.h"
#include "extensions/renderer/api_activity_logger.h"
#include "extensions/renderer/bindings/api_request_handler.h" #include "extensions/renderer/bindings/api_request_handler.h"
#include "extensions/renderer/bindings/api_signature.h" #include "extensions/renderer/bindings/api_signature.h"
#include "extensions/renderer/script_context_set.h" #include "extensions/renderer/script_context_set.h"
...@@ -17,11 +18,15 @@ void IsInstalledGetterCallback( ...@@ -17,11 +18,15 @@ void IsInstalledGetterCallback(
v8::Local<v8::String> property, v8::Local<v8::String> property,
const v8::PropertyCallbackInfo<v8::Value>& info) { const v8::PropertyCallbackInfo<v8::Value>& info) {
v8::HandleScope handle_scope(info.GetIsolate()); v8::HandleScope handle_scope(info.GetIsolate());
v8::Local<v8::Context> context = info.Holder()->CreationContext();
ScriptContext* script_context = ScriptContext* script_context =
ScriptContextSet::GetContextByV8Context(info.Holder()->CreationContext()); ScriptContextSet::GetContextByV8Context(context);
DCHECK(script_context); DCHECK(script_context);
auto* core = auto* core =
static_cast<AppBindingsCore*>(info.Data().As<v8::External>()->Value()); static_cast<AppBindingsCore*>(info.Data().As<v8::External>()->Value());
// Since this is more-or-less an API, log it as an API call.
APIActivityLogger::LogAPICall(context, "app.getIsInstalled",
std::vector<v8::Local<v8::Value>>());
info.GetReturnValue().Set(core->GetIsInstalled(script_context)); info.GetReturnValue().Set(core->GetIsInstalled(script_context));
} }
......
...@@ -15,9 +15,7 @@ import os ...@@ -15,9 +15,7 @@ import os
def GetPathsToPrepend(input_api): def GetPathsToPrepend(input_api):
web_dev_style_path = input_api.os_path.join( web_dev_style_path = input_api.os_path.join(
input_api.change.RepositoryRoot(), input_api.change.RepositoryRoot(),
'chrome', 'tools')
'browser',
'resources')
return [input_api.PresubmitLocalPath(), web_dev_style_path] return [input_api.PresubmitLocalPath(), web_dev_style_path]
def RunWithPrependedPath(prepended_path, fn, *args): def RunWithPrependedPath(prepended_path, fn, *args):
......
...@@ -39,9 +39,11 @@ testCases.push({ ...@@ -39,9 +39,11 @@ testCases.push({
'app_bindings', function response() { }); 'app_bindings', function response() { });
}, },
expected_activity: [ expected_activity: [
'app.GetDetails', // These API calls show up differently depending on whether native bindings
'app.GetIsInstalled', // are enabled.
'app.getInstallState' /app\.[gG]etDetails/,
/app\.[gG]etIsInstalled/,
/app\.(getI|i)nstallState/,
] ]
}); });
testCases.push({ testCases.push({
...@@ -482,7 +484,11 @@ chrome.activityLogPrivate.onExtensionActivity.addListener( ...@@ -482,7 +484,11 @@ chrome.activityLogPrivate.onExtensionActivity.addListener(
expectedCall = testCase.expected_activity[callIndx]; expectedCall = testCase.expected_activity[callIndx];
} }
console.log('Logged:' + apiCall + ' Expected:' + expectedCall); console.log('Logged:' + apiCall + ' Expected:' + expectedCall);
chrome.test.assertEq(expectedCall, apiCall); // Allow either a RegExp or a strict string comparison.
if (expectedCall instanceof RegExp)
chrome.test.assertTrue(expectedCall.test(apiCall));
else
chrome.test.assertEq(expectedCall, apiCall);
// Check that no real URLs are logged in incognito-mode tests. Ignore // Check that no real URLs are logged in incognito-mode tests. Ignore
// the initial call to windows.create opening the tab. // the initial call to windows.create opening the tab.
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "extensions/renderer/api_activity_logger.h"
#include <stddef.h> #include <stddef.h>
#include <string> #include <string>
...@@ -11,8 +13,8 @@ ...@@ -11,8 +13,8 @@
#include "content/public/renderer/render_thread.h" #include "content/public/renderer/render_thread.h"
#include "extensions/common/extension_messages.h" #include "extensions/common/extension_messages.h"
#include "extensions/renderer/activity_log_converter_strategy.h" #include "extensions/renderer/activity_log_converter_strategy.h"
#include "extensions/renderer/api_activity_logger.h"
#include "extensions/renderer/dispatcher.h" #include "extensions/renderer/dispatcher.h"
#include "extensions/renderer/extensions_renderer_client.h"
#include "extensions/renderer/script_context.h" #include "extensions/renderer/script_context.h"
namespace extensions { namespace extensions {
...@@ -20,28 +22,45 @@ namespace extensions { ...@@ -20,28 +22,45 @@ namespace extensions {
APIActivityLogger::APIActivityLogger(ScriptContext* context, APIActivityLogger::APIActivityLogger(ScriptContext* context,
Dispatcher* dispatcher) Dispatcher* dispatcher)
: ObjectBackedNativeHandler(context), dispatcher_(dispatcher) { : ObjectBackedNativeHandler(context), dispatcher_(dispatcher) {
RouteFunction("LogEvent", base::Bind(&APIActivityLogger::LogEvent, RouteFunction("LogEvent", base::Bind(&APIActivityLogger::LogForJS,
base::Unretained(this))); base::Unretained(this), EVENT));
RouteFunction("LogAPICall", base::Bind(&APIActivityLogger::LogAPICall, RouteFunction("LogAPICall", base::Bind(&APIActivityLogger::LogForJS,
base::Unretained(this))); base::Unretained(this), APICALL));
} }
APIActivityLogger::~APIActivityLogger() {} APIActivityLogger::~APIActivityLogger() {}
// static // static
void APIActivityLogger::LogAPICall( void APIActivityLogger::LogAPICall(
const v8::FunctionCallbackInfo<v8::Value>& args) { v8::Local<v8::Context> context,
LogInternal(APICALL, args); const std::string& call_name,
} const std::vector<v8::Local<v8::Value>>& arguments) {
const Dispatcher* dispatcher =
ExtensionsRendererClient::Get()->GetDispatcher();
if (!dispatcher || // dispatcher can be null in unittests.
!dispatcher->activity_logging_enabled()) {
return;
}
// static ScriptContext* script_context =
void APIActivityLogger::LogEvent( ScriptContextSet::GetContextByV8Context(context);
const v8::FunctionCallbackInfo<v8::Value>& args) { auto value_args = base::MakeUnique<base::ListValue>();
LogInternal(EVENT, args); std::unique_ptr<content::V8ValueConverter> converter =
content::V8ValueConverter::Create();
ActivityLogConverterStrategy strategy;
converter->SetFunctionAllowed(true);
converter->SetStrategy(&strategy);
value_args->Reserve(arguments.size());
// TODO(devlin): This doesn't protect against custom properties, so it might
// not perfectly reflect the passed arguments.
for (const auto& arg : arguments)
value_args->Append(converter->FromV8Value(arg, context));
LogInternal(APICALL, script_context->GetExtensionID(), call_name,
std::move(value_args), std::string());
} }
// static void APIActivityLogger::LogForJS(
void APIActivityLogger::LogInternal(
const CallType call_type, const CallType call_type,
const v8::FunctionCallbackInfo<v8::Value>& args) { const v8::FunctionCallbackInfo<v8::Value>& args) {
CHECK_GT(args.Length(), 2); CHECK_GT(args.Length(), 2);
...@@ -52,38 +71,51 @@ void APIActivityLogger::LogInternal( ...@@ -52,38 +71,51 @@ void APIActivityLogger::LogInternal(
if (!dispatcher_->activity_logging_enabled()) if (!dispatcher_->activity_logging_enabled())
return; return;
std::string ext_id = *v8::String::Utf8Value(args[0]); v8::Isolate* isolate = args.GetIsolate();
ExtensionHostMsg_APIActionOrEvent_Params params; v8::HandleScope handle_scope(isolate);
params.api_call = *v8::String::Utf8Value(args[1]); v8::Local<v8::Context> context = isolate->GetCurrentContext();
if (args.Length() == 4) // Extras are optional.
params.extra = *v8::String::Utf8Value(args[3]); std::string extension_id = *v8::String::Utf8Value(args[0]);
else std::string call_name = *v8::String::Utf8Value(args[1]);
params.extra = ""; std::string extra;
if (args.Length() == 4) { // Extras are optional.
CHECK(args[3]->IsString());
extra = *v8::String::Utf8Value(args[3]);
}
// Get the array of api call arguments. // Get the array of call arguments.
auto arguments = base::MakeUnique<base::ListValue>();
v8::Local<v8::Array> arg_array = v8::Local<v8::Array>::Cast(args[2]); v8::Local<v8::Array> arg_array = v8::Local<v8::Array>::Cast(args[2]);
if (arg_array->Length() > 0) { if (arg_array->Length() > 0) {
arguments->Reserve(arg_array->Length());
std::unique_ptr<content::V8ValueConverter> converter = std::unique_ptr<content::V8ValueConverter> converter =
content::V8ValueConverter::Create(); content::V8ValueConverter::Create();
ActivityLogConverterStrategy strategy; ActivityLogConverterStrategy strategy;
converter->SetFunctionAllowed(true); converter->SetFunctionAllowed(true);
converter->SetStrategy(&strategy); converter->SetStrategy(&strategy);
std::unique_ptr<base::ListValue> arg_list(new base::ListValue()); for (size_t i = 0; i < arg_array->Length(); ++i)
for (size_t i = 0; i < arg_array->Length(); ++i) { arguments->Append(converter->FromV8Value(arg_array->Get(i), context));
arg_list->Set(
i,
converter->FromV8Value(arg_array->Get(i),
args.GetIsolate()->GetCurrentContext()));
}
params.arguments.Swap(arg_list.get());
} }
LogInternal(call_type, extension_id, call_name, std::move(arguments), extra);
}
// static
void APIActivityLogger::LogInternal(const CallType call_type,
const std::string& extension_id,
const std::string& call_name,
std::unique_ptr<base::ListValue> arguments,
const std::string& extra) {
ExtensionHostMsg_APIActionOrEvent_Params params;
params.api_call = call_name;
params.arguments.Swap(arguments.get());
params.extra = extra;
if (call_type == APICALL) { if (call_type == APICALL) {
content::RenderThread::Get()->Send( content::RenderThread::Get()->Send(
new ExtensionHostMsg_AddAPIActionToActivityLog(ext_id, params)); new ExtensionHostMsg_AddAPIActionToActivityLog(extension_id, params));
} else if (call_type == EVENT) { } else if (call_type == EVENT) {
content::RenderThread::Get()->Send( content::RenderThread::Get()->Send(
new ExtensionHostMsg_AddEventToActivityLog(ext_id, params)); new ExtensionHostMsg_AddEventToActivityLog(extension_id, params));
} }
} }
......
...@@ -5,13 +5,18 @@ ...@@ -5,13 +5,18 @@
#ifndef EXTENSIONS_RENDERER_API_ACTIVITY_LOGGER_H_ #ifndef EXTENSIONS_RENDERER_API_ACTIVITY_LOGGER_H_
#define EXTENSIONS_RENDERER_API_ACTIVITY_LOGGER_H_ #define EXTENSIONS_RENDERER_API_ACTIVITY_LOGGER_H_
#include <memory>
#include <string> #include <string>
#include <vector>
#include "base/macros.h" #include "base/macros.h"
#include "extensions/common/features/feature.h"
#include "extensions/renderer/object_backed_native_handler.h" #include "extensions/renderer/object_backed_native_handler.h"
#include "v8/include/v8.h" #include "v8/include/v8.h"
namespace base {
class ListValue;
}
namespace extensions { namespace extensions {
class Dispatcher; class Dispatcher;
...@@ -23,28 +28,31 @@ class APIActivityLogger : public ObjectBackedNativeHandler { ...@@ -23,28 +28,31 @@ class APIActivityLogger : public ObjectBackedNativeHandler {
APIActivityLogger(ScriptContext* context, Dispatcher* dispatcher); APIActivityLogger(ScriptContext* context, Dispatcher* dispatcher);
~APIActivityLogger() override; ~APIActivityLogger() override;
// Notifies the browser that an API method has been called, if and only if
// activity logging is enabled.
static void LogAPICall(v8::Local<v8::Context> context,
const std::string& call_name,
const std::vector<v8::Local<v8::Value>>& arguments);
private: private:
// Used to distinguish API calls & events from each other in LogInternal. // Used to distinguish API calls & events from each other in LogInternal.
enum CallType { APICALL, EVENT }; enum CallType { APICALL, EVENT };
// This is ultimately invoked in bindings.js with JavaScript arguments. // This is ultimately invoked in bindings.js with JavaScript arguments.
// arg0 - extension ID as a string // arg0 - extension ID as a string
// arg1 - API call name as a string // arg1 - API method/Event name as a string
// arg2 - arguments to the API call // arg2 - arguments to the API call/event
// arg3 - any extra logging info as a string (optional)
void LogAPICall(const v8::FunctionCallbackInfo<v8::Value>& args);
// This is ultimately invoked in bindings.js with JavaScript arguments.
// arg0 - extension ID as a string
// arg1 - Event name as a string
// arg2 - Event arguments
// arg3 - any extra logging info as a string (optional) // arg3 - any extra logging info as a string (optional)
void LogEvent(const v8::FunctionCallbackInfo<v8::Value>& args); // TODO(devlin): Does arg3 ever exist?
void LogForJS(const CallType call_type,
const v8::FunctionCallbackInfo<v8::Value>& args);
// LogAPICall and LogEvent are really the same underneath except for // Common implementation method for sending a logging IPC.
// how they are ultimately dispatched to the log. static void LogInternal(const CallType call_type,
void LogInternal(const CallType call_type, const std::string& extension_id,
const v8::FunctionCallbackInfo<v8::Value>& args); const std::string& call_name,
std::unique_ptr<base::ListValue> arguments,
const std::string& extra);
Dispatcher* dispatcher_; Dispatcher* dispatcher_;
......
...@@ -179,6 +179,7 @@ APIBinding::APIBinding(const std::string& api_name, ...@@ -179,6 +179,7 @@ APIBinding::APIBinding(const std::string& api_name,
const base::ListValue* event_definitions, const base::ListValue* event_definitions,
const base::DictionaryValue* property_definitions, const base::DictionaryValue* property_definitions,
const CreateCustomType& create_custom_type, const CreateCustomType& create_custom_type,
const OnSilentRequest& on_silent_request,
std::unique_ptr<APIBindingHooks> binding_hooks, std::unique_ptr<APIBindingHooks> binding_hooks,
APITypeReferenceMap* type_refs, APITypeReferenceMap* type_refs,
APIRequestHandler* request_handler, APIRequestHandler* request_handler,
...@@ -187,6 +188,7 @@ APIBinding::APIBinding(const std::string& api_name, ...@@ -187,6 +188,7 @@ APIBinding::APIBinding(const std::string& api_name,
: api_name_(api_name), : api_name_(api_name),
property_definitions_(property_definitions), property_definitions_(property_definitions),
create_custom_type_(create_custom_type), create_custom_type_(create_custom_type),
on_silent_request_(on_silent_request),
binding_hooks_(std::move(binding_hooks)), binding_hooks_(std::move(binding_hooks)),
type_refs_(type_refs), type_refs_(type_refs),
request_handler_(request_handler), request_handler_(request_handler),
...@@ -570,6 +572,7 @@ void APIBinding::HandleCall(const std::string& name, ...@@ -570,6 +572,7 @@ void APIBinding::HandleCall(const std::string& name,
bool invalid_invocation = false; bool invalid_invocation = false;
v8::Local<v8::Function> custom_callback; v8::Local<v8::Function> custom_callback;
bool updated_args = false; bool updated_args = false;
int old_request_id = request_handler_->last_sent_request_id();
{ {
v8::TryCatch try_catch(isolate); v8::TryCatch try_catch(isolate);
APIBindingHooks::RequestResult hooks_result = binding_hooks_->RunHooks( APIBindingHooks::RequestResult hooks_result = binding_hooks_->RunHooks(
...@@ -588,6 +591,14 @@ void APIBinding::HandleCall(const std::string& name, ...@@ -588,6 +591,14 @@ void APIBinding::HandleCall(const std::string& name,
case APIBindingHooks::RequestResult::HANDLED: case APIBindingHooks::RequestResult::HANDLED:
if (!hooks_result.return_value.IsEmpty()) if (!hooks_result.return_value.IsEmpty())
arguments->Return(hooks_result.return_value); arguments->Return(hooks_result.return_value);
// TODO(devlin): This is a pretty simplistic implementation of this,
// but it's similar to the current JS logic. If we wanted to be more
// correct, we could create a RequestScope object that watches outgoing
// requests.
if (old_request_id == request_handler_->last_sent_request_id())
on_silent_request_.Run(context, name, argument_list);
return; // Our work here is done. return; // Our work here is done.
case APIBindingHooks::RequestResult::ARGUMENTS_UPDATED: case APIBindingHooks::RequestResult::ARGUMENTS_UPDATED:
updated_args = true; // Intentional fall-through. updated_args = true; // Intentional fall-through.
......
...@@ -52,6 +52,12 @@ class APIBinding { ...@@ -52,6 +52,12 @@ class APIBinding {
const std::string& property_name, const std::string& property_name,
const base::ListValue* property_values)>; const base::ListValue* property_values)>;
// Called when a request is handled without notifying the browser.
using OnSilentRequest =
base::Callback<void(v8::Local<v8::Context>,
const std::string& name,
const std::vector<v8::Local<v8::Value>>& arguments)>;
// The callback type for handling an API call. // The callback type for handling an API call.
using HandlerCallback = base::Callback<void(gin::Arguments*)>; using HandlerCallback = base::Callback<void(gin::Arguments*)>;
...@@ -64,6 +70,7 @@ class APIBinding { ...@@ -64,6 +70,7 @@ class APIBinding {
const base::ListValue* event_definitions, const base::ListValue* event_definitions,
const base::DictionaryValue* property_definitions, const base::DictionaryValue* property_definitions,
const CreateCustomType& create_custom_type, const CreateCustomType& create_custom_type,
const OnSilentRequest& on_silent_request,
std::unique_ptr<APIBindingHooks> binding_hooks, std::unique_ptr<APIBindingHooks> binding_hooks,
APITypeReferenceMap* type_refs, APITypeReferenceMap* type_refs,
APIRequestHandler* request_handler, APIRequestHandler* request_handler,
...@@ -131,6 +138,8 @@ class APIBinding { ...@@ -131,6 +138,8 @@ class APIBinding {
// The callback for constructing a custom type. // The callback for constructing a custom type.
CreateCustomType create_custom_type_; CreateCustomType create_custom_type_;
OnSilentRequest on_silent_request_;
// The registered hooks for this API. // The registered hooks for this API.
std::unique_ptr<APIBindingHooks> binding_hooks_; std::unique_ptr<APIBindingHooks> binding_hooks_;
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "gin/arguments.h" #include "gin/arguments.h"
#include "gin/converter.h" #include "gin/converter.h"
#include "gin/public/context_holder.h" #include "gin/public/context_holder.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/WebKit/public/web/WebScopedUserGesture.h" #include "third_party/WebKit/public/web/WebScopedUserGesture.h"
#include "v8/include/v8.h" #include "v8/include/v8.h"
...@@ -80,6 +81,11 @@ void OnEventListenersChanged(const std::string& event_name, ...@@ -80,6 +81,11 @@ void OnEventListenersChanged(const std::string& event_name,
bool was_manual, bool was_manual,
v8::Local<v8::Context> context) {} v8::Local<v8::Context> context) {}
void DoNothingWithSilentRequest(
v8::Local<v8::Context> context,
const std::string& call_name,
const std::vector<v8::Local<v8::Value>>& arguments) {}
} // namespace } // namespace
class APIBindingUnittest : public APIBindingTest { class APIBindingUnittest : public APIBindingTest {
...@@ -150,6 +156,10 @@ class APIBindingUnittest : public APIBindingTest { ...@@ -150,6 +156,10 @@ class APIBindingUnittest : public APIBindingTest {
create_custom_type_ = callback; create_custom_type_ = callback;
} }
void SetOnSilentRequest(const APIBinding::OnSilentRequest& callback) {
on_silent_request_ = callback;
}
void SetAvailabilityCallback( void SetAvailabilityCallback(
const BindingAccessChecker::AvailabilityCallback& callback) { const BindingAccessChecker::AvailabilityCallback& callback) {
availability_callback_ = callback; availability_callback_ = callback;
...@@ -162,6 +172,8 @@ class APIBindingUnittest : public APIBindingTest { ...@@ -162,6 +172,8 @@ class APIBindingUnittest : public APIBindingTest {
} }
if (binding_hooks_delegate_) if (binding_hooks_delegate_)
binding_hooks_->SetDelegate(std::move(binding_hooks_delegate_)); binding_hooks_->SetDelegate(std::move(binding_hooks_delegate_));
if (!on_silent_request_)
on_silent_request_ = base::Bind(&DoNothingWithSilentRequest);
if (!availability_callback_) if (!availability_callback_)
availability_callback_ = base::Bind(&AllowAllFeatures); availability_callback_ = base::Bind(&AllowAllFeatures);
event_handler_ = base::MakeUnique<APIEventHandler>( event_handler_ = base::MakeUnique<APIEventHandler>(
...@@ -173,8 +185,8 @@ class APIBindingUnittest : public APIBindingTest { ...@@ -173,8 +185,8 @@ class APIBindingUnittest : public APIBindingTest {
binding_ = base::MakeUnique<APIBinding>( binding_ = base::MakeUnique<APIBinding>(
kBindingName, binding_functions_.get(), binding_types_.get(), kBindingName, binding_functions_.get(), binding_types_.get(),
binding_events_.get(), binding_properties_.get(), create_custom_type_, binding_events_.get(), binding_properties_.get(), create_custom_type_,
std::move(binding_hooks_), &type_refs_, request_handler_.get(), on_silent_request_, std::move(binding_hooks_), &type_refs_,
event_handler_.get(), access_checker_.get()); request_handler_.get(), event_handler_.get(), access_checker_.get());
EXPECT_EQ(!binding_types_.get(), type_refs_.empty()); EXPECT_EQ(!binding_types_.get(), type_refs_.empty());
} }
...@@ -243,6 +255,7 @@ class APIBindingUnittest : public APIBindingTest { ...@@ -243,6 +255,7 @@ class APIBindingUnittest : public APIBindingTest {
std::unique_ptr<APIBindingHooks> binding_hooks_; std::unique_ptr<APIBindingHooks> binding_hooks_;
std::unique_ptr<APIBindingHooksDelegate> binding_hooks_delegate_; std::unique_ptr<APIBindingHooksDelegate> binding_hooks_delegate_;
APIBinding::CreateCustomType create_custom_type_; APIBinding::CreateCustomType create_custom_type_;
APIBinding::OnSilentRequest on_silent_request_;
BindingAccessChecker::AvailabilityCallback availability_callback_; BindingAccessChecker::AvailabilityCallback availability_callback_;
DISALLOW_COPY_AND_ASSIGN(APIBindingUnittest); DISALLOW_COPY_AND_ASSIGN(APIBindingUnittest);
...@@ -1301,4 +1314,188 @@ TEST_F(APIBindingUnittest, HooksTemplateInitializer) { ...@@ -1301,4 +1314,188 @@ TEST_F(APIBindingUnittest, HooksTemplateInitializer) {
GetStringPropertyFromObject(binding_object, context, "oneString")); GetStringPropertyFromObject(binding_object, context, "oneString"));
} }
// Test that running hooks returning different results correctly sends requests
// or notifies of silent requests.
TEST_F(APIBindingUnittest, TestSendingRequestsAndSilentRequestsWithHooks) {
SetFunctions(
"[{"
" 'name': 'modifyArgs',"
" 'parameters': []"
"}, {"
" 'name': 'invalidInvocation',"
" 'parameters': []"
"}, {"
" 'name': 'throwException',"
" 'parameters': []"
"}, {"
" 'name': 'dontHandle',"
" 'parameters': []"
"}, {"
" 'name': 'handle',"
" 'parameters': []"
"}, {"
" 'name': 'handleAndSendRequest',"
" 'parameters': []"
"}, {"
" 'name': 'handleWithArgs',"
" 'parameters': [{"
" 'name': 'first',"
" 'type': 'string'"
" }, {"
" 'name': 'second',"
" 'type': 'integer'"
" }]"
"}]");
using RequestResult = APIBindingHooks::RequestResult;
auto basic_handler = [](RequestResult::ResultCode code, const APISignature*,
v8::Local<v8::Context> context,
std::vector<v8::Local<v8::Value>>* arguments,
const APITypeReferenceMap& map) {
return RequestResult(code);
};
auto hooks = base::MakeUnique<APIBindingHooksTestDelegate>();
hooks->AddHandler(
"test.modifyArgs",
base::Bind(basic_handler, RequestResult::ARGUMENTS_UPDATED));
hooks->AddHandler(
"test.invalidInvocation",
base::Bind(basic_handler, RequestResult::INVALID_INVOCATION));
hooks->AddHandler("test.dontHandle",
base::Bind(basic_handler, RequestResult::NOT_HANDLED));
hooks->AddHandler("test.handle",
base::Bind(basic_handler, RequestResult::HANDLED));
hooks->AddHandler(
"test.throwException",
base::Bind([](const APISignature*, v8::Local<v8::Context> context,
std::vector<v8::Local<v8::Value>>* arguments,
const APITypeReferenceMap& map) {
context->GetIsolate()->ThrowException(
gin::StringToV8(context->GetIsolate(), "some error"));
return RequestResult(RequestResult::THROWN);
}));
hooks->AddHandler(
"test.handleWithArgs",
base::Bind([](const APISignature*, v8::Local<v8::Context> context,
std::vector<v8::Local<v8::Value>>* arguments,
const APITypeReferenceMap& map) {
arguments->push_back(v8::Integer::New(context->GetIsolate(), 42));
return RequestResult(RequestResult::HANDLED);
}));
auto handle_and_send_request =
[](APIRequestHandler* handler, const APISignature*,
v8::Local<v8::Context> context,
std::vector<v8::Local<v8::Value>>* arguments,
const APITypeReferenceMap& map) {
handler->StartRequest(
context, "test.handleAndSendRequest",
base::MakeUnique<base::ListValue>(), v8::Local<v8::Function>(),
v8::Local<v8::Function>(), binding::RequestThread::UI);
return RequestResult(RequestResult::HANDLED);
};
hooks->AddHandler("test.handleAndSendRequest",
base::Bind(handle_and_send_request, request_handler()));
SetHooksDelegate(std::move(hooks));
auto on_silent_request =
[](base::Optional<std::string>* name_out,
base::Optional<std::vector<std::string>>* args_out,
v8::Local<v8::Context> context, const std::string& call_name,
const std::vector<v8::Local<v8::Value>>& arguments) {
*name_out = call_name;
*args_out = std::vector<std::string>();
(*args_out)->reserve(arguments.size());
for (const auto& arg : arguments)
(*args_out)->push_back(V8ToString(arg, context));
};
base::Optional<std::string> silent_request;
base::Optional<std::vector<std::string>> request_arguments;
SetOnSilentRequest(
base::Bind(on_silent_request, &silent_request, &request_arguments));
InitializeBinding();
v8::HandleScope handle_scope(isolate());
v8::Local<v8::Context> context = MainContext();
v8::Local<v8::Object> binding_object = binding()->CreateInstance(context);
auto call_api_method = [binding_object, context](
base::StringPiece name,
base::StringPiece string_args) {
v8::Local<v8::Function> call = FunctionFromString(
context, base::StringPrintf("(function(binding) { binding.%s(%s); })",
name.data(), string_args.data()));
v8::Local<v8::Value> args[] = {binding_object};
v8::TryCatch try_catch(context->GetIsolate());
// The throwException call will throw an exception; ignore it.
ignore_result(call->Call(context, v8::Undefined(context->GetIsolate()),
arraysize(args), args));
};
call_api_method("modifyArgs", "");
ASSERT_TRUE(last_request());
EXPECT_EQ("test.modifyArgs", last_request()->method_name);
EXPECT_FALSE(silent_request);
reset_last_request();
silent_request.reset();
request_arguments.reset();
call_api_method("invalidInvocation", "");
EXPECT_FALSE(last_request());
EXPECT_FALSE(silent_request);
reset_last_request();
silent_request.reset();
request_arguments.reset();
call_api_method("throwException", "");
EXPECT_FALSE(last_request());
EXPECT_FALSE(silent_request);
reset_last_request();
silent_request.reset();
request_arguments.reset();
call_api_method("dontHandle", "");
ASSERT_TRUE(last_request());
EXPECT_EQ("test.dontHandle", last_request()->method_name);
EXPECT_FALSE(silent_request);
reset_last_request();
silent_request.reset();
request_arguments.reset();
call_api_method("handle", "");
EXPECT_FALSE(last_request());
ASSERT_TRUE(silent_request);
EXPECT_EQ("test.handle", *silent_request);
ASSERT_TRUE(request_arguments);
EXPECT_TRUE(request_arguments->empty());
reset_last_request();
silent_request.reset();
request_arguments.reset();
call_api_method("handleAndSendRequest", "");
ASSERT_TRUE(last_request());
EXPECT_EQ("test.handleAndSendRequest", last_request()->method_name);
EXPECT_FALSE(silent_request);
reset_last_request();
silent_request.reset();
request_arguments.reset();
call_api_method("handleWithArgs", "'str'");
EXPECT_FALSE(last_request());
ASSERT_TRUE(silent_request);
ASSERT_EQ("test.handleWithArgs", *silent_request);
ASSERT_TRUE(request_arguments);
EXPECT_THAT(
*request_arguments,
testing::ElementsAre("\"str\"", "42")); // 42 was added by the handler.
reset_last_request();
silent_request.reset();
request_arguments.reset();
}
} // namespace extensions } // namespace extensions
...@@ -18,6 +18,7 @@ APIBindingsSystem::APIBindingsSystem( ...@@ -18,6 +18,7 @@ APIBindingsSystem::APIBindingsSystem(
const BindingAccessChecker::AvailabilityCallback& is_available, const BindingAccessChecker::AvailabilityCallback& is_available,
const APIRequestHandler::SendRequestMethod& send_request, const APIRequestHandler::SendRequestMethod& send_request,
const APIEventHandler::EventListenersChangedMethod& event_listeners_changed, const APIEventHandler::EventListenersChangedMethod& event_listeners_changed,
const APIBinding::OnSilentRequest& on_silent_request,
APILastError last_error) APILastError last_error)
: type_reference_map_(base::Bind(&APIBindingsSystem::InitializeType, : type_reference_map_(base::Bind(&APIBindingsSystem::InitializeType,
base::Unretained(this))), base::Unretained(this))),
...@@ -26,7 +27,8 @@ APIBindingsSystem::APIBindingsSystem( ...@@ -26,7 +27,8 @@ APIBindingsSystem::APIBindingsSystem(
access_checker_(is_available), access_checker_(is_available),
call_js_(call_js), call_js_(call_js),
call_js_sync_(call_js_sync), call_js_sync_(call_js_sync),
get_api_schema_(get_api_schema) {} get_api_schema_(get_api_schema),
on_silent_request_(on_silent_request) {}
APIBindingsSystem::~APIBindingsSystem() {} APIBindingsSystem::~APIBindingsSystem() {}
...@@ -72,8 +74,8 @@ std::unique_ptr<APIBinding> APIBindingsSystem::CreateNewAPIBinding( ...@@ -72,8 +74,8 @@ std::unique_ptr<APIBinding> APIBindingsSystem::CreateNewAPIBinding(
api_name, function_definitions, type_definitions, event_definitions, api_name, function_definitions, type_definitions, event_definitions,
property_definitions, property_definitions,
base::Bind(&APIBindingsSystem::CreateCustomType, base::Unretained(this)), base::Bind(&APIBindingsSystem::CreateCustomType, base::Unretained(this)),
std::move(hooks), &type_reference_map_, &request_handler_, on_silent_request_, std::move(hooks), &type_reference_map_,
&event_handler_, &access_checker_); &request_handler_, &event_handler_, &access_checker_);
} }
void APIBindingsSystem::InitializeType(const std::string& type_name) { void APIBindingsSystem::InitializeType(const std::string& type_name) {
......
...@@ -51,6 +51,7 @@ class APIBindingsSystem { ...@@ -51,6 +51,7 @@ class APIBindingsSystem {
const APIRequestHandler::SendRequestMethod& send_request, const APIRequestHandler::SendRequestMethod& send_request,
const APIEventHandler::EventListenersChangedMethod& const APIEventHandler::EventListenersChangedMethod&
event_listeners_changed, event_listeners_changed,
const APIBinding::OnSilentRequest& on_silent_request,
APILastError last_error); APILastError last_error);
~APIBindingsSystem(); ~APIBindingsSystem();
...@@ -141,6 +142,10 @@ class APIBindingsSystem { ...@@ -141,6 +142,10 @@ class APIBindingsSystem {
// API. Curried in for testing purposes so we can use fake APIs. // API. Curried in for testing purposes so we can use fake APIs.
GetAPISchemaMethod get_api_schema_; GetAPISchemaMethod get_api_schema_;
// The method to call when the system silently handles an API request without
// notifying the browser.
APIBinding::OnSilentRequest on_silent_request_;
DISALLOW_COPY_AND_ASSIGN(APIBindingsSystem); DISALLOW_COPY_AND_ASSIGN(APIBindingsSystem);
}; };
......
...@@ -94,6 +94,11 @@ bool AllowAllAPIs(v8::Local<v8::Context> context, const std::string& name) { ...@@ -94,6 +94,11 @@ bool AllowAllAPIs(v8::Local<v8::Context> context, const std::string& name) {
return true; return true;
} }
void DoNothingWithSilentRequest(
v8::Local<v8::Context> context,
const std::string& call_name,
const std::vector<v8::Local<v8::Value>>& arguments) {}
} // namespace } // namespace
APIBindingsSystemTest::APIBindingsSystemTest() {} APIBindingsSystemTest::APIBindingsSystemTest() {}
...@@ -118,6 +123,7 @@ void APIBindingsSystemTest::SetUp() { ...@@ -118,6 +123,7 @@ void APIBindingsSystemTest::SetUp() {
base::Bind(&APIBindingsSystemTest::OnAPIRequest, base::Unretained(this)), base::Bind(&APIBindingsSystemTest::OnAPIRequest, base::Unretained(this)),
base::Bind(&APIBindingsSystemTest::OnEventListenersChanged, base::Bind(&APIBindingsSystemTest::OnEventListenersChanged,
base::Unretained(this)), base::Unretained(this)),
base::Bind(&DoNothingWithSilentRequest),
APILastError(base::Bind(&APIBindingsSystemTest::GetLastErrorParent, APILastError(base::Bind(&APIBindingsSystemTest::GetLastErrorParent,
base::Unretained(this)), base::Unretained(this)),
APILastError::AddConsoleError())); APILastError::AddConsoleError()));
......
...@@ -106,6 +106,7 @@ int APIRequestHandler::StartRequest(v8::Local<v8::Context> context, ...@@ -106,6 +106,7 @@ int APIRequestHandler::StartRequest(v8::Local<v8::Context> context,
request->method_name = method; request->method_name = method;
request->thread = thread; request->thread = thread;
last_sent_request_id_ = request_id;
send_request_.Run(std::move(request), context); send_request_.Run(std::move(request), context);
return request_id; return request_id;
} }
......
...@@ -85,6 +85,7 @@ class APIRequestHandler { ...@@ -85,6 +85,7 @@ class APIRequestHandler {
void InvalidateContext(v8::Local<v8::Context> context); void InvalidateContext(v8::Local<v8::Context> context);
APILastError* last_error() { return &last_error_; } APILastError* last_error() { return &last_error_; }
int last_sent_request_id() const { return last_sent_request_id_; }
std::set<int> GetPendingRequestIdsForTesting() const; std::set<int> GetPendingRequestIdsForTesting() const;
...@@ -108,6 +109,11 @@ class APIRequestHandler { ...@@ -108,6 +109,11 @@ class APIRequestHandler {
// The next available request identifier. // The next available request identifier.
int next_request_id_ = 0; int next_request_id_ = 0;
// The id of the last request we sent to the browser. This can be used as a
// flag for whether or not a request was sent (if the last_sent_request_id_
// changes).
int last_sent_request_id_ = -1;
// A map of all pending requests. // A map of all pending requests.
std::map<int, PendingRequest> pending_requests_; std::map<int, PendingRequest> pending_requests_;
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "extensions/common/extension_api.h" #include "extensions/common/extension_api.h"
#include "extensions/common/extension_messages.h" #include "extensions/common/extension_messages.h"
#include "extensions/common/features/feature_provider.h" #include "extensions/common/features/feature_provider.h"
#include "extensions/renderer/api_activity_logger.h"
#include "extensions/renderer/bindings/api_binding_bridge.h" #include "extensions/renderer/bindings/api_binding_bridge.h"
#include "extensions/renderer/bindings/api_binding_hooks.h" #include "extensions/renderer/bindings/api_binding_hooks.h"
#include "extensions/renderer/bindings/api_binding_js_util.h" #include "extensions/renderer/bindings/api_binding_js_util.h"
...@@ -361,6 +362,7 @@ NativeExtensionBindingsSystem::NativeExtensionBindingsSystem( ...@@ -361,6 +362,7 @@ NativeExtensionBindingsSystem::NativeExtensionBindingsSystem(
base::Unretained(this)), base::Unretained(this)),
base::Bind(&NativeExtensionBindingsSystem::OnEventListenerChanged, base::Bind(&NativeExtensionBindingsSystem::OnEventListenerChanged,
base::Unretained(this)), base::Unretained(this)),
base::Bind(&APIActivityLogger::LogAPICall),
APILastError(base::Bind(&GetLastErrorParents), APILastError(base::Bind(&GetLastErrorParents),
base::Bind(&AddConsoleError))), base::Bind(&AddConsoleError))),
weak_factory_(this) { weak_factory_(this) {
......
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#include "extensions/renderer/script_context.h" #include "extensions/renderer/script_context.h"
#include "extensions/renderer/script_context_set.h" #include "extensions/renderer/script_context_set.h"
#include "extensions/renderer/string_source_map.h" #include "extensions/renderer/string_source_map.h"
#include "extensions/renderer/test_extensions_renderer_client.h"
#include "extensions/renderer/test_v8_extension_configuration.h" #include "extensions/renderer/test_v8_extension_configuration.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
...@@ -203,6 +204,7 @@ class NativeExtensionBindingsSystemUnittest : public APIBindingTest { ...@@ -203,6 +204,7 @@ class NativeExtensionBindingsSystemUnittest : public APIBindingTest {
std::unique_ptr<MockEventChangeHandler> event_change_handler_; std::unique_ptr<MockEventChangeHandler> event_change_handler_;
StringSourceMap source_map_; StringSourceMap source_map_;
TestExtensionsRendererClient renderer_client_;
DISALLOW_COPY_AND_ASSIGN(NativeExtensionBindingsSystemUnittest); DISALLOW_COPY_AND_ASSIGN(NativeExtensionBindingsSystemUnittest);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment