Suppress accessibility events when user is navigating away.

content/browser/web_contents/web_contents_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 2484 matching lines @@
 void WebContentsImpl::DidFailProvisionalLoadWithError(
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidFailProvisionalLoadWithError_Params& params) {
   GURL validated_url(params.url);
   FOR_EACH_OBSERVER(WebContentsObserver,
                     observers_,
                     DidFailProvisionalLoad(render_frame_host,
                                            validated_url,
                                            params.error_code,
                                            params.error_description));
+
+    BrowserAccessibilityManager* manager =

[nasko, 2015/02/03 17:28:05]

nit: wrong indent.

[dmazzoni, 2015/02/04 20:56:06]

Done.

+      render_frame_host->browser_accessibility_manager();
+  if (manager)
+    manager->NavigationFailed();
 }
 
 void WebContentsImpl::DidFailLoadWithError(
     RenderFrameHostImpl* render_frame_host,
     const GURL& url,
     int error_code,
     const base::string16& error_description) {
   FOR_EACH_OBSERVER(
       WebContentsObserver,
       observers_,
       DidFailLoad(render_frame_host, url, error_code, error_description));
 }
 
 void WebContentsImpl::NotifyChangedNavigationState(
     InvalidateTypes changed_flags) {
   NotifyNavigationStateChanged(changed_flags);
 }
 
 void WebContentsImpl::AboutToNavigateRenderFrame(

[dmazzoni, 2015/01/21 18:59:03]

This function is called at the right time, but (1) render_frame_host is the new
RFH, not the old one still visible, and (2) it doesn't have reload_type, which
I'd really like to pass to accessibility.

[nasko, 2015/02/03 17:28:05]

It doesn't need to be the new RFH in all cases. If navigation is to the same
site, it will be the same one I think.

OnDidStartLoading is much better signal to use, as it will catch both
renderer-initiated navigations as well as browser-initiated navigations. It
doesn't have the reload flag, but that one is also browser-initiated navigations
specific. 

[dmazzoni, 2015/02/03 17:40:54]

So what should I use if I want to catch both browser-initiated and
renderer-initiated, and the reload flag? Can we extend one of the notifications
to provide all that?

[nasko, 2015/02/03 18:05:19]

Let's step back a second and clarify why is "reload" flag needed for fixing this
issue? What will the flag help us know?

[dmazzoni, 2015/02/03 18:20:27]

As soon as a navigation starts we can fire a notification telling screen readers
exactly what's happening: either "page busy" (meaning navigation) or "page
reloading". A prerequisite to firing those events is that we need to *not* fire
any events on the "old" page, which is what this changelist fixes. Once we stop
firing events on the old page, I can add the code to fire the busy/reload events
along with tests for that.

       RenderFrameHostImpl* render_frame_host) {
   // Notify observers that we will navigate in this RenderFrame.
   FOR_EACH_OBSERVER(
       WebContentsObserver,
       observers_,
       AboutToNavigateRenderFrame(render_frame_host));
 }
 
 void WebContentsImpl::DidStartNavigationToPendingEntry(
       RenderFrameHostImpl* render_frame_host,
       const GURL& url,
       NavigationController::ReloadType reload_type) {
   // Notify observers about navigation.
   FOR_EACH_OBSERVER(
       WebContentsObserver,
       observers_,
       DidStartNavigationToPendingEntry(url, reload_type));
+
+  // Notify accessibility that the user is navigating away from the
+  // current RenderFrameHost.
+  FrameTreeNode* ftn = render_frame_host->frame_tree_node();

[dmazzoni, 2015/01/21 18:59:04]

This is the only function that gets called early enough, and has |reload_type| -
but |render_frame_host| is the new RFH, not the "current" one. It'd be nice to
have a notification on the current RFH that it's going to be replaced with
another one.

[nasko, 2015/02/03 17:28:05]

Architecturally, we can't notify before commit time what the new RFH is going to
be. Commit time is too late for you, based on my understanding, otherwise there
is such thing - RenderFrameHostChanged.

[dmazzoni, 2015/02/03 17:40:54]

What I'm observing is that for a browser-initiated navigation,
|render_frame_host| is the *new* RFH. I had to add this code to get the current
frame host from the frame tree node in order to get the *old* RFH, because
that's the one that's currently in the accessibility tree and we want to notify
it that it should enter the "busy" state.

[nasko, 2015/02/03 18:05:18]

What you are observing is correct for this very specific bug you are trying to
fix. If you go to google.com and then type another google.com, you shouldn't be
getting a new RFH different from the current one.

[dmazzoni, 2015/02/03 18:20:27]

That's true.

The point is that I need a notification telling me that a navigation *may* be
happening, and I need to know the current/previous RFH at that time. If the
navigation leads us to the same RFH or a different RFH, or if it fails and we
end up with the original RFH, either way we'll catch that in either
DidCommitProvisionalLoad or DidFailProvisionalLoadWithError.

+  BrowserAccessibilityManager* manager =
+      ftn->current_frame_host()->browser_accessibility_manager();
+  if (manager) {
+    bool is_reload = reload_type != NavigationController::NO_RELOAD;
+    manager->UserIsNavigatingAway(is_reload);
+  }
 }
 
 void WebContentsImpl::RequestOpenURL(RenderFrameHostImpl* render_frame_host,
                                      const OpenURLParams& params) {
   // OpenURL can blow away the source RFH. Use the process/frame routing ID as a
   // weak pointer of sorts.
   const int32_t process_id = render_frame_host->GetProcess()->GetID();
   const int32_t frame_id = render_frame_host->GetRoutingID();
 
   WebContents* new_contents = OpenURL(params);
@@ skipping 18 matching lines @@
 
 void WebContentsImpl::DidCommitProvisionalLoad(
     RenderFrameHostImpl* render_frame_host,
     const GURL& url,
     ui::PageTransition transition_type) {
   // Notify observers about the commit of the provisional load.
   FOR_EACH_OBSERVER(WebContentsObserver,
                     observers_,
                     DidCommitProvisionalLoadForFrame(
                         render_frame_host, url, transition_type));
+
+  BrowserAccessibilityManager* manager =
+      render_frame_host->browser_accessibility_manager();
+  if (manager)
+    manager->NavigationSucceeded();
 }
 
 void WebContentsImpl::DidNavigateMainFramePreCommit(
     bool navigation_is_within_page) {
   // Ensure fullscreen mode is exited before committing the navigation to a
   // different page.  The next page will not start out assuming it is in
   // fullscreen mode.
   if (navigation_is_within_page) {
     // No page change?  Then, the renderer and browser can remain in fullscreen.
     return;
@@ skipping 1818 matching lines @@
   node->render_manager()->ResumeResponseDeferredAtStart();
 }
 
 void WebContentsImpl::SetForceDisableOverscrollContent(bool force_disable) {
   force_disable_overscroll_content_ = force_disable;
   if (view_)
     view_->SetOverscrollControllerEnabled(CanOverscrollContent());
 }
 
 }  // namespace content