Commit cdd843da authored by Ian Prest's avatar Ian Prest Committed by Commit Bot

Revert "Notify BrowserAccessibilityManager via WebContentsObserver"

This reverts commit 0885c956.

Reason for revert: Unfortunately, this change results in the
notification of page-load-complete happening much later than it
did previously.

Since a11y events aren't fired when |user_is_navigating_away_|, the
result is that some events that occur early in a page's lifetime are
not emitted.  This is most visible for programmatic 'focus' changes;
if they're not emitted, any connected AT doesn't know the focus has
changed.

Original change's description:
> Notify BrowserAccessibilityManager via WebContentsObserver
>
> This is the last of a series of CLs that remove manual notifications
> BrowserAccessibilityManager, now that it implements WebContentsObserver.
> This CL removes manual BAM::Navigation{Succeeded,Failed} notifications
> in favor of WCO::DidFinishNavigation, and BAM::UserIsNavigatingAway in
> favor of WCO::DidStartLoading.
>
> Bug: 981271
> Change-Id: Ieca6ce0b97f2a0267fd7f2582ab1e333deb11b95
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1938489
> Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Commit-Queue: Dominic Farolino <dom@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#720102}

TBR=falken@chromium.org,dmazzoni@chromium.org,dom@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 981271
Change-Id: I41d68985b964792083aa5b6628bb3bc0972c9790
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2024154Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarIan Prest <iapres@microsoft.com>
Commit-Queue: Ian Prest <iapres@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#735980}
parent fdde4b45
...@@ -342,14 +342,22 @@ void BrowserAccessibilityManager::OnWindowBlurred() { ...@@ -342,14 +342,22 @@ void BrowserAccessibilityManager::OnWindowBlurred() {
SetLastFocusedNode(nullptr); SetLastFocusedNode(nullptr);
} }
void BrowserAccessibilityManager::UserIsReloading() { void BrowserAccessibilityManager::UserIsNavigatingAway() {
user_is_navigating_away_ = true; user_is_navigating_away_ = true;
} }
void BrowserAccessibilityManager::DidStartLoading() { void BrowserAccessibilityManager::UserIsReloading() {
user_is_navigating_away_ = true; user_is_navigating_away_ = true;
} }
void BrowserAccessibilityManager::NavigationSucceeded() {
user_is_navigating_away_ = false;
}
void BrowserAccessibilityManager::NavigationFailed() {
user_is_navigating_away_ = false;
}
void BrowserAccessibilityManager::DidStopLoading() { void BrowserAccessibilityManager::DidStopLoading() {
user_is_navigating_away_ = false; user_is_navigating_away_ = false;
} }
......
...@@ -194,16 +194,14 @@ class CONTENT_EXPORT BrowserAccessibilityManager : public ui::AXTreeObserver, ...@@ -194,16 +194,14 @@ class CONTENT_EXPORT BrowserAccessibilityManager : public ui::AXTreeObserver,
// view lost focus. // view lost focus.
virtual void OnWindowBlurred(); virtual void OnWindowBlurred();
virtual void UserIsReloading();
// WebContentsObserver implementation.
// Notify the accessibility manager about page navigation. // Notify the accessibility manager about page navigation.
// BrowserAccessibilityManager used to be manually notified at the same time // TODO(domfarolino, dmazzoni): Implement WebContentsObserver methods that
// WebContentsObserver's DidStartLoading(), DidStopLoading(), and // correspond to the ones we provide today, so we can stop being manually
// DidFinishNavigation() methods were called. Since then, it was determined // notified of navigation events when they happen.
// BrowserAccessibilityManager does not need to distinguish between void UserIsNavigatingAway();
// DidFinishNavigation() and DidStopLoading(). virtual void UserIsReloading();
void DidStartLoading() override; void NavigationSucceeded();
void NavigationFailed();
void DidStopLoading() override; void DidStopLoading() override;
// Keep track of if this page is hidden by an interstitial, in which case // Keep track of if this page is hidden by an interstitial, in which case
......
...@@ -4488,6 +4488,20 @@ void WebContentsImpl::DidFinishNavigation(NavigationHandle* navigation_handle) { ...@@ -4488,6 +4488,20 @@ void WebContentsImpl::DidFinishNavigation(NavigationHandle* navigation_handle) {
display_cutout_host_impl_->DidFinishNavigation(navigation_handle); display_cutout_host_impl_->DidFinishNavigation(navigation_handle);
if (navigation_handle->HasCommitted()) { if (navigation_handle->HasCommitted()) {
// TODO(domfarolino, dmazzoni): Do this using WebContentsObserver. See
// https://crbug.com/981271.
BrowserAccessibilityManager* manager =
static_cast<RenderFrameHostImpl*>(
navigation_handle->GetRenderFrameHost())
->browser_accessibility_manager();
if (manager) {
if (navigation_handle->IsErrorPage()) {
manager->NavigationFailed();
} else {
manager->NavigationSucceeded();
}
}
if (navigation_handle->IsInMainFrame()) { if (navigation_handle->IsInMainFrame()) {
last_committed_source_id_including_same_document_ = last_committed_source_id_including_same_document_ =
ukm::ConvertToSourceId(navigation_handle->GetNavigationId(), ukm::ConvertToSourceId(navigation_handle->GetNavigationId(),
...@@ -5956,6 +5970,15 @@ void WebContentsImpl::DidStartLoading(FrameTreeNode* frame_tree_node, ...@@ -5956,6 +5970,15 @@ void WebContentsImpl::DidStartLoading(FrameTreeNode* frame_tree_node,
// Reset the focus state from DidStartNavigation to false if a new load starts // Reset the focus state from DidStartNavigation to false if a new load starts
// afterward, in case loading logic triggers a FocusLocationBarByDefault call. // afterward, in case loading logic triggers a FocusLocationBarByDefault call.
should_focus_location_bar_by_default_ = false; should_focus_location_bar_by_default_ = false;
// Notify accessibility that the user is navigating away from the
// current document.
// TODO(domfarolino, dmazzoni): Do this using WebContentsObserver. See
// https://crbug.com/981271.
BrowserAccessibilityManager* manager =
frame_tree_node->current_frame_host()->browser_accessibility_manager();
if (manager)
manager->UserIsNavigatingAway();
} }
void WebContentsImpl::DidStopLoading() { void WebContentsImpl::DidStopLoading() {
......
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