Make Windows accessibility event firing aware of guest / child frames.

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 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/frame_host/render_frame_host_impl.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
@@ skipping 434 matching lines @@
       render_view_host_->GetView());
   if (view)
     return view->AccessibilityGetNativeViewAccessible();
   return NULL;
 }
 
 BrowserAccessibilityManager* RenderFrameHostImpl::AccessibilityGetChildFrame(
     int accessibility_node_id) {
   RenderFrameHostImpl* child_frame =
       FrameAccessibility::GetInstance()->GetChild(this, accessibility_node_id);
-  if (!child_frame)
-    return NULL;
-
-  // Return NULL if this isn't an out-of-process iframe. Same-process iframes
-  // are already part of the accessibility tree.
-  if (child_frame->GetProcess()->GetID() == GetProcess()->GetID())
-    return NULL;
-
-  // As a sanity check, make sure the frame we're going to return belongs
-  // to the same BrowserContext.
-  if (GetSiteInstance()->GetBrowserContext() !=
-      child_frame->GetSiteInstance()->GetBrowserContext()) {
-    NOTREACHED();
+  if (!child_frame ||
+      !IsOutOfProcessFrameInSameBrowserContext(child_frame)) {
     return NULL;
   }
 
   return child_frame->GetOrCreateBrowserAccessibilityManager();
 }
 
+void RenderFrameHostImpl::AccessibilityGetAllChildFrames(
+    std::vector<BrowserAccessibilityManager*>* child_frames) {
+  std::vector<RenderFrameHostImpl*> child_frame_hosts;
+  FrameAccessibility::GetInstance()->GetAllChildFrames(
+      this, &child_frame_hosts);
+  for (size_t i = 0; i < child_frame_hosts.size(); ++i) {
+    RenderFrameHostImpl* child_frame_host = child_frame_hosts[i];
+    if (!child_frame_host ||
+        !IsOutOfProcessFrameInSameBrowserContext(child_frame_host)) {
+      continue;
+    }
+
+    BrowserAccessibilityManager* manager =
+        child_frame_host->GetOrCreateBrowserAccessibilityManager();
+    if (manager)
+      child_frames->push_back(manager);
+  }
+}
+
 BrowserAccessibility* RenderFrameHostImpl::AccessibilityGetParentFrame() {
   RenderFrameHostImpl* parent_frame = NULL;
   int parent_node_id = 0;
   if (!FrameAccessibility::GetInstance()->GetParent(
       this, &parent_frame, &parent_node_id)) {
     return NULL;
   }
 
   // As a sanity check, make sure the frame we're going to return belongs
   // to the same BrowserContext.
@@ skipping 986 matching lines @@
   for (const auto& iter : node_to_browser_plugin_instance_id_map) {
     // This is the id of the accessibility node that hosts a plugin.
     int32 node_id = iter.first;
     // The id of the browser plugin.
     int browser_plugin_instance_id = iter.second;
     FrameAccessibility::GetInstance()->AddGuestWebContents(
         this, node_id, browser_plugin_instance_id);
   }
 }
 
+bool RenderFrameHostImpl::IsOutOfProcessFrameInSameBrowserContext(

[nasko, 2015/01/15 21:44:38]

The name is a bit too long and confusing. What about IsSameSiteInstance() and
invert the meaning of the return value?

[dmazzoni, 2015/01/15 23:09:22]

Done.

+    RenderFrameHostImpl* child_frame) {

[nasko, 2015/01/15 21:44:38]

This method doesn't really check if child_frame is indeed child of the current
RFH.

[dmazzoni, 2015/01/15 23:09:23]

Is it better now that it's called IsSameSiteInstance, or do you think an
additional check is needed?

+  if (child_frame->GetProcess()->GetID() == GetProcess()->GetID())

[nasko, 2015/01/15 21:44:38]

This check doesn't seem semantically correct. We isolate iframes which are in
separate SiteInstance. The process isolation is an implementation detail of how
SiteInstances are isolated from each other, but they can end up in the same
process if we are above the process limit.

[dmazzoni, 2015/01/15 23:09:22]

Got it - so all that matters is if it's the same SiteInstance or not.

[nasko, 2015/01/16 00:20:33]

Yes, indeed.

+    return false;
+
+  // As a sanity check, make sure the frame belongs to the same BrowserContext.
+  if (GetSiteInstance()->GetBrowserContext() !=

[nasko, 2015/01/15 21:44:38]

If this ever fails, we have really really bad problems. If you want to keep
that, convert it to a CHECK.

[dmazzoni, 2015/01/15 23:09:22]

Done.

+      child_frame->GetSiteInstance()->GetBrowserContext()) {
+    NOTREACHED();
+    return false;
+  }
+
+  return true;
+}
+
 void RenderFrameHostImpl::SetAccessibilityMode(AccessibilityMode mode) {
   Send(new FrameMsg_SetAccessibilityMode(routing_id_, mode));
 }
 
 void RenderFrameHostImpl::SetAccessibilityCallbackForTesting(
     const base::Callback<void(ui::AXEvent, int)>& callback) {
   accessibility_testing_callback_ = callback;
 }
 
 const ui::AXTree* RenderFrameHostImpl::GetAXTreeForTesting() {
@@ skipping 120 matching lines @@
 void RenderFrameHostImpl::DidUseGeolocationPermission() {
   RenderFrameHost* top_frame = frame_tree_node()->frame_tree()->GetMainFrame();
   GetContentClient()->browser()->RegisterPermissionUsage(
       PERMISSION_GEOLOCATION,
       delegate_->GetAsWebContents(),
       GetLastCommittedURL().GetOrigin(),
       top_frame->GetLastCommittedURL().GetOrigin());
 }
 
 }  // namespace content