Skip to content

Commit

Permalink
[Extensions Bindings] Add support for DeclarativeEvents
Browse files Browse the repository at this point in the history
A small number of APIs have declarative events - events that support
"rules" rather than listeners.  These events take a list of conditions
and actions to perform when those conditions are met.  For instance,
declarativeContent.onPageChanged allows extensions to register an
action to make the extension's page action visible when a set of
conditions, like the page having a certain url, are met.

Create a DeclarativeEvent class in the native bindings, and wire it up
for use, and add tests for the same.

Some complications arise because each event can have a somewhat
different "rule" schema, since the actions and conditions for that event
can be unique. The way we specify these in the JSON schema files is
pretty convoluted, by specifying "any" object in the events.json file,
then listing "actions" and "conditions" in the specific API file, and
finally using the custom JS event bindings to validate them.
Unfortunately, this creates complexity in the native side as well, since
we cannot simply look up the schema directly.

BUG=653596

Review-Url: https://codereview.chromium.org/2754073002
Cr-Commit-Position: refs/heads/master@{#459176}
  • Loading branch information
rdcronin authored and Commit bot committed Mar 23, 2017
1 parent bedb0ae commit a45979a
Show file tree
Hide file tree
Showing 17 changed files with 761 additions and 17 deletions.
43 changes: 43 additions & 0 deletions chrome/browser/extensions/native_bindings_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,17 @@
// found in the LICENSE file.

#include "base/command_line.h"
#include "base/run_loop.h"
#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
#include "chrome/browser/extensions/extension_action.h"
#include "chrome/browser/extensions/extension_action_manager.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/sessions/session_tab_helper.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/test/base/ui_test_utils.h"
#include "extensions/common/switches.h"
#include "extensions/test/extension_test_message_listener.h"
#include "net/dns/mock_host_resolver.h"

namespace extensions {
Expand Down Expand Up @@ -44,4 +53,38 @@ IN_PROC_BROWSER_TEST_F(NativeBindingsApiTest, SimpleAppTest) {
ASSERT_TRUE(RunPlatformAppTest("native_bindings/platform_app")) << message_;
}

// Tests the declarativeContent API and declarative events.
IN_PROC_BROWSER_TEST_F(NativeBindingsApiTest, DeclarativeEvents) {
host_resolver()->AddRule("*", "127.0.0.1");
embedded_test_server()->ServeFilesFromDirectory(test_data_dir_);
ASSERT_TRUE(StartEmbeddedTestServer());
// Load an extension and wait for it to be ready.
ExtensionTestMessageListener listener("ready", false);
const Extension* extension = LoadExtension(
test_data_dir_.AppendASCII("native_bindings/declarative_content"));
ASSERT_TRUE(extension);
ASSERT_TRUE(listener.WaitUntilSatisfied());

// The extension's page action should currently be hidden.
ExtensionAction* page_action =
ExtensionActionManager::Get(profile())->GetPageAction(*extension);
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
int tab_id = SessionTabHelper::IdForTab(web_contents);
EXPECT_FALSE(page_action->GetIsVisible(tab_id));

// Navigating to example.com should show the page action.
ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL(
"example.com", "/native_bindings/simple.html"));
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(page_action->GetIsVisible(tab_id));

// And the extension should be notified of the click.
ExtensionTestMessageListener clicked_listener("clicked and removed", false);
ExtensionActionAPI::Get(profile())->DispatchExtensionActionClicked(
*page_action, web_contents);
ASSERT_TRUE(clicked_listener.WaitUntilSatisfied());
}

} // namespace extensions
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@

// Custom binding for the declarativeContent API.

var binding = require('binding').Binding.create('declarativeContent');
var binding =
apiBridge || require('binding').Binding.create('declarativeContent');

var utils = require('utils');
var validate = require('schemaUtils').validate;
var canonicalizeCompoundSelector =
requireNative('css_natives').CanonicalizeCompoundSelector;
var setIcon = require('setIcon').setIcon;

