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

ui/views/view.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.
 
 #define _USE_MATH_DEFINES // For VC++ to get M_PI. This has to be first.
 
 #include "ui/views/view.h"
 
 #include <algorithm>
 #include <cmath>
@@ skipping 126 matching lines @@
       needs_layout_(true),
       snap_layer_to_pixel_boundary_(false),
       flip_canvas_on_paint_for_rtl_ui_(false),
       paint_to_layer_(false),
       accelerator_focus_manager_(NULL),
       registered_accelerator_count_(0),
       next_focusable_view_(NULL),
       previous_focusable_view_(NULL),
       focus_behavior_(FocusBehavior::NEVER),
       context_menu_controller_(NULL),
-      drag_controller_(NULL),
-      native_view_accessibility_(NULL) {
+      drag_controller_(NULL) {
   SetTargetHandler(this);
 }
 
 View::~View() {
   if (parent_)
     parent_->RemoveChildView(this);
 
   ViewStorage::GetInstance()->ViewRemoved(this);
 
   {
     internal::ScopedChildrenLock lock(this);
     for (auto* child : children_) {
       child->parent_ = NULL;
       if (!child->owned_by_client_)
         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 -------------------------------------------------------------
 
 const Widget* View::GetWidget() const {
   // The root view holds a reference to this view hierarchy's Widget.
   return parent_ ? parent_->GetWidget() : NULL;
 }
 
 Widget* View::GetWidget() {
@@ skipping 1205 matching lines @@
           abs(delta.y()) > GetVerticalDragThreshold());
 }
 
 // Accessibility----------------------------------------------------------------
 
 bool View::HandleAccessibleAction(const ui::AXActionData& action_data) {
   return false;
 }
 
 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(
     ui::AXEvent event_type,
     bool send_native_event) {
   if (ViewsDelegate::GetInstance())
     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);
   }
 }
 
 // Scrolling -------------------------------------------------------------------
 
 void View::ScrollRectToVisible(const gfx::Rect& rect) {
   // We must take RTL UI mirroring into account when adjusting the position of
   // the region.
@@ skipping 1124 matching lines @@
   // Message the RootView to do the drag and drop. That way if we're removed
   // the RootView can detect it and avoid calling us back.
   gfx::Point widget_location(event.location());
   ConvertPointToWidget(this, &widget_location);
   widget->RunShellDrag(this, data, widget_location, drag_operations, source);
   // WARNING: we may have been deleted.
   return true;
 }
 
 }  // namespace views