Commit 4e8f4c06 authored by jiayl's avatar jiayl Committed by Commit bot

Revert of Reland #2: Ensure WebView notifies desktop automation on creation,...

Revert of Reland #2: Ensure WebView notifies desktop automation on creation, destruction, and change  Original (patchset #7 id:120001 of https://codereview.chromium.org/890013006/)

Reason for revert:
Suspected to cause chromeos vox test failure:
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%281%29/builds/56046

Original issue's description:
> 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
>
> Committed: https://crrev.com/9b006b56ca402da3774708f08e062c89018d57a6
> Cr-Commit-Position: refs/heads/master@{#315362}

TBR=dmazzoni@chromium.org,tfarina@chromium.org,dtseng@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

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

Cr-Commit-Position: refs/heads/master@{#315373}
parent 9a77a723
......@@ -1511,7 +1511,6 @@ chrome.automation.RoleType = {
unknown: 'unknown',
tooltip: 'tooltip',
webArea: 'webArea',
webView: 'webView',
window: 'window'
};
/**
......
......@@ -6,7 +6,6 @@ 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,12 +88,8 @@ 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;
}
};
......@@ -168,10 +164,6 @@ 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();
......@@ -234,17 +226,4 @@ 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.runWithLoadedTree(this.basicDoc,
this.runWithAutomation(this.basicDoc,
function(root) {
var expectedLength = 1;
while (root) {
......@@ -40,7 +40,7 @@ TEST_F('AutomationUtilE2ETest', 'GetAncestors', function() {
});
TEST_F('AutomationUtilE2ETest', 'GetUniqueAncestors', function() {
this.runWithLoadedTree(this.basicDoc, function(root) {
this.runWithAutomation(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.runWithLoadedTree(this.basicDoc, function(root) {
this.runWithAutomation(this.basicDoc, function(root) {
var left = root, right = root;
// Same node.
......
......@@ -16,6 +16,7 @@ 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;
......@@ -35,6 +36,14 @@ 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
......@@ -73,29 +82,43 @@ Background = function() {
// Register listeners for ...
// Desktop.
chrome.automation.getDesktop(this.onGotDesktop);
chrome.automation.getDesktop(this.onGotTree);
// Tabs.
chrome.tabs.onUpdated.addListener(this.onTabUpdated);
};
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} desktop
* @param {chrome.automation.AutomationNode} root
*/
onGotDesktop: function(desktop) {
onGotTree: function(root) {
// Register all automation event listeners.
for (var eventType in this.listeners_)
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});
}
root.addEventListener(eventType, this.listeners_[eventType], true);
if (root.attributes.docLoaded) {
this.onLoadComplete(
{target: root, type: chrome.automation.EventType.loadComplete});
}
},
......@@ -247,28 +270,14 @@ 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;
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,
var node = AutomationUtil.findNodePost(evt.target,
Dir.FORWARD,
AutomationPredicate.leaf);
if (node)
this.currentRange_ = cursors.Range.fromNode(node);
......@@ -353,11 +362,21 @@ Background.prototype = {
if (opt_options.next) {
if (!chrome.commands.onCommand.hasListener(this.onGotCommand))
chrome.commands.onCommand.addListener(this.onGotCommand);
this.active_ = true;
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,17 +92,22 @@ TEST_F('BackgroundTest', 'DesktopFocus', function() {
/** Tests feedback once a page loads. */
TEST_F('BackgroundTest', 'InitialFeedback', function() {
cvox.ChromeVox.tts.expectSpeech('start', testDone);
cvox.ChromeVox.tts.expectSpeech('start', function() {
global.backgroundObj.onGotCommand('nextLine');
}, true);
cvox.ChromeVox.tts.expectSpeech('end', testDone, true);
this.runWithTab(function() {/*!
this.runWithDocument(function() {/*!
<p>start
<p>end
*/});
*/},
function() {});
});
/** Tests consistency of navigating forward and backward. */
TEST_F('BackgroundTest', 'ForwardBackwardNavigation', function() {
this.runWithLoadedTree(this.linksAndHeadingsDoc, function() {
cvox.ChromeVox.tts.expectSpeech('start', null, true);
this.runWithDocument(this.linksAndHeadingsDoc, function() {
var doCmd = this.doCmd.bind(this);
var expectAfter =
cvox.ChromeVox.tts.expectSpeechAfter.bind(cvox.ChromeVox.tts);
......@@ -132,7 +137,8 @@ TEST_F('BackgroundTest', 'ForwardBackwardNavigation', function() {
});
TEST_F('BackgroundTest', 'CaretNavigation', function() {
this.runWithLoadedTree(this.linksAndHeadingsDoc, function() {
cvox.ChromeVox.tts.expectSpeech('start', null, true);
this.runWithDocument(this.linksAndHeadingsDoc, function() {
var doCmd = this.doCmd.bind(this);
var expectAfter =
cvox.ChromeVox.tts.expectSpeechAfter.bind(cvox.ChromeVox.tts);
......@@ -162,7 +168,7 @@ TEST_F('BackgroundTest', 'CaretNavigation', function() {
// Flaky: http://crbug.com/451362
TEST_F('BackgroundTest', 'DISABLED_SelectSingleBasic', function() {
this.runWithLoadedTree(this.formsDoc, function(tabId) {
this.runWithDocument(this.formsDoc, function(tabId) {
var sendDownToSelect =
this.sendKeyToElement.bind(this, tabId, 'Down', '#fruitSelect');
var expect = cvox.ChromeVox.tts.expectSpeech.bind(cvox.ChromeVox.tts);
......@@ -174,7 +180,8 @@ TEST_F('BackgroundTest', 'DISABLED_SelectSingleBasic', function() {
});
TEST_F('BackgroundTest', 'ContinuousRead', function() {
this.runWithLoadedTree(this.linksAndHeadingsDoc, function() {
cvox.ChromeVox.tts.expectSpeech('start', null, true);
this.runWithDocument(this.linksAndHeadingsDoc, function() {
var doCmd = this.doCmd.bind(this);
var expect =
cvox.ChromeVox.tts.expectSpeechAfter.bind(cvox.ChromeVox.tts);
......
......@@ -70,7 +70,6 @@ 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());
}
......@@ -94,25 +93,27 @@ CursorsTest.prototype = {
* @param {string=} opt_testType Either CURSOR or RANGE.
*/
runCursorMovesOnDocument: function(doc, moves, opt_testType) {
this.runWithLoadedTree(doc,
function(root) {
var start = null;
this.runWithDocument(doc,
function() {
chrome.automation.getTree(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);
this.cursorMoveAndAssert(cursor, moves);
testDone();
} else if (opt_testType == this.RANGE) {
var range = new cursors.Range(cursor, cursor);
this.rangeMoveAndAssert(range, moves);
testDone();
}
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));
}.bind(this));
},
......
......@@ -13,8 +13,6 @@ 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.runWithLoadedTree('<a href="#">Click here</a>',
this.runWithAutomation('<a href="#">Click here</a>',
function(root) {
var link = root.firstChild.firstChild;
var range = cursors.Range.fromNode(link);
......
......@@ -58,26 +58,11 @@ ChromeVoxE2ETest.prototype = {
},
/**
* Launch a new tab, wait until tab status complete, then run callback.
* Run a test with the specified HTML snippet loaded.
* @param {function() : void} doc Snippet wrapped inside of a function.
* @param {function()} callback Called once the document is ready.
*/
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) {
runWithDocument: function(doc, callback) {
var docString = TestUtils.extractHtmlFromCommentEncodedString(doc);
var url = 'data:text/html,<!doctype html>' +
docString +
......@@ -86,7 +71,13 @@ ChromeVoxE2ETest.prototype = {
active: true,
url: url
};
chrome.tabs.create(createParams, opt_callback);
chrome.tabs.create(createParams, function(tab) {
chrome.tabs.onUpdated.addListener(function(tabId, changeInfo) {
if (tabId == tab.id && changeInfo.status == 'complete') {
callback(tabId);
}
});
});
},
/**
......
......@@ -29,20 +29,11 @@ ChromeVoxNextE2ETest.prototype = {
GEN('#include "base/command_line.h"');
},
/**
* 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);
runWithAutomation: function(doc, callback) {
this.runWithDocument(doc, function() {
chrome.automation.getTree(function(root) {
callback(root);
}.bind(this));
}.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.childTreeID !== nodeData.intAttributes.childTreeId)
if (nodeImpl.pendingChildFrame === undefined)
nodeImpl.pendingChildFrame = true;
if (nodeImpl.pendingChildFrame) {
......
......@@ -17,7 +17,7 @@ class ExtensionJSBrowserTest : public JavaScriptBrowserTest {
public:
ExtensionJSBrowserTest();
~ExtensionJSBrowserTest() override;
virtual ~ExtensionJSBrowserTest();
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();
~ExtensionLoadWaiterOneShot() override;
virtual ~ExtensionLoadWaiterOneShot();
// 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.
void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
virtual 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,11 +76,10 @@ void AXAuraObjCache::Remove(int32 id) {
delete obj;
}
AXAuraObjCache::AXAuraObjCache() : current_id_(1), is_destroying_(false) {
AXAuraObjCache::AXAuraObjCache() : current_id_(1) {
}
AXAuraObjCache::~AXAuraObjCache() {
is_destroying_ = true;
STLDeleteContainerPairSecondPointers(cache_.begin(), cache_.end());
cache_.clear();
}
......
......@@ -52,9 +52,6 @@ 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>;
......@@ -78,9 +75,6 @@ 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,10 +17,8 @@ AXWidgetObjWrapper::AXWidgetObjWrapper(Widget* widget) : widget_(widget) {
}
AXWidgetObjWrapper::~AXWidgetObjWrapper() {
if (!AXAuraObjCache::GetInstance()->is_destroying()) {
widget_->RemoveObserver(this);
widget_->RemoveRemovalsObserver(this);
}
widget_->RemoveObserver(this);
widget_->RemoveRemovalsObserver(this);
widget_ = NULL;
}
......
......@@ -78,7 +78,7 @@ void WebView::SetWebContents(content::WebContents* replacement) {
DCHECK(!is_embedding_fullscreen_widget_);
}
AttachWebContents();
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
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) {
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
void WebView::RenderProcessHostDestroyed(content::RenderProcessHost* host) {
......@@ -293,11 +293,11 @@ bool WebView::EmbedsFullscreenWidget() const {
// WebView, content::WebContentsObserver implementation:
void WebView::RenderViewReady() {
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
void WebView::RenderViewDeleted(content::RenderViewHost* render_view_host) {
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
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();
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
void WebView::WebContentsDestroyed() {
......@@ -313,7 +313,7 @@ void WebView::WebContentsDestroyed() {
observing_render_process_host_->RemoveObserver(this);
observing_render_process_host_ = nullptr;
}
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
void WebView::DidShowFullscreenWidget(int routing_id) {
......@@ -332,11 +332,11 @@ void WebView::DidToggleFullscreenModeForTab(bool entered_fullscreen) {
}
void WebView::DidAttachInterstitialPage() {
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
void WebView::DidDetachInterstitialPage() {
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
////////////////////////////////////////////////////////////////////////////////
......@@ -398,19 +398,14 @@ void WebView::ReattachForFullscreenChange(bool enter_fullscreen) {
// the same. So, do not change attachment.
OnBoundsChanged(bounds());
}
NotifyMaybeTextInputClientAndAccessibilityChanged();
NotifyMaybeTextInputClientChanged();
}
void WebView::NotifyMaybeTextInputClientAndAccessibilityChanged() {
void WebView::NotifyMaybeTextInputClientChanged() {
// 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 NotifyMaybeTextInputClientAndAccessibilityChanged();
void NotifyMaybeTextInputClientChanged();
// 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