Commit 47cb3b37 authored by dtseng's avatar dtseng Committed by Commit bot

Reland #4: Ensure WebView notifies desktop automation on creation, destruction, and change

Previous attempts:
https://codereview.chromium.org/880063002
https://codereview.chromium.org/895623003/
https://codereview.chromium.org/890013006/
https://codereview.chromium.org/907973004/

This latest attempt removes the logic that reloads ChromeVox at the beginning of a test.

Locally, under extremely heavy load, tests progressively ran slowe. The same copy of a test ran 1 second slower than a previous copy. Ultimately, this led to a timeout.

With the above change, this no longer occurs and hopefully fixes the flakeyness.

Change is located in chromevox_e2e_test_base.js.

TBR=dmazzoni@chromium.org

Review URL: https://codereview.chromium.org/940763002

Cr-Commit-Position: refs/heads/master@{#316950}
parent f6ee6a22
......@@ -1511,6 +1511,7 @@ chrome.automation.RoleType = {
unknown: 'unknown',
tooltip: 'tooltip',
webArea: 'webArea',
webView: 'webView',
window: 'window'
};
/**
......
......@@ -6,6 +6,7 @@ goog.provide('cvox.ChromeVoxEditableTextBase');
goog.provide('cvox.TextChangeEvent');
goog.provide('cvox.TypingEcho');
goog.require('cvox.AbstractTts');
goog.require('cvox.ChromeVox');
goog.require('cvox.TtsInterface');
goog.require('goog.i18n.MessageFormat');
......
......@@ -88,8 +88,12 @@ AutomationUtil.findNextSubtree = function(cur, dir) {
while (cur) {
var next = dir == Dir.BACKWARD ?
cur.previousSibling : cur.nextSibling;
if (!AutomationUtil.isInSameTree(cur, next))
return null;
if (next)
return next;
if (!AutomationUtil.isInSameTree(cur, cur.parent))
return null;
cur = cur.parent;
}
};
......@@ -164,6 +168,10 @@ AutomationUtil.getAncestors = function(node) {
var candidate = node;
while (candidate) {
ret.push(candidate);
if (!AutomationUtil.isInSameTree(candidate, candidate.parent))
break;
candidate = candidate.parent;
}
return ret.reverse();
......@@ -226,4 +234,17 @@ AutomationUtil.getDirection = function(nodeA, nodeB) {
return divA.indexInParent <= divB.indexInParent ? Dir.FORWARD : Dir.BACKWARD;
};
/**
* Determines whether the two given nodes come from the same tree source.
* @param {AutomationNode} a
* @param {AutomationNode} b
* @return {boolean}
*/
AutomationUtil.isInSameTree = function(a, b) {
if (!a || !b)
return true;
return a.root === b.root;
};
}); // goog.scope
......@@ -27,7 +27,7 @@ AutomationUtilE2ETest.prototype = {
};
TEST_F('AutomationUtilE2ETest', 'GetAncestors', function() {
this.runWithAutomation(this.basicDoc,
this.runWithLoadedTree(this.basicDoc,
function(root) {
var expectedLength = 1;
while (root) {
......@@ -40,7 +40,7 @@ TEST_F('AutomationUtilE2ETest', 'GetAncestors', function() {
});
TEST_F('AutomationUtilE2ETest', 'GetUniqueAncestors', function() {
this.runWithAutomation(this.basicDoc, function(root) {
this.runWithLoadedTree(this.basicDoc, function(root) {
var leftmost = root, rightmost = root;
while (leftmost.firstChild)
leftmost = leftmost.firstChild;
......@@ -83,7 +83,7 @@ TEST_F('AutomationUtilE2ETest', 'GetUniqueAncestors', function() {
});
TEST_F('AutomationUtilE2ETest', 'GetDirection', function() {
this.runWithAutomation(this.basicDoc, function(root) {
this.runWithLoadedTree(this.basicDoc, function(root) {
var left = root, right = root;
// Same node.
......
......@@ -16,7 +16,6 @@ goog.require('Output');
goog.require('Output.EventType');
goog.require('cursors.Cursor');
goog.require('cvox.ChromeVoxEditableTextBase');
goog.require('cvox.TabsApiHandler');
goog.scope(function() {
var AutomationNode = chrome.automation.AutomationNode;
......@@ -36,14 +35,6 @@ Background = function() {
*/
this.whitelist_ = ['chromevox_next_test'];
/**
* @type {cvox.TabsApiHandler}
* @private
*/
this.tabsHandler_ = new cvox.TabsApiHandler(cvox.ChromeVox.tts,
cvox.ChromeVox.braille,
cvox.ChromeVox.earcons);
/**
* @type {cursors.Range}
* @private
......@@ -82,43 +73,29 @@ Background = function() {
// Register listeners for ...
// Desktop.
chrome.automation.getDesktop(this.onGotTree);
// Tabs.
chrome.tabs.onUpdated.addListener(this.onTabUpdated);
chrome.automation.getDesktop(this.onGotDesktop);
};
Background.prototype = {
/**
* Handles chrome.tabs.onUpdated.
* @param {number} tabId
* @param {Object} changeInfo
*/
onTabUpdated: function(tabId, changeInfo) {
if (changeInfo.status != 'complete')
return;
chrome.tabs.get(tabId, function(tab) {
if (!tab.url)
return;
var next = this.isWhitelisted_(tab.url);
this.toggleChromeVoxVersion({next: next, classic: !next});
}.bind(this));
},
/**
* Handles all setup once a new automation tree appears.
* @param {chrome.automation.AutomationNode} root
* @param {chrome.automation.AutomationNode} desktop
*/
onGotTree: function(root) {
onGotDesktop: function(desktop) {
// Register all automation event listeners.
for (var eventType in this.listeners_)
root.addEventListener(eventType, this.listeners_[eventType], true);
if (root.attributes.docLoaded) {
desktop.addEventListener(eventType, this.listeners_[eventType], true);
// The focused state gets set on the containing webView node.
var webView = desktop.find({role: chrome.automation.RoleType.webView,
state: {focused: true}});
if (webView) {
var root = webView.find({role: chrome.automation.RoleType.rootWebArea});
if (root) {
this.onLoadComplete(
{target: root, type: chrome.automation.EventType.loadComplete});
{target: root,
type: chrome.automation.EventType.loadComplete});
}
}
},
......@@ -270,14 +247,28 @@ Background.prototype = {
* @param {Object} evt
*/
onLoadComplete: function(evt) {
var next = this.isWhitelisted_(evt.target.attributes.url);
this.toggleChromeVoxVersion({next: next, classic: !next});
// Don't process nodes inside of web content if ChromeVox Next is inactive.
if (evt.target.root.role != chrome.automation.RoleType.desktop &&
!this.active_)
return;
var node = AutomationUtil.findNodePost(evt.target,
if (this.currentRange_)
return;
var root = evt.target;
var webView = root;
while (webView && webView.role != chrome.automation.RoleType.webView)
webView = webView.parent;
if (!webView || !webView.state.focused)
return;
var node = AutomationUtil.findNodePost(root,
Dir.FORWARD,
AutomationPredicate.leaf);
if (node)
this.currentRange_ = cursors.Range.fromNode(node);
......@@ -363,20 +354,10 @@ Background.prototype = {
if (opt_options.next) {
if (!chrome.commands.onCommand.hasListener(this.onGotCommand))
chrome.commands.onCommand.addListener(this.onGotCommand);
if (!this.active_)
chrome.automation.getTree(this.onGotTree);
this.active_ = true;
} else {
if (chrome.commands.onCommand.hasListener(this.onGotCommand))
chrome.commands.onCommand.removeListener(this.onGotCommand);
if (this.active_) {
for (var eventType in this.listeners_) {
this.currentRange_.getStart().getNode().root.removeEventListener(
eventType, this.listeners_[eventType], true);
}
}
this.active_ = false;
}
......
......@@ -92,22 +92,17 @@ TEST_F('BackgroundTest', 'DesktopFocus', function() {
/** Tests feedback once a page loads. */
TEST_F('BackgroundTest', 'InitialFeedback', function() {
cvox.ChromeVox.tts.expectSpeech('start', function() {
global.backgroundObj.onGotCommand('nextLine');
}, true);
cvox.ChromeVox.tts.expectSpeech('end', testDone, true);
cvox.ChromeVox.tts.expectSpeech('start', testDone);
this.runWithDocument(function() {/*!
this.runWithTab(function() {/*!
<p>start
<p>end
*/},
function() {});
*/});
});
/** Tests consistency of navigating forward and backward. */
TEST_F('BackgroundTest', 'ForwardBackwardNavigation', function() {
cvox.ChromeVox.tts.expectSpeech('start', null, true);
this.runWithDocument(this.linksAndHeadingsDoc, function() {
this.runWithLoadedTree(this.linksAndHeadingsDoc, function() {
var doCmd = this.doCmd.bind(this);
var expectAfter =
cvox.ChromeVox.tts.expectSpeechAfter.bind(cvox.ChromeVox.tts);
......@@ -137,8 +132,7 @@ TEST_F('BackgroundTest', 'ForwardBackwardNavigation', function() {
});
TEST_F('BackgroundTest', 'CaretNavigation', function() {
cvox.ChromeVox.tts.expectSpeech('start', null, true);
this.runWithDocument(this.linksAndHeadingsDoc, function() {
this.runWithLoadedTree(this.linksAndHeadingsDoc, function() {
var doCmd = this.doCmd.bind(this);
var expectAfter =
cvox.ChromeVox.tts.expectSpeechAfter.bind(cvox.ChromeVox.tts);
......@@ -168,7 +162,7 @@ TEST_F('BackgroundTest', 'CaretNavigation', function() {
// Flaky: http://crbug.com/451362
TEST_F('BackgroundTest', 'DISABLED_SelectSingleBasic', function() {
this.runWithDocument(this.formsDoc, function(tabId) {
this.runWithLoadedTree(this.formsDoc, function(tabId) {
var sendDownToSelect =
this.sendKeyToElement.bind(this, tabId, 'Down', '#fruitSelect');
var expect = cvox.ChromeVox.tts.expectSpeech.bind(cvox.ChromeVox.tts);
......@@ -180,8 +174,7 @@ TEST_F('BackgroundTest', 'DISABLED_SelectSingleBasic', function() {
});
TEST_F('BackgroundTest', 'ContinuousRead', function() {
cvox.ChromeVox.tts.expectSpeech('start', null, true);
this.runWithDocument(this.linksAndHeadingsDoc, function() {
this.runWithLoadedTree(this.linksAndHeadingsDoc, function() {
var doCmd = this.doCmd.bind(this);
var expect =
cvox.ChromeVox.tts.expectSpeechAfter.bind(cvox.ChromeVox.tts);
......
......@@ -70,6 +70,7 @@ CursorsTest.prototype = {
range = range.move(move[0], move[1]);
var expectedStart = move[2];
var expectedEnd = move[3];
this.makeCursorAssertion(expectedStart, range.getStart());
this.makeCursorAssertion(expectedEnd, range.getEnd());
}
......@@ -93,9 +94,8 @@ CursorsTest.prototype = {
* @param {string=} opt_testType Either CURSOR or RANGE.
*/
runCursorMovesOnDocument: function(doc, moves, opt_testType) {
this.runWithDocument(doc,
function() {
chrome.automation.getTree(function(root) {
this.runWithLoadedTree(doc,
function(root) {
var start = null;
// This occurs as a result of a load complete.
......@@ -114,7 +114,6 @@ CursorsTest.prototype = {
testDone();
}
}.bind(this));
}.bind(this));
},
simpleDoc: function() {/*!
......
......@@ -13,6 +13,8 @@ goog.require('AutomationUtil.Dir');
goog.require('cursors.Cursor');
goog.require('cursors.Range');
goog.require('cursors.Unit');
goog.require('cvox.AbstractEarcons');
goog.require('cvox.NavBraille');
goog.require('cvox.Spannable');
goog.require('cvox.ValueSelectionSpan');
goog.require('cvox.ValueSpan');
......
......@@ -22,7 +22,7 @@ OutputE2ETest.prototype = {
};
TEST_F('OutputE2ETest', 'RenderBasic', function() {
this.runWithAutomation('<a href="#">Click here</a>',
this.runWithLoadedTree('<a href="#">Click here</a>',
function(root) {
var link = root.firstChild.firstChild;
var range = cursors.Range.fromNode(link);
......
......@@ -43,11 +43,6 @@ ChromeVoxE2ETest.prototype = {
/** @override */
testGenPreamble: function() {
GEN_BLOCK(function() {/*!
if (chromeos::AccessibilityManager::Get()->IsSpokenFeedbackEnabled()) {
chromeos::AccessibilityManager::Get()->EnableSpokenFeedback(false,
ui::A11Y_NOTIFICATION_NONE);
}
base::Closure load_cb =
base::Bind(&chromeos::AccessibilityManager::EnableSpokenFeedback,
base::Unretained(chromeos::AccessibilityManager::Get()),
......@@ -58,11 +53,26 @@ ChromeVoxE2ETest.prototype = {
},
/**
* Run a test with the specified HTML snippet loaded.
* Launch a new tab, wait until tab status complete, then run callback.
* @param {function() : void} doc Snippet wrapped inside of a function.
* @param {function()} callback Called once the document is ready.
*/
runWithDocument: function(doc, callback) {
runWithLoadedTab: function(doc, callback) {
this.launchNewTabWithDoc(doc, function(tab) {
chrome.tabs.onUpdated.addListener(function(tabId, changeInfo) {
if (tabId == tab.id && changeInfo.status == 'complete') {
callback(tabId);
}
});
});
},
/**
* Launches the given document in a new tab.
* @param {function() : void} doc Snippet wrapped inside of a function.
* @param {function()} opt_callback Called once the document is created.
*/
runWithTab: function(doc, opt_callback) {
var docString = TestUtils.extractHtmlFromCommentEncodedString(doc);
var url = 'data:text/html,<!doctype html>' +
docString +
......@@ -71,13 +81,7 @@ ChromeVoxE2ETest.prototype = {
active: true,
url: url
};
chrome.tabs.create(createParams, function(tab) {
chrome.tabs.onUpdated.addListener(function(tabId, changeInfo) {
if (tabId == tab.id && changeInfo.status == 'complete') {
callback(tabId);
}
});
});
chrome.tabs.create(createParams, opt_callback);
},
/**
......
......@@ -29,11 +29,24 @@ ChromeVoxNextE2ETest.prototype = {
GEN('#include "base/command_line.h"');
},
runWithAutomation: function(doc, callback) {
this.runWithDocument(doc, function() {
chrome.automation.getTree(function(root) {
callback(root);
}.bind(this));
/**
* Launches a new tab with the given document, and runs callback when a load
* complete fires.
* @param {function() : void} doc Snippet wrapped inside of a function.
* @param {function()} opt_callback Called once the document is ready.
*/
runWithLoadedTree: function(doc, callback) {
chrome.automation.getDesktop(function(r) {
function callbackInternal(evt) {
if (!evt.target.attributes.url ||
evt.target.attributes.url.indexOf('test') == -1)
return;
r.removeEventListener(callbackInternal);
callback(evt.target);
}
r.addEventListener('loadComplete', callbackInternal, true);
this.runWithTab(doc);
}.bind(this));
}
};
......@@ -113,7 +113,7 @@ MockTts.prototype = {
// Process any idleUtterances.
this.idleUtterances_.forEach(function(utterance) {
this.process_(utterance, true);
});
}.bind(this));
},
/**
......
......@@ -728,18 +728,20 @@ AutomationRootNodeImpl.prototype = {
// TODO(dtseng): Make into set listing all hosting node roles.
if (nodeData.role == schema.RoleType.webView) {
if (nodeImpl.pendingChildFrame === undefined)
if (nodeImpl.childTreeID !== nodeData.intAttributes.childTreeId)
nodeImpl.pendingChildFrame = true;
if (nodeImpl.pendingChildFrame) {
nodeImpl.childTreeID = nodeData.intAttributes.childTreeId;
automationInternal.enableFrame(nodeImpl.childTreeID);
automationUtil.storeTreeCallback(nodeImpl.childTreeID, function(root) {
nodeImpl.pendingChildFrame = false;
nodeImpl.childTree = root;
privates(root).impl.hostTree = node;
if (root.attributes.docLoadingProgress == 1)
privates(root).impl.dispatchEvent(schema.EventType.loadComplete);
nodeImpl.dispatchEvent(schema.EventType.childrenChanged);
});
automationInternal.enableFrame(nodeImpl.childTreeID);
}
}
for (var key in AutomationAttributeDefaults) {
......
......@@ -76,10 +76,11 @@ void AXAuraObjCache::Remove(int32 id) {
delete obj;
}
AXAuraObjCache::AXAuraObjCache() : current_id_(1) {
AXAuraObjCache::AXAuraObjCache() : current_id_(1), is_destroying_(false) {
}
AXAuraObjCache::~AXAuraObjCache() {
is_destroying_ = true;
STLDeleteContainerPairSecondPointers(cache_.begin(), cache_.end());
cache_.clear();
}
......
......@@ -52,6 +52,9 @@ class VIEWS_EXPORT AXAuraObjCache {
// Remove a cached entry based on an id.
void Remove(int32 id);
// Indicates if this object's currently being destroyed.
bool is_destroying() { return is_destroying_; }
private:
friend struct DefaultSingletonTraits<AXAuraObjCache>;
......@@ -75,6 +78,9 @@ class VIEWS_EXPORT AXAuraObjCache {
std::map<int32, AXAuraObjWrapper*> cache_;
int32 current_id_;
// True immediately when entering this object's destructor.
bool is_destroying_;
DISALLOW_COPY_AND_ASSIGN(AXAuraObjCache);
};
......
......@@ -17,8 +17,10 @@ AXWidgetObjWrapper::AXWidgetObjWrapper(Widget* widget) : widget_(widget) {
}
AXWidgetObjWrapper::~AXWidgetObjWrapper() {
if (!AXAuraObjCache::GetInstance()->is_destroying()) {
widget_->RemoveObserver(this);
widget_->RemoveRemovalsObserver(this);
}
widget_ = NULL;
}
......
......@@ -78,7 +78,7 @@ void WebView::SetWebContents(content::WebContents* replacement) {
DCHECK(!is_embedding_fullscreen_widget_);
}
AttachWebContents();
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
void WebView::SetEmbedFullscreenWidgetMode(bool enable) {
......@@ -266,7 +266,7 @@ gfx::Size WebView::GetPreferredSize() const {
void WebView::RenderProcessExited(content::RenderProcessHost* host,
base::TerminationStatus status,
int exit_code) {
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
void WebView::RenderProcessHostDestroyed(content::RenderProcessHost* host) {
......@@ -293,11 +293,11 @@ bool WebView::EmbedsFullscreenWidget() const {
// WebView, content::WebContentsObserver implementation:
void WebView::RenderViewReady() {
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
void WebView::RenderViewDeleted(content::RenderViewHost* render_view_host) {
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
void WebView::RenderViewHostChanged(content::RenderViewHost* old_host,
......@@ -305,7 +305,7 @@ void WebView::RenderViewHostChanged(content::RenderViewHost* old_host,
FocusManager* const focus_manager = GetFocusManager();
if (focus_manager && focus_manager->GetFocusedView() == this)
OnFocus();
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
void WebView::WebContentsDestroyed() {
......@@ -313,7 +313,7 @@ void WebView::WebContentsDestroyed() {
observing_render_process_host_->RemoveObserver(this);
observing_render_process_host_ = nullptr;
}
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
void WebView::DidShowFullscreenWidget(int routing_id) {
......@@ -332,11 +332,11 @@ void WebView::DidToggleFullscreenModeForTab(bool entered_fullscreen) {
}
void WebView::DidAttachInterstitialPage() {
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
void WebView::DidDetachInterstitialPage() {
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
////////////////////////////////////////////////////////////////////////////////
......@@ -398,14 +398,19 @@ void WebView::ReattachForFullscreenChange(bool enter_fullscreen) {
// the same. So, do not change attachment.
OnBoundsChanged(bounds());
}
NotifyMaybeTextInputClientChanged();
NotifyMaybeTextInputClientAndAccessibilityChanged();
}
void WebView::NotifyMaybeTextInputClientChanged() {
void WebView::NotifyMaybeTextInputClientAndAccessibilityChanged() {
// Update the TextInputClient as needed; see GetTextInputClient().
FocusManager* const focus_manager = GetFocusManager();
if (focus_manager)
focus_manager->OnTextInputClientChanged(this);
#if defined(OS_CHROMEOS)
if (web_contents())
NotifyAccessibilityEvent(ui::AX_EVENT_CHILDREN_CHANGED, false);
#endif // defined OS_CHROMEOS
}
content::WebContents* WebView::CreateWebContents(
......
......@@ -149,7 +149,7 @@ class WEBVIEW_EXPORT WebView : public View,
void AttachWebContents();
void DetachWebContents();
void ReattachForFullscreenChange(bool enter_fullscreen);
void NotifyMaybeTextInputClientChanged();
void NotifyMaybeTextInputClientAndAccessibilityChanged();
// Create a regular or test web contents (based on whether we're running
// in a unit test or not).
......
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