Looking a bit further into the class, I don't think it necessary to define another class for subframe. They need to access Java layer through the main frame one too, and destructor for subframe BAM reaching Java layer can be avoided simply by managing the one for the main frame separately from the rest.

This also affects other refactoring in progress. https://chromium-review.googlesource.com/c/517683/ as hover event may be intercepted by BAM.

Quoted from the reviewer's comment for reference's sake:

> So depends on whether accessibility should be entirely internal to
> content, or it can be exposed to content embedders. I don't really know 
> which way to go, probably can ask owners or look at what other platforms do.

> If it's internal, can just have the web contents view intercept the event, 
> and bubble it up to java before sending it further down native? It's one 
> more jni hope than necessary, but not a big deal.

> If it's public, then embedder should do the check before forwarding to view tree?

My preferred choice is the former, since all the embedders will required the same flow. I'm thinking of handling any embedder-specific differences(focus on page load, for instance) through ViewAndroidDelegate.  

Added the owner dmazzoni@ (sorry should have added you sooner). Basically this is decouple BAM from ContentViewCore as much as possible as a part of breaking up CVC which has been bloated in years. There will be no change in terms of functionalities. Any feedback/suggestion would be welcome.

All of those goals sound great to me.

As background, when the code was first written there was only one (C++) BrowserAccessibilityManager for a whole page. Now there's one per frame, but on the Java side we still only need one class. Perhaps the Java class should be renamed to make it more clear, it should be something like WebContentsAccessibility or ContentViewAccessibility. For now, I agree that only the "main" BrowserAccessibilityManager should access it, but over time maybe we could refactor this on all platforms so that there's a class to manage the connection to the native view, and a separate class to manage the specifics of each frame.

Short-term, let's keep the lifetime of BrowserAccessibilityManagerAndroid (C++) the same, but let's possibly rename the Java class, and fix its ownership and lifetime issues.

As to the second question about the hover events, I think accessibility should be internal to content. Embedders can always access it via accessibility APIs. Depending on the embedder to get hover events right sounds like a bad idea, it means a lot of apps will accidentally break accessibility. This would hurt users who depend on accessibility because their numbers are small so they wouldn't get a lot of attention.

Let me know how I can help!

Thanks for the feedback dmazzoni@. I'm wondering how I can make sure I don't break anything along the way. Maybe adding some tests would help. I cannot find relevant tests on Java layer. Could you help how 

(Oops.. clicked the button too soon) Could you give some ideas on what to put together to test the feature? Unfortunately I haven't used it so my knowledge is almost zero in that part.

I found AccessibilitySnapshotTest under content/browser/wecontents. Will use it as a reference if I need to add more tests.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea

commit c6c1d15d250e1c51893cf2abc8a242a8bcf801ea
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Tue Jun 13 10:03:53 2017

Refactor BrowserAccessibilityManager for Android

Refactored BrowserAccessibilityManager on Android. This CL introduces a new
class WebContentsAccessibility that manages the connection between native and
Java layer. The class also updates webcontents - renderer frame link by using
RenderWidgetHostConnector. It has the lifetime of webcontents, is created
lazily.  This helps take CVC out of the picture and makes the overall
initialization process of BAMA more straightforward - i.e. WCAX is created
first and then connected BAMA later upon request by RenderFrameHostImpl.
It also eliminates RenderWidgetHostViewAndroid's dependency on CVC in regard
to the creation of BAMA.

Drawings to illustrate the changes are here https://goo.gl/B4arVS

Bug:  727210 
Change-Id: I41f3452f56f3f7ac8d24710cd54f35850c4eef1e
Reviewed-on: https://chromium-review.googlesource.com/520868
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Paul Miller <paulmiller@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478954}
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopupBridge.java
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/BUILD.gn
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/accessibility/browser_accessibility_android.cc
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/accessibility/browser_accessibility_manager_android.cc
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/accessibility/browser_accessibility_manager_android.h
[add] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/accessibility/web_contents_accessibility.h
[add] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/accessibility/web_contents_accessibility_android.cc
[add] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/accessibility/web_contents_accessibility_android.h
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/android/browser_jni_registrar.cc
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/android/render_widget_host_connector.cc
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/public/android/BUILD.gn
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[rename] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/public/android/java/src/org/chromium/content/browser/accessibility/KitKatWebContentsAccessibility.java
[rename] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/public/android/java/src/org/chromium/content/browser/accessibility/LollipopWebContentsAccessibility.java
[rename] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/public/android/java/src/org/chromium/content/browser/accessibility/WebContentsAccessibility.java
[modify] https://crrev.com/c6c1d15d250e1c51893cf2abc8a242a8bcf801ea/content/public/android/java/src/org/chromium/content_public/browser/AccessibilitySnapshotNode.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14daf81d4cb56c32e83ebeed5a2aed37e36fa6b4

