Commit 9b006b56 authored by dtseng's avatar dtseng Committed by Commit bot

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

Original issue: https://codereview.chromium.org/880063002
Previous reland attempt: https://codereview.chromium.org/895623003/

This cl changes the way in which ChromeVox initially verbalizes a page which led to flakeyness in
  chromevox_tests --gtest_filter=BackgroundTest.*

To attempt and address the flakeyness,
- chromevox_e2e_test_base.js and chromevox_next_e2e_test_base.js now explicitly separates listening to page load events from the chrome.automation tree and the chrome.tabs APIs.
They are named: runWithLoadedTree, runWithLoadedTab, and runWithTab. The last method allows a caller to create a tab, but not wait for it to load (based on the tab status being 'complete').
- each test in background_test now no longer waits for ChromeVox to speak 'start' (which formerly could have been triggered by a loadComplete automation event).
- tab creation is still done via the tabs API, but listening for load is done via chrome.automation. (i.e. add a listener for an automation tree to load completely, then create a tab).

It is not known at this time if this cl will continue to cause flakes in the above tests. Try bots and local runs yielded no flakeyness. The next step may be to introduce logging to identify the source of the flakeyness.

TBR=tfarina

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

Cr-Commit-Position: refs/heads/master@{#315362}
parent fa52c810
......@@ -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) {
this.onLoadComplete(
{target: root, type: chrome.automation.EventType.loadComplete});
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});
}
}
},
......@@ -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);
......@@ -362,21 +353,11 @@ 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;
chrome.commands.onCommand.addListener(this.onGotCommand);
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,27 +94,25 @@ 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) {
var start = null;
this.runWithLoadedTree(doc,
function(root) {
var start = null;
// This occurs as a result of a load complete.
var start = AutomationUtil.findNodePost(root,
FORWARD,
AutomationPredicate.leaf);
// This occurs as a result of a load complete.
var start = AutomationUtil.findNodePost(root,
FORWARD,
AutomationPredicate.leaf);
var cursor = new cursors.Cursor(start, 0);
if (!opt_testType || opt_testType == this.CURSOR) {
var cursor = new cursors.Cursor(start, 0);
if (!opt_testType || opt_testType == this.CURSOR) {
var cursor = new cursors.Cursor(start, 0);
this.cursorMoveAndAssert(cursor, moves);
testDone();
} else if (opt_testType == this.RANGE) {
var range = new cursors.Range(cursor, cursor);
this.rangeMoveAndAssert(range, moves);
testDone();
}
}.bind(this));
this.cursorMoveAndAssert(cursor, moves);
testDone();
} else if (opt_testType == this.RANGE) {
var range = new cursors.Range(cursor, cursor);
this.rangeMoveAndAssert(range, moves);
testDone();
}
}.bind(this));
},
......
......@@ -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);
......
......@@ -58,11 +58,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 +86,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,20 @@ 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) {
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,7 +728,7 @@ 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) {
......
......@@ -17,7 +17,7 @@ class ExtensionJSBrowserTest : public JavaScriptBrowserTest {
public:
ExtensionJSBrowserTest();
virtual ~ExtensionJSBrowserTest();
~ExtensionJSBrowserTest() override;
protected:
// Waits for an extension to load; returns immediately if already loaded.
......
......@@ -19,16 +19,16 @@ class BrowserContext;
class ExtensionLoadWaiterOneShot : public content::NotificationObserver {
public:
ExtensionLoadWaiterOneShot();
virtual ~ExtensionLoadWaiterOneShot();
~ExtensionLoadWaiterOneShot() override;
// Waits for extension with |extension_id| to load. The id should be a pointer
// to a static char array.
void WaitForExtension(const char* extension_id, const base::Closure& load_cb);
// content::NotificationObserver overrides.
virtual void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
// Get the browser context associated with the loaded extension. Returns
// NULL if |WaitForExtension| was not previously called.
......
......@@ -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() {
widget_->RemoveObserver(this);
widget_->RemoveRemovalsObserver(this);
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