Commit 435fa9d3 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions] Remove RenderView Routing ID from AutomationInternal

The AutomationInternal API currently passes the RenderView routing ID
to the API implementation for enableTab and enableDesktop. This routing
ID is then stored, and used to "route" a response IPC 
(ExtensionMsg_AccessibilityEvent or
ExtensionMsg_AccessibilityLocationChange) back to the renderer.

However, the listener in the renderer is, in fact, just an IPCListener
that attaches itself to the main RenderThread. It doesn't look at the
routing at all. Thus, we don't need the routing ID for the browser to
send the message, and, since that's the only place it was used, don't 
need to send it in the API params.

Remove the routing ID parameter from both automationInternal.enableTab
and automationInternal.enableDesktop, and update the IPC messages.

Bug: 304341

Change-Id: I3a82ab3193c70e7cf8694277e28c82c47f431ce4
Reviewed-on: https://chromium-review.googlesource.com/721268Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522174}
parent 36b6fa26
...@@ -49,22 +49,18 @@ AutomationEventRouter::~AutomationEventRouter() { ...@@ -49,22 +49,18 @@ AutomationEventRouter::~AutomationEventRouter() {
void AutomationEventRouter::RegisterListenerForOneTree( void AutomationEventRouter::RegisterListenerForOneTree(
const ExtensionId& extension_id, const ExtensionId& extension_id,
int listener_process_id, int listener_process_id,
int listener_routing_id,
int source_ax_tree_id) { int source_ax_tree_id) {
Register(extension_id, Register(extension_id,
listener_process_id, listener_process_id,
listener_routing_id,
source_ax_tree_id, source_ax_tree_id,
false); false);
} }
void AutomationEventRouter::RegisterListenerWithDesktopPermission( void AutomationEventRouter::RegisterListenerWithDesktopPermission(
const ExtensionId& extension_id, const ExtensionId& extension_id,
int listener_process_id, int listener_process_id) {
int listener_routing_id) {
Register(extension_id, Register(extension_id,
listener_process_id, listener_process_id,
listener_routing_id,
api::automation::kDesktopTreeID, api::automation::kDesktopTreeID,
true); true);
} }
...@@ -85,8 +81,7 @@ void AutomationEventRouter::DispatchAccessibilityEvent( ...@@ -85,8 +81,7 @@ void AutomationEventRouter::DispatchAccessibilityEvent(
content::RenderProcessHost* rph = content::RenderProcessHost* rph =
content::RenderProcessHost::FromID(listener.process_id); content::RenderProcessHost::FromID(listener.process_id);
rph->Send(new ExtensionMsg_AccessibilityEvent(listener.routing_id, rph->Send(new ExtensionMsg_AccessibilityEvent(params,
params,
listener.is_active_profile)); listener.is_active_profile));
} }
} }
...@@ -102,9 +97,7 @@ void AutomationEventRouter::DispatchAccessibilityLocationChange( ...@@ -102,9 +97,7 @@ void AutomationEventRouter::DispatchAccessibilityLocationChange(
content::RenderProcessHost* rph = content::RenderProcessHost* rph =
content::RenderProcessHost::FromID(listener.process_id); content::RenderProcessHost::FromID(listener.process_id);
rph->Send(new ExtensionMsg_AccessibilityLocationChange( rph->Send(new ExtensionMsg_AccessibilityLocationChange(params));
listener.routing_id,
params));
} }
} }
...@@ -154,23 +147,18 @@ AutomationEventRouter::AutomationListener::~AutomationListener() { ...@@ -154,23 +147,18 @@ AutomationEventRouter::AutomationListener::~AutomationListener() {
void AutomationEventRouter::Register( void AutomationEventRouter::Register(
const ExtensionId& extension_id, const ExtensionId& extension_id,
int listener_process_id, int listener_process_id,
int listener_routing_id,
int ax_tree_id, int ax_tree_id,
bool desktop) { bool desktop) {
auto iter = std::find_if( auto iter =
listeners_.begin(), std::find_if(listeners_.begin(), listeners_.end(),
listeners_.end(), [listener_process_id](const AutomationListener& item) {
[listener_process_id, listener_routing_id]( return item.process_id == listener_process_id;
const AutomationListener& item) { });
return (item.process_id == listener_process_id &&
item.routing_id == listener_routing_id); // Add a new entry if we don't have one with that process.
});
// Add a new entry if we don't have one with that process and routing id.
if (iter == listeners_.end()) { if (iter == listeners_.end()) {
AutomationListener listener; AutomationListener listener;
listener.extension_id = extension_id; listener.extension_id = extension_id;
listener.routing_id = listener_routing_id;
listener.process_id = listener_process_id; listener.process_id = listener_process_id;
listener.desktop = desktop; listener.desktop = desktop;
listener.tree_ids.insert(ax_tree_id); listener.tree_ids.insert(ax_tree_id);
...@@ -179,8 +167,8 @@ void AutomationEventRouter::Register( ...@@ -179,8 +167,8 @@ void AutomationEventRouter::Register(
return; return;
} }
// We have an entry with that process and routing id, so update the set of // We have an entry with that process so update the set of tree ids it wants
// tree ids it wants to listen to, and update its desktop permission. // to listen to, and update its desktop permission.
iter->tree_ids.insert(ax_tree_id); iter->tree_ids.insert(ax_tree_id);
if (desktop) if (desktop)
iter->desktop = true; iter->desktop = true;
......
...@@ -36,21 +36,19 @@ class AutomationEventRouter : public content::NotificationObserver { ...@@ -36,21 +36,19 @@ class AutomationEventRouter : public content::NotificationObserver {
public: public:
static AutomationEventRouter* GetInstance(); static AutomationEventRouter* GetInstance();
// Indicates that the listener at |listener_process_id|, |listener_routing_id| // Indicates that the listener at |listener_process_id| wants to receive
// wants to receive automation events from the accessibility tree indicated // automation events from the accessibility tree indicated by
// by |source_ax_tree_id|. Automation events are forwarded from now on // |source_ax_tree_id|. Automation events are forwarded from now on until the
// until the listener process dies. // listener process dies.
void RegisterListenerForOneTree(const ExtensionId& extension_id, void RegisterListenerForOneTree(const ExtensionId& extension_id,
int listener_process_id, int listener_process_id,
int listener_routing_id,
int source_ax_tree_id); int source_ax_tree_id);
// Indicates that the listener at |listener_process_id|, |listener_routing_id| // Indicates that the listener at |listener_process_id| wants to receive
// wants to receive automation events from all accessibility trees because // automation events from all accessibility trees because it has Desktop
// it has Desktop permission. // permission.
void RegisterListenerWithDesktopPermission(const ExtensionId& extension_id, void RegisterListenerWithDesktopPermission(const ExtensionId& extension_id,
int listener_process_id, int listener_process_id);
int listener_routing_id);
void DispatchAccessibilityEvent( void DispatchAccessibilityEvent(
const ExtensionMsg_AccessibilityEventParams& params); const ExtensionMsg_AccessibilityEventParams& params);
...@@ -74,7 +72,6 @@ class AutomationEventRouter : public content::NotificationObserver { ...@@ -74,7 +72,6 @@ class AutomationEventRouter : public content::NotificationObserver {
~AutomationListener(); ~AutomationListener();
ExtensionId extension_id; ExtensionId extension_id;
int routing_id;
int process_id; int process_id;
bool desktop; bool desktop;
std::set<int> tree_ids; std::set<int> tree_ids;
...@@ -87,7 +84,6 @@ class AutomationEventRouter : public content::NotificationObserver { ...@@ -87,7 +84,6 @@ class AutomationEventRouter : public content::NotificationObserver {
void Register( void Register(
const ExtensionId& extension_id, const ExtensionId& extension_id,
int listener_process_id, int listener_process_id,
int listener_routing_id,
int source_ax_tree_id, int source_ax_tree_id,
bool desktop); bool desktop);
......
...@@ -295,7 +295,6 @@ AutomationInternalEnableTabFunction::Run() { ...@@ -295,7 +295,6 @@ AutomationInternalEnableTabFunction::Run() {
AutomationEventRouter::GetInstance()->RegisterListenerForOneTree( AutomationEventRouter::GetInstance()->RegisterListenerForOneTree(
extension_id(), extension_id(),
source_process_id(), source_process_id(),
params->args.routing_id,
ax_tree_id); ax_tree_id);
return RespondNow(ArgumentList( return RespondNow(ArgumentList(
...@@ -497,15 +496,9 @@ AutomationInternalEnableDesktopFunction::Run() { ...@@ -497,15 +496,9 @@ AutomationInternalEnableDesktopFunction::Run() {
if (!automation_info || !automation_info->desktop) if (!automation_info || !automation_info->desktop)
return RespondNow(Error("desktop permission must be requested")); return RespondNow(Error("desktop permission must be requested"));
using api::automation_internal::EnableDesktop::Params;
std::unique_ptr<Params> params(Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params.get());
// This gets removed when the extension process dies. // This gets removed when the extension process dies.
AutomationEventRouter::GetInstance()->RegisterListenerWithDesktopPermission( AutomationEventRouter::GetInstance()->RegisterListenerWithDesktopPermission(
extension_id(), extension_id(), source_process_id());
source_process_id(),
params->routing_id);
AutomationManagerAura::GetInstance()->Enable(browser_context()); AutomationManagerAura::GetInstance()->Enable(browser_context());
return RespondNow(NoArguments()); return RespondNow(NoArguments());
......
...@@ -82,7 +82,6 @@ namespace automationInternal { ...@@ -82,7 +82,6 @@ namespace automationInternal {
// Arguments for the enableTab function. // Arguments for the enableTab function.
dictionary EnableTabParams { dictionary EnableTabParams {
long routingID;
long? tabID; long? tabID;
}; };
...@@ -120,8 +119,7 @@ namespace automationInternal { ...@@ -120,8 +119,7 @@ namespace automationInternal {
static void enableFrame(long tree_id); static void enableFrame(long tree_id);
// Enables desktop automation. // Enables desktop automation.
static void enableDesktop(long routingID, static void enableDesktop(EnableDesktopCallback callback);
EnableDesktopCallback callback);
// Performs an action on an automation node. // Performs an action on an automation node.
static void performAction(PerformActionRequiredParams args, static void performAction(PerformActionRequiredParams args,
......
...@@ -108,14 +108,14 @@ IPC_STRUCT_END() ...@@ -108,14 +108,14 @@ IPC_STRUCT_END()
// Forward an accessibility message to an extension process where an // Forward an accessibility message to an extension process where an
// extension is using the automation API to listen for accessibility events. // extension is using the automation API to listen for accessibility events.
IPC_MESSAGE_ROUTED2(ExtensionMsg_AccessibilityEvent, IPC_MESSAGE_CONTROL2(ExtensionMsg_AccessibilityEvent,
ExtensionMsg_AccessibilityEventParams, ExtensionMsg_AccessibilityEventParams,
bool /* is_active_profile */) bool /* is_active_profile */)
// Forward an accessibility location change message to an extension process // Forward an accessibility location change message to an extension process
// where an extension is using the automation API to listen for // where an extension is using the automation API to listen for
// accessibility events. // accessibility events.
IPC_MESSAGE_ROUTED1(ExtensionMsg_AccessibilityLocationChange, IPC_MESSAGE_CONTROL1(ExtensionMsg_AccessibilityLocationChange,
ExtensionMsg_AccessibilityLocationChangeParams) ExtensionMsg_AccessibilityLocationChangeParams)
#endif // CHROME_COMMON_EXTENSIONS_CHROME_EXTENSION_MESSAGES_H_ #endif // CHROME_COMMON_EXTENSIONS_CHROME_EXTENSION_MESSAGES_H_
...@@ -399,7 +399,6 @@ AutomationInternalCustomBindings::AutomationInternalCustomBindings( ...@@ -399,7 +399,6 @@ AutomationInternalCustomBindings::AutomationInternalCustomBindings(
base::Unretained(this))) base::Unretained(this)))
ROUTE_FUNCTION(IsInteractPermitted); ROUTE_FUNCTION(IsInteractPermitted);
ROUTE_FUNCTION(GetSchemaAdditions); ROUTE_FUNCTION(GetSchemaAdditions);
ROUTE_FUNCTION(GetRoutingID);
ROUTE_FUNCTION(StartCachingAccessibilityTrees); ROUTE_FUNCTION(StartCachingAccessibilityTrees);
ROUTE_FUNCTION(DestroyAccessibilityTree); ROUTE_FUNCTION(DestroyAccessibilityTree);
ROUTE_FUNCTION(AddTreeChangeObserver); ROUTE_FUNCTION(AddTreeChangeObserver);
...@@ -835,12 +834,6 @@ void AutomationInternalCustomBindings::IsInteractPermitted( ...@@ -835,12 +834,6 @@ void AutomationInternalCustomBindings::IsInteractPermitted(
v8::Boolean::New(GetIsolate(), automation_info->interact)); v8::Boolean::New(GetIsolate(), automation_info->interact));
} }
void AutomationInternalCustomBindings::GetRoutingID(
const v8::FunctionCallbackInfo<v8::Value>& args) {
int routing_id = context()->GetRenderFrame()->GetRenderView()->GetRoutingID();
args.GetReturnValue().Set(v8::Integer::New(GetIsolate(), routing_id));
}
void AutomationInternalCustomBindings::StartCachingAccessibilityTrees( void AutomationInternalCustomBindings::StartCachingAccessibilityTrees(
const v8::FunctionCallbackInfo<v8::Value>& args) { const v8::FunctionCallbackInfo<v8::Value>& args) {
if (should_ignore_context_) if (should_ignore_context_)
......
...@@ -70,9 +70,6 @@ class AutomationInternalCustomBindings : public ObjectBackedNativeHandler, ...@@ -70,9 +70,6 @@ class AutomationInternalCustomBindings : public ObjectBackedNativeHandler,
// chrome.automation namespace. // chrome.automation namespace.
void GetSchemaAdditions(const v8::FunctionCallbackInfo<v8::Value>& args); void GetSchemaAdditions(const v8::FunctionCallbackInfo<v8::Value>& args);
// Get the routing ID for the extension.
void GetRoutingID(const v8::FunctionCallbackInfo<v8::Value>& args);
// This is called by automation_internal_custom_bindings.js to indicate // This is called by automation_internal_custom_bindings.js to indicate
// that an API was called that needs access to accessibility trees. This // that an API was called that needs access to accessibility trees. This
// enables the MessageFilter that allows us to listen to accessibility // enables the MessageFilter that allows us to listen to accessibility
......
...@@ -13,7 +13,6 @@ var automationInternal = ...@@ -13,7 +13,6 @@ var automationInternal =
var exceptionHandler = require('uncaught_exception_handler'); var exceptionHandler = require('uncaught_exception_handler');
var logging = requireNative('logging'); var logging = requireNative('logging');
var nativeAutomationInternal = requireNative('automationInternal'); var nativeAutomationInternal = requireNative('automationInternal');
var GetRoutingID = nativeAutomationInternal.GetRoutingID;
var DestroyAccessibilityTree = var DestroyAccessibilityTree =
nativeAutomationInternal.DestroyAccessibilityTree; nativeAutomationInternal.DestroyAccessibilityTree;
var GetIntAttribute = nativeAutomationInternal.GetIntAttribute; var GetIntAttribute = nativeAutomationInternal.GetIntAttribute;
...@@ -108,7 +107,6 @@ automation.registerCustomHook(function(bindingsAPI) { ...@@ -108,7 +107,6 @@ automation.registerCustomHook(function(bindingsAPI) {
// TODO(aboxhall, dtseng): Make this return the speced AutomationRootNode obj. // TODO(aboxhall, dtseng): Make this return the speced AutomationRootNode obj.
apiFunctions.setHandleRequest('getTree', function getTree(tabID, callback) { apiFunctions.setHandleRequest('getTree', function getTree(tabID, callback) {
var routingID = GetRoutingID();
StartCachingAccessibilityTrees(); StartCachingAccessibilityTrees();
// enableTab() ensures the renderer for the active or specified tab has // enableTab() ensures the renderer for the active or specified tab has
...@@ -118,7 +116,7 @@ automation.registerCustomHook(function(bindingsAPI) { ...@@ -118,7 +116,7 @@ automation.registerCustomHook(function(bindingsAPI) {
// the tree is available (either due to having been cached earlier, or after // the tree is available (either due to having been cached earlier, or after
// an accessibility event occurs which causes the tree to be populated), the // an accessibility event occurs which causes the tree to be populated), the
// callback can be called. // callback can be called.
var params = { routingID: routingID, tabID: tabID }; var params = { tabID: tabID };
automationInternal.enableTab(params, automationInternal.enableTab(params,
function onEnable(id) { function onEnable(id) {
if (hasLastError()) { if (hasLastError()) {
...@@ -139,11 +137,9 @@ automation.registerCustomHook(function(bindingsAPI) { ...@@ -139,11 +137,9 @@ automation.registerCustomHook(function(bindingsAPI) {
else else
idToCallback[DESKTOP_TREE_ID] = [callback]; idToCallback[DESKTOP_TREE_ID] = [callback];
var routingID = GetRoutingID();
// TODO(dtseng): Disable desktop tree once desktop object goes out of // TODO(dtseng): Disable desktop tree once desktop object goes out of
// scope. // scope.
automationInternal.enableDesktop(routingID, function() { automationInternal.enableDesktop(function() {
if (hasLastError()) { if (hasLastError()) {
AutomationRootNode.destroy(DESKTOP_TREE_ID); AutomationRootNode.destroy(DESKTOP_TREE_ID);
callback(); callback();
......
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