commit 14daf81d4cb56c32e83ebeed5a2aed37e36fa6b4
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Thu Jun 15 04:01:55 2017

Remove null check against native WebContentsAccessibility

Now that WebContentsAccessibility's lifetime is the same as that
of WebContents, the native pointer is guranteed to be non-null
all through the time while a java WebContentsAccessibility instance
is alive and in use. This CL removes the null checks against the native
pointer which became not necessary any more.

Also turned some protected methods to private.

Tested manually with TalkBack/Select-to-speak on.

BUG= 727210 

Change-Id: Id946e92a93a1f31b8af4db92126a91e9d0634df8
Reviewed-on: https://chromium-review.googlesource.com/535417
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479595}
[modify] https://crrev.com/14daf81d4cb56c32e83ebeed5a2aed37e36fa6b4/content/public/android/java/src/org/chromium/content/browser/accessibility/WebContentsAccessibility.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ee829ff64a71d28956515e2276b1a5679e726dd

commit 8ee829ff64a71d28956515e2276b1a5679e726dd
Author: Brian White <bcwhite@chromium.org>
Date: Wed Sep 27 23:51:31 2017

Remove accessibility from manager during destruction.

If "accessibility" is enabled then a pointer to the WebContents-
AccessibilityAndroid object is held by an BrowserAccessibility-
ManagerAndroid object leaving it dangling if the former is
destructed.  During destruction of the WCAA object, remove any
reference to itself from the BAMA object.

Bug: 736675,  727210 
Change-Id: I67050e7708abbdfe396a0571bc515a5bda88dcbe
Reviewed-on: https://chromium-review.googlesource.com/688035
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504807}
[modify] https://crrev.com/8ee829ff64a71d28956515e2276b1a5679e726dd/content/browser/accessibility/web_contents_accessibility_android.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e961d1300741f1b2f33cd6dd015e5fdd0c8bf110

commit e961d1300741f1b2f33cd6dd015e5fdd0c8bf110
Author: Brian White <bcwhite@chromium.org>
Date: Mon Oct 02 21:23:34 2017

Remove accessibility from manager during destruction.

If "accessibility" is enabled then a pointer to the WebContents-
AccessibilityAndroid object is held by an BrowserAccessibility-
ManagerAndroid object leaving it dangling if the former is
destructed.  During destruction of the WCAA object, remove any
reference to itself from the BAMA object.

TBR=bcwhite@chromium.org

(cherry picked from commit 8ee829ff64a71d28956515e2276b1a5679e726dd)

Bug: 736675,  727210 
Change-Id: I67050e7708abbdfe396a0571bc515a5bda88dcbe
Reviewed-on: https://chromium-review.googlesource.com/688035
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#504807}
Reviewed-on: https://chromium-review.googlesource.com/696221
Reviewed-by: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#551}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/e961d1300741f1b2f33cd6dd015e5fdd0c8bf110/content/browser/accessibility/web_contents_accessibility_android.cc

With r532739, refactoring WebContentsAccessibility on Android I had in mind was completed. Closing this now.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f0a3c480479816dc42eaac104498c442444a9f5

commit 9f0a3c480479816dc42eaac104498c442444a9f5
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Thu Mar 22 01:27:46 2018

Android: Factor out SystemCaptionBridge into accessibility/

This CL moves Android system captioning bridge management out
of ContentViewCoreImpl to accessibility/captioning, and lets
WebContentsAccessibility keeps the reference to it. No
behavioral change was made.

Bug:  598880 ,  727210 

Change-Id: I811970ea3654fe8e976ae4000c11e077b14b1f8c
Reviewed-on: https://chromium-review.googlesource.com/970045
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544936}
[modify] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/browser/BUILD.gn
[add] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/browser/accessibility/captioning_controller.cc
[add] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/browser/accessibility/captioning_controller.h
[modify] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/browser/android/content_view_core.cc
[modify] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/browser/android/content_view_core.h
[modify] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/public/android/BUILD.gn
[modify] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java
[modify] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/public/android/java/src/org/chromium/content/browser/accessibility/WebContentsAccessibilityImpl.java
[add] https://crrev.com/9f0a3c480479816dc42eaac104498c442444a9f5/content/public/android/java/src/org/chromium/content/browser/accessibility/captioning/CaptioningController.java