a11y: Exclude children of nested keyboard accessible controls from a11y tree.

ui/views/view.cc

Index: ui/views/view.cc
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 48f3fbbe8f5bd85201cfcfdd559426442cc25ef9..ce17c91a763fc28c76e12053777149c5084ef12a 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -144,8 +144,7 @@ View::View()
       previous_focusable_view_(NULL),
       focus_behavior_(FocusBehavior::NEVER),
       context_menu_controller_(NULL),
-      drag_controller_(NULL),
-      native_view_accessibility_(NULL) {
+      drag_controller_(NULL) {
   SetTargetHandler(this);
 }
 
@@ -163,11 +162,6 @@ View::~View() {
         delete child;
     }
   }
-
-  // Release ownership of the native accessibility object, but it's
-  // reference-counted on some platforms, so it may not be deleted right away.
-  if (native_view_accessibility_)
-    native_view_accessibility_->Destroy();
 }
 
 // Tree operations -------------------------------------------------------------
@@ -1393,11 +1387,13 @@ bool View::HandleAccessibleAction(const ui::AXActionData& action_data) {
 }
 
 gfx::NativeViewAccessible View::GetNativeViewAccessible() {
-  if (!native_view_accessibility_)
-    native_view_accessibility_ = NativeViewAccessibility::Create(this);
+  if (!native_view_accessibility_) {
+    native_view_accessibility_.reset(
+        NativeViewAccessibility::GetOrCreate(this));

[tapted, 2017/01/11 18:27:40]

it does seem a bit spooky that a call to GetOrCreate not done from here could
end up putting a View* into map that could be destroyed without removing that
map entry. But! I think I have a solution: `GetOrCreate` just needs to call
view->GetNativeViewAccessible(). Of course, that will be infinite recursion
right now. So here I think it should be:

native_view_accessiblity_ = base::WrapUnique(NVA::CreateForView(this));

and instead of GetOrCreate(view), something like

NVA* GetForView(View* view) {
  view->GetNativeViewAccesible();  // Ensure a mapping exists.
  NVA* entry = GetNativeViewAccessibilityMap()[view];
  DCHECK(entry);
  return entry;
}


I think then we can also check that the call to nvamap.erase() always erases
exactly 1 object too.

[Patti Lor, 2017/02/21 03:29:17]

So I now have 

public:
 - static NVA::CreateForView() - should only be called by View
 - static NVA::GetForView() - should be called by everyone else

Should we enforce this by making CreateForView protected also and friend View?
Not sure when it's OK to do that.

+  }
   if (native_view_accessibility_)
     return native_view_accessibility_->GetNativeObject();
-  return NULL;
+  return nullptr;
 }
 
 void View::NotifyAccessibilityEvent(
@@ -1407,8 +1403,10 @@ void View::NotifyAccessibilityEvent(
     ViewsDelegate::GetInstance()->NotifyAccessibilityEvent(this, event_type);
 
   if (send_native_event && GetWidget()) {
-    if (!native_view_accessibility_)
-      native_view_accessibility_ = NativeViewAccessibility::Create(this);
+    if (!native_view_accessibility_) {
+      native_view_accessibility_.reset(

[tapted, 2017/01/11 18:27:40]

= base::WrapUnique(..)

Or, better, NVA::CreateForView might be able to return a std::unique_ptr and use
WrapUnique itself

[Patti Lor, 2017/02/21 03:29:17]

Done.

+          NativeViewAccessibility::GetOrCreate(this));
+    }
     if (native_view_accessibility_)
       native_view_accessibility_->NotifyAccessibilityEvent(event_type);
   }