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

ui/views/accessibility/native_view_accessibility_mac.mm

 // Copyright 2017 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 "ui/views/accessibility/native_view_accessibility_mac.h"
 
 #include <memory>
 
 #include "base/memory/ptr_util.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 namespace views {
 
 // static
 std::unique_ptr<NativeViewAccessibility> NativeViewAccessibility::Create(
     View* view) {
   return base::MakeUnique<NativeViewAccessibilityMac>(view);
 }
 
 NativeViewAccessibilityMac::NativeViewAccessibilityMac(View* view)
     : NativeViewAccessibilityBase(view) {}
 
-gfx::NativeViewAccessible NativeViewAccessibilityMac::GetParent() {
-  if (view_->parent())
-    return view_->parent()->GetNativeViewAccessible();
-
+gfx::NativeViewAccessible
+NativeViewAccessibilityMac::GetNativeViewAccessibleForWidget() {
   if (view_->GetWidget())

[tapted, 2017/04/03 05:50:36]

comment here why |parent_widget_| doesn't work for Mac?

(e.g. is it a bug that we don't set a parent_widget_ on mac?)

[Patti Lor, 2017/04/04 02:46:29]

I'm not sure why we are preferring to use the NativeView here over the
NativeViewAccessible of the Widget's RootView. I think this part of the code was
added before the |parent_widget_| NVA code in the current
NVABase::GetNVAForWidget() - see https://crrev.com/420653003, so maybe it's
actually just not needed any more. (Like, will there ever be a situation where
|view_| is attached to a Widget, but its Widget doesn't have a RootView? I don't
think that should happen, but you know more than me.)

I think the only reason why we don't set |parent_widget_| on Mac is because we
never happen to hit the part of the code in PopulateChildWidgetVector() that
actually calls SetParentWidget() (I messed around a bit to try and find a
situation where this might happen on Mac, including with MacViews browser, but
couldn't really). But PopulateChildWidgetVector() doesn't actually seem to need
parent_widget_ itself, I think maybe it just assumes that whatever calls it
might need to access |parent_widget_| later (probably by calling GetParent()).

WDYT of deleting this override and replacing the getter for parent_widget_ that
will instead retrieve it on the fly? We might also have to DCHECK() in
GetParent() that |view_| is a RootView when we try accessing |parent_widget_|
(since PopulateChildWidgetVector() only sets |parent_widget_| if that's the
case).

[tapted, 2017/04/04 08:13:13]

I think |parent_widget_| came about because in aura the NVA can teleport between
widgets, and they need to set up WidgetObservers correctly.. I need to look at
this some more. (Maybe dmazzoni has some ideas..)

[Patti Lor, 2017/04/11 05:40:45]

O, ok. I think that's still OK though right? If we get |parent_widget_| on the
fly instead of keeping a reference then we don't really need to set up
WidgetObservers for it. According to codesearch there is nowhere else in the
code that tries to use the getter we've got on NVABase, and if I delete it the
only compile error is on PopulateChildWidgetVector(). But yeah, I'll link this
thread on asking dmazzoni to look further.

     return view_->GetWidget()->GetNativeView();
-
-  if (parent_widget_)
-    return parent_widget_->GetRootView()->GetNativeViewAccessible();
-
-  return nullptr;
+  return NativeViewAccessibilityBase::GetNativeViewAccessibleForWidget();
 }
 
 }  // namespace views