binding.registerCustomHook( function(api) {
binding.registerCustomHook(function(api) {
var declarativeContent = api.compiledApi;

// Returns the schema definition of type |typeId| defined in |namespace|.
Expand All @@ -34,8 +35,16 @@ binding.registerCustomHook( function(api) {
}
}
instance.instanceType = 'declarativeContent.' + typeId;
var schema = getSchema(typeId);
validate([instance], [schema]);
// TODO(devlin): This is wrong. It means we don't validate the construction
// of the instance (which really only matters for PageStateMatcher).
// Currently, we don't pass the schema to JS with native bindings because
// validation should be done natively. We'll need to fix this by either
// allowing some validation to occur in JS, or by moving the instantiation
// of these types to native code.
if (!apiBridge) {
var schema = getSchema(typeId);
validate([instance], [schema]);
}
}

function canonicalizeCssSelectors(selectors) {
Expand Down Expand Up @@ -73,4 +82,5 @@ binding.registerCustomHook( function(api) {
};
});

exports.$set('binding', binding.generate());
if (!apiBridge)
exports.$set('binding', binding.generate());
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Register a rule to show the page action whenever we see a page with 'example'
// in the host. Send messages after registration of the rule is complete and
// when the page action is clicked.

const kRuleId = 'rule1';

var rule = {
conditions: [
new chrome.declarativeContent.PageStateMatcher(
{pageUrl: {hostPrefix: 'example'}}),
], actions: [
new chrome.declarativeContent.ShowPageAction(),
],
id: kRuleId,
};

chrome.pageAction.onClicked.addListener(function() {
chrome.declarativeContent.onPageChanged.removeRules([kRuleId], function() {
chrome.declarativeContent.onPageChanged.getRules(function(rules) {
chrome.test.assertEq(0, rules.length);
chrome.test.sendMessage('clicked and removed');
});
});
});

chrome.declarativeContent.onPageChanged.addRules([rule], function() {
chrome.declarativeContent.onPageChanged.getRules(function(rules) {
chrome.test.assertEq(1, rules.length);
chrome.test.assertEq(kRuleId, rules[0].id);
chrome.test.sendMessage('ready');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "Test declarative content",
"description": "Test declarative content with native bindings",
"version": "0.1",
"manifest_version": 2,
"permissions": ["declarativeContent"],
"page_action": {},
"background": {
"peristent": false,
"scripts": ["background.js"]
}
}
3 changes: 3 additions & 0 deletions extensions/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ source_set("renderer") {
"context_menus_custom_bindings.h",
"css_native_handler.cc",
"css_native_handler.h",
"declarative_event.cc",
"declarative_event.h",
"dispatcher.cc",
"dispatcher.h",
"dispatcher_delegate.h",
Expand Down Expand Up @@ -305,6 +307,7 @@ source_set("unit_tests") {
"api_test_base.h",
"api_test_base_unittest.cc",
"argument_spec_unittest.cc",
"declarative_event_unittest.cc",
"event_unittest.cc",
"gc_callback_unittest.cc",
"json_schema_unittest.cc",
Expand Down
76 changes: 67 additions & 9 deletions extensions/renderer/api_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
#include "extensions/renderer/api_request_handler.h"
#include "extensions/renderer/api_signature.h"
#include "extensions/renderer/api_type_reference_map.h"
#include "extensions/renderer/declarative_event.h"
#include "extensions/renderer/v8_helpers.h"
#include "gin/arguments.h"
#include "gin/handle.h"
#include "gin/per_context_data.h"
#include "third_party/WebKit/public/web/WebUserGestureIndicator.h"

Expand Down Expand Up @@ -87,27 +89,45 @@ struct APIBinding::MethodData {
APIBinding::HandlerCallback callback;
};

// TODO(devlin): Maybe separate EventData into two classes? Rules, actions, and
// conditions should never be present on vanilla events.
struct APIBinding::EventData {
EventData(std::string exposed_name,
std::string full_name,
bool supports_filters,
APIEventHandler* event_handler)
bool supports_rules,
std::vector<std::string> actions,
std::vector<std::string> conditions,
APIBinding* binding)
: exposed_name(std::move(exposed_name)),
full_name(std::move(full_name)),
supports_filters(supports_filters),
event_handler(event_handler) {}
supports_rules(supports_rules),
actions(std::move(actions)),
conditions(std::move(conditions)),
binding(binding) {}

// The name of the event on the API object (e.g. onCreated).
std::string exposed_name;

// The fully-specified name of the event (e.g. tabs.onCreated).
std::string full_name;

// Whether the event supports filters.
bool supports_filters;
// The associated event handler. This raw pointer is safe because the

// Whether the event supports rules.
bool supports_rules;

// The associated actions and conditions for declarative events.
std::vector<std::string> actions;
std::vector<std::string> conditions;

// The associated APIBinding. This raw pointer is safe because the
// EventData is only accessed from the callbacks associated with the
// APIBinding, and both the APIBinding and APIEventHandler are owned by the
// same object (the APIBindingsSystem).
APIEventHandler* event_handler;
APIBinding* binding;
};

struct APIBinding::CustomPropertyData {
Expand Down Expand Up @@ -143,6 +163,7 @@ APIBinding::APIBinding(const std::string& api_name,
binding_hooks_(std::move(binding_hooks)),
type_refs_(type_refs),
request_handler_(request_handler),
event_handler_(event_handler),
weak_factory_(this) {
// TODO(devlin): It might make sense to instantiate the object_template_
// directly here, which would avoid the need to hold on to
Expand Down Expand Up @@ -221,9 +242,35 @@ APIBinding::APIBinding(const std::string& api_name,
const base::ListValue* filters = nullptr;
bool supports_filters =
event_dict->GetList("filters", &filters) && !filters->empty();
events_.push_back(
base::MakeUnique<EventData>(std::move(name), std::move(full_name),
supports_filters, event_handler));

std::vector<std::string> rule_actions;
std::vector<std::string> rule_conditions;
const base::DictionaryValue* options = nullptr;
bool supports_rules = false;
if (event_dict->GetDictionary("options", &options) &&
options->GetBoolean("supportsRules", &supports_rules) &&
supports_rules) {
bool supports_listeners = false;
DCHECK(options->GetBoolean("supportsListeners", &supports_listeners));
DCHECK(!supports_listeners)
<< "Events cannot support rules and listeners.";
auto get_values = [options](base::StringPiece name,
std::vector<std::string>* out_value) {
const base::ListValue* list = nullptr;
CHECK(options->GetList(name, &list));
for (const auto& entry : *list) {
DCHECK(entry->is_string());
out_value->push_back(entry->GetString());
}
};
get_values("actions", &rule_actions);
get_values("conditions", &rule_conditions);
}

events_.push_back(base::MakeUnique<EventData>(
std::move(name), std::move(full_name), supports_filters,
supports_rules, std::move(rule_actions), std::move(rule_conditions),
this));
}
}
}
Expand Down Expand Up @@ -382,8 +429,19 @@ void APIBinding::GetEventObject(
CHECK(info.Data()->IsExternal());
auto* event_data =
static_cast<EventData*>(info.Data().As<v8::External>()->Value());
info.GetReturnValue().Set(event_data->event_handler->CreateEventInstance(
event_data->full_name, event_data->supports_filters, context));
v8::Local<v8::Value> retval;
if (event_data->supports_rules) {
gin::Handle<DeclarativeEvent> event = gin::CreateHandle(
isolate, new DeclarativeEvent(
event_data->full_name, event_data->binding->type_refs_,
event_data->binding->request_handler_, event_data->actions,
event_data->conditions));
retval = event.ToV8();
} else {
retval = event_data->binding->event_handler_->CreateEventInstance(
event_data->full_name, event_data->supports_filters, context);
}
info.GetReturnValue().Set(retval);
}

void APIBinding::GetCustomPropertyObject(
Expand Down
6 changes: 5 additions & 1 deletion extensions/renderer/api_binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,16 @@ class APIBinding {
std::unique_ptr<APIBindingHooks> binding_hooks_;

// The reference map for all known types; required to outlive this object.
const APITypeReferenceMap* type_refs_;
APITypeReferenceMap* type_refs_;

// The associated request handler, shared between this and other bindings.
// Required to outlive this object.
APIRequestHandler* request_handler_;

// The associated event handler, shared between this and other bindings.
// Required to outlive this object.
APIEventHandler* event_handler_;

// The template for this API. Note: some methods may only be available in
// certain contexts, but this template contains all methods. Those that are
// unavailable are removed after object instantiation.
Expand Down
2 changes: 1 addition & 1 deletion extensions/renderer/api_bindings_system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void APIBindingsSystem::InitializeType(const std::string& type_name) {
// will also go away if/when we generate all these specifications.
std::string::size_type dot = type_name.rfind('.');
// The type name should be fully qualified (include the API name).
DCHECK_NE(std::string::npos, dot);
DCHECK_NE(std::string::npos, dot) << type_name;
DCHECK_LT(dot, type_name.size() - 1);
std::string api_name = type_name.substr(0, dot);
// If we've already instantiated the binding, the type should have been in
Expand Down
3 changes: 3 additions & 0 deletions extensions/renderer/api_signature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ APISignature::APISignature(const base::ListValue& specification) {
}
}

APISignature::APISignature(std::vector<std::unique_ptr<ArgumentSpec>> signature)
: signature_(std::move(signature)) {}

APISignature::~APISignature() {}

bool APISignature::ParseArgumentsToV8(
Expand Down
3 changes: 2 additions & 1 deletion extensions/renderer/api_signature.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ class ArgumentSpec;
// ability to match provided arguments and convert them to base::Values.
class APISignature {
public:
APISignature(const base::ListValue& specification);
explicit APISignature(const base::ListValue& specification);
explicit APISignature(std::vector<std::unique_ptr<ArgumentSpec>> signature);
~APISignature();

// Parses |arguments| against this signature, and populates |args_out| with
Expand Down
5 changes: 5 additions & 0 deletions extensions/renderer/api_type_reference_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,9 @@ const APISignature* APITypeReferenceMap::GetTypeMethodSignature(
return iter == type_methods_.end() ? nullptr : iter->second.get();
}

bool APITypeReferenceMap::HasTypeMethodSignature(
const std::string& name) const {
return type_methods_.find(name) != type_methods_.end();
}

} // namespace extensions
5 changes: 5 additions & 0 deletions extensions/renderer/api_type_reference_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ class APITypeReferenceMap {
// storage.StorageArea.get).
const APISignature* GetTypeMethodSignature(const std::string& name) const;

// Returns true if the map has a signature for the given |name|. Unlike
// GetTypeMethodSignature(), this will not try to fetch the type by loading
// an API.
bool HasTypeMethodSignature(const std::string& name) const;

bool empty() const { return type_refs_.empty(); }
size_t size() const { return type_refs_.size(); }

Expand Down
2 changes: 2 additions & 0 deletions extensions/renderer/argument_spec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ ArgumentSpec::ArgumentSpec(const base::Value& value)
InitializeType(dict);
}

ArgumentSpec::ArgumentSpec(ArgumentType type) : type_(type), optional_(false) {}

void ArgumentSpec::InitializeType(const base::DictionaryValue* dict) {
std::string ref_string;
if (dict->GetString("$ref", &ref_string)) {
Expand Down
Loading

0 comments on commit a45979a

Please sign in to comment.