From ace50646dcd95261ef8e6aca2096458828532fbc Mon Sep 17 00:00:00 2001 From: dmazzoni Date: Mon, 31 Aug 2015 09:15:27 -0700 Subject: [PATCH] Fix crash when accessing automation node from stale AX tree The problem is that the root node of a stale AX tree computes its id dynamically, but if that AX tree doesn't exist anymore it was returning undefined, and then passing undefined to any of the native bindinds was a fatal error. The solution is just to return -1 instead of undefined as the id. BUG=526255 Review URL: https://codereview.chromium.org/1308813008 Cr-Commit-Position: refs/heads/master@{#346394} --- .../resources/extensions/automation/automation_node.js | 9 ++++++++- .../extensions/api_test/automation/tests/tabs/close_tab.js | 13 +++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/chrome/renderer/resources/extensions/automation/automation_node.js b/chrome/renderer/resources/extensions/automation/automation_node.js index 1598c6f9b0b8..5e96405fd1cb 100644 --- a/chrome/renderer/resources/extensions/automation/automation_node.js +++ b/chrome/renderer/resources/extensions/automation/automation_node.js @@ -757,7 +757,14 @@ AutomationRootNodeImpl.prototype = { axNodeDataCache_: null, get id() { - return GetRootID(this.treeID); + var result = GetRootID(this.treeID); + + // Don't return undefined, because the id is often passed directly + // as an argument to a native binding that expects only a valid number. + if (result === undefined) + return -1; + + return result; }, get: function(id) { diff --git a/chrome/test/data/extensions/api_test/automation/tests/tabs/close_tab.js b/chrome/test/data/extensions/api_test/automation/tests/tabs/close_tab.js index bb2d619b8c05..aa04332d6584 100644 --- a/chrome/test/data/extensions/api_test/automation/tests/tabs/close_tab.js +++ b/chrome/test/data/extensions/api_test/automation/tests/tabs/close_tab.js @@ -8,10 +8,15 @@ var allTests = [ chrome.tabs.create({'url': url}, function(tab) { chrome.automation.getTree(function(rootNode) { rootNode.addEventListener(EventType.destroyed, function() { - // Should get a 'destroyed' event when the tab is closed. - // TODO(aboxhall): assert actual cleanup has occurred once - // crbug.com/387954 makes it possible. - chrome.test.succeed(); + // Poll until the root node doesn't have a role anymore + // indicating that it really did get cleaned up. + function checkSuccess() { + if (rootNode.role === undefined) + chrome.test.succeed(); + else + window.setTimeout(checkSuccess, 10); + } + checkSuccess(); }); chrome.tabs.remove(tab.id); }); -- 2.11.4.GIT