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

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index f70b5808da311d3ae0d0c68be53834ef2c510cc6..0d2067bb1ea5c27462a9370b67019317f98c8fc4 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -452,25 +452,33 @@ 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;
@@ -1477,6 +1485,21 @@ void RenderFrameHostImpl::UpdateGuestFrameAccessibility(
   }
 }
 
+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));
 }