Views a11y: Implement AXPlatformNode::FromNativeViewAccessible on all platforms.

Views a11y: Implement AXPlatformNode::FromNativeViewAccessible() for Mac/Linux.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View is required. While this is
possible using AXPlatformNode::FromNativeViewAccessible() on Windows, it is not
implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac, which makes it possible to retrieve a NativeViewAccessibility instance for
any given View.

BUG=610589
Review-Url: https://codereview.chromium.org/2715543003
Cr-Commit-Position: refs/heads/master@{#461026}
Committed: https://chromium.googlesource.com/chromium/src/+/54bfa63a7eedc669fc481aa985d1f5979e695a1a

[Patti Lor, 2017-02-23 06:11:31]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 06:11:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 06:28:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 06:28:28]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Patti Lor, 2017-02-23 23:36:01]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 23:36:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 23:54:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 23:54:36]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Patti Lor, 2017-02-24 00:39:49]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 00:40:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 00:57:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 00:57:04]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[Patti Lor, 2017-02-24 02:03:26]

Patchset #2 (id:20001) has been deleted

[Patti Lor, 2017-02-24 02:06:38]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 02:07:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-02-24 02:29:25]

Patchset #2 (id:40001) has been deleted

[Patti Lor, 2017-02-24 03:13:21]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 03:13:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 04:11:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 04:11:54]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2017-02-24 04:43:47]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 04:44:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-02-24 05:04:00]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 05:04:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-02-24 05:11:58]

Patchset #3 (id:80001) has been deleted

[commit-bot: I haz the power, 2017-02-24 06:36:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 06:36:44]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2017-02-27 03:57:13]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 03:57:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-27 05:03:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-27 05:03:06]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2017-02-27 05:29:18]

patricialor@chromium.org changed reviewers:
+ tapted@chromium.org

[Patti Lor, 2017-02-27 05:29:19]

Hi Trent, PTAL - let me know if you'd like this split up more. Not sure if there
is a better name for this patch, too D:

[tapted, 2017-02-27 05:37:48]

https://codereview.chromium.org/2715543003/diff/140001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility.h (right):

https://codereview.chromium.org/2715543003/diff/140001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.h:80: static
NativeViewAccessibility* GetForView(View* view);
Hm - this is the controversial bit that I was hoping to isolate in the other
CL... I don't think the change in views::View to make native_view_accessibility_
a std::unique_ptr relies on GetForView() (and that unique_ptr change is a good
change, that I don't think will be controversial at all).

[Patti Lor, 2017-02-28 02:06:51]

Patchset #6 (id:160001) has been deleted

[Patti Lor, 2017-02-28 04:51:58]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 04:52:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-02-28 05:31:05]

Description was changed from

==========
Views a11y: Tie the NativeViewAccessibility lifetime to that of its View.

Currently, instances of NativeViewAccessibility depend on
NativeViewAccessibility::Destroy() being called in order to be released. By
converting the Views NativeViewAccessibility member to a unique_ptr, it will be
automatically destroyed with its corresponding View. To do this, implement
NativeViewAccessibility::CreateForView(), which should only be called by View,
and NativeViewAccessibility::GetForView(), which can be called by everything
else.

Also implement AXPlatformNode::FromNativeViewAccessible() for Linux and Mac,
which allows the correct NativeViewAccessibility instance to be retrieved from a
gfx::NativeViewAccessible. A follow up CL, https://crrev.com/2119413004,
requires the ability to do this in order to exclude unnecessary accessibility
elements from the accessibility tree.

BUG=610589
==========

to

==========
Views a11y: Implement AXPlatformNode::FromNativeViewAccessible on all platforms.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View or a gfx::NativeViewAccessible
is required. While AXPlatformNode::FromNativeViewAccessible() already does the
latter on Windows, it is not implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac with a fallback for ChromeOS. Then to retrieve a NativeViewAccessibility
instance from a View, implement NativeViewAccessibility::GetForView().

BUG=610589
==========

[Patti Lor, 2017-02-28 06:01:37]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 06:02:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-02-28 06:12:17]

Description was changed from

==========
Views a11y: Implement AXPlatformNode::FromNativeViewAccessible on all platforms.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View or a gfx::NativeViewAccessible
is required. While AXPlatformNode::FromNativeViewAccessible() already does the
latter on Windows, it is not implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac with a fallback for ChromeOS. Then to retrieve a NativeViewAccessibility
instance from a View, implement NativeViewAccessibility::GetForView().

BUG=610589
==========

to

==========
Views a11y: Implement AXPlatformNode::FromNativeViewAccessible on all platforms.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View or a gfx::NativeViewAccessible
is required. While AXPlatformNode::FromNativeViewAccessible() already does the
latter on Windows, it is not implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac with a fallback for ChromeOS. Then to retrieve a NativeViewAccessibility
instance from a View, implement NativeViewAccessibility::GetForView().

BUG=610589
==========

[commit-bot: I haz the power, 2017-02-28 07:33:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 07:33:56]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2017-02-28 23:32:32]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 23:34:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-02-28 23:57:07]

Patchset #7 (id:200001) has been deleted

[commit-bot: I haz the power, 2017-03-01 01:27:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-01 01:27:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Patti Lor, 2017-03-02 04:42:17]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 04:43:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 06:41:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 06:41:58]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2017-03-03 02:55:54]

Oops, forgot to send this out. Thanks Trent :)

https://codereview.chromium.org/2715543003/diff/140001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility.h (right):

https://codereview.chromium.org/2715543003/diff/140001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.h:80: static
NativeViewAccessibility* GetForView(View* view);
On 2017/02/27 05:37:48, tapted wrote:
> Hm - this is the controversial bit that I was hoping to isolate in the other
> CL... I don't think the change in views::View to make
native_view_accessibility_
> a std::unique_ptr relies on GetForView() (and that unique_ptr change is a good
> change, that I don't think will be controversial at all).

Done.

[tapted, 2017-03-06 01:43:10]

https://codereview.chromium.org/2715543003/diff/240001/testing/platform_test.h
File testing/platform_test.h (right):

https://codereview.chromium.org/2715543003/diff/240001/testing/platform_test....
testing/platform_test.h:12: typedef struct objc_object* id;
be sure to call out the reason for this in the CL description. i.e. that 
`void*` used to work when `id` was just a return type, but now we need it as a
method argument, and we can't have .cc files and .mm files seeing different
types, or they will declare an overload that doesn't exist.

https://codereview.chromium.org/2715543003/diff/240001/ui/accessibility/platf...
File ui/accessibility/platform/ax_platform_node_auralinux.cc (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/accessibility/platf...
ui/accessibility/platform/ax_platform_node_auralinux.cc:377: AX_EXPORT
AXPlatformNode* AXPlatformNode::FromNativeViewAccessible(
AX_EXPORT shouldn't be needed in the .cc files

https://codereview.chromium.org/2715543003/diff/240001/ui/accessibility/platf...
File ui/accessibility/platform/ax_platform_node_mac.mm (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/accessibility/platf...
ui/accessibility/platform/ax_platform_node_mac.mm:635: AX_EXPORT AXPlatformNode*
AXPlatformNode::FromNativeViewAccessible(
remove AX_EXPORT

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility.cc (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.cc:125: return
view_->parent()->GetNativeViewAccessible();
I think this is where we need GetForView? Except GetParent() here will always
return nullptr on ChromeOS. (so what happens if we don't define GetForView on
ChromeOS at all?)

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.cc:256: return
view->native_view_accessibility_.get();
The CL description needs to be clear why this is needed too (I think it's
related to skipping over parents?)

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility.h (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.h:94: static
NativeViewAccessibility* Create(View* view);
Do we need this? (i.e. Why can't we use the current `Create` that returns a
unique_ptr -- it looks like only CreateForView calls it.

Note if we don't need a version that returns a raw pointer, we probably don't
need to rename Create to CreateForView either)

[tapted, 2017-03-06 01:46:00]

https://codereview.chromium.org/2715543003/diff/240001/testing/platform_test.h
File testing/platform_test.h (right):

https://codereview.chromium.org/2715543003/diff/240001/testing/platform_test....
testing/platform_test.h:12: typedef struct objc_object* id;
On 2017/03/06 01:43:09, tapted wrote:
> be sure to call out the reason for this in the CL description. i.e. that 
> `void*` used to work when `id` was just a return type, but now we need it as a
> method argument, and we can't have .cc files and .mm files seeing different
> types, or they will declare an overload that doesn't exist.

actually I think we should move this (and the native_widget_types.h change) to a
new CL :/ (more splits - sorry!)

[Patti Lor, 2017-03-08 01:58:09]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 01:59:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-03-08 02:03:14]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 02:04:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-03-08 02:08:55]

Patchset #10 (id:280001) has been deleted

[commit-bot: I haz the power, 2017-03-08 02:11:15]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[commit-bot: I haz the power, 2017-03-08 02:12:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 02:12:19]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Patti Lor, 2017-03-08 02:22:27]

Patchset #9 (id:260001) has been deleted

[Patti Lor, 2017-03-08 02:22:28]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 02:23:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 02:39:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 02:39:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Patti Lor, 2017-03-08 05:29:52]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 05:30:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 05:40:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 05:40:15]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Patti Lor, 2017-03-08 05:57:42]

Patchset #10 (id:320001) has been deleted

[Patti Lor, 2017-03-08 05:59:02]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 05:59:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 07:32:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 07:32:36]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2017-03-13 02:58:45]

Description was changed from

==========
Views a11y: Implement AXPlatformNode::FromNativeViewAccessible on all platforms.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View or a gfx::NativeViewAccessible
is required. While AXPlatformNode::FromNativeViewAccessible() already does the
latter on Windows, it is not implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac with a fallback for ChromeOS. Then to retrieve a NativeViewAccessibility
instance from a View, implement NativeViewAccessibility::GetForView().

BUG=610589
==========

to

==========
Views a11y: Implement method to retrieve a NativeViewAccessibility from a View.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View is required. While this is
possible using AXPlatformNode::FromNativeViewAccessible() on Windows, it is not
implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac, which then allows a NativeViewAccessibility instance to be retrieved for
any given View. Since ChromeOS doesn't have a gfx::NativeViewAccessible, this
isn't possible, so instead friend NativeViewAccessibility from View in order to
access View's NativeViewAccessibility member directly.

BUG=610589
==========

[Patti Lor, 2017-03-13 03:03:26]

Hey Trent, sorry for the delay :/ Hopefully I didn't miss anything. Thanks!

https://codereview.chromium.org/2715543003/diff/240001/testing/platform_test.h
File testing/platform_test.h (right):

https://codereview.chromium.org/2715543003/diff/240001/testing/platform_test....
testing/platform_test.h:12: typedef struct objc_object* id;
On 2017/03/06 01:46:00, tapted wrote:
> On 2017/03/06 01:43:09, tapted wrote:
> > be sure to call out the reason for this in the CL description. i.e. that 
> > `void*` used to work when `id` was just a return type, but now we need it as
a
> > method argument, and we can't have .cc files and .mm files seeing different
> > types, or they will declare an overload that doesn't exist.
> 
> actually I think we should move this (and the native_widget_types.h change) to
a
> new CL :/ (more splits - sorry!)

Done with the cl desc explanation in the other CL as well :)

https://codereview.chromium.org/2715543003/diff/240001/ui/accessibility/platf...
File ui/accessibility/platform/ax_platform_node_auralinux.cc (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/accessibility/platf...
ui/accessibility/platform/ax_platform_node_auralinux.cc:377: AX_EXPORT
AXPlatformNode* AXPlatformNode::FromNativeViewAccessible(
On 2017/03/06 01:43:09, tapted wrote:
> AX_EXPORT shouldn't be needed in the .cc files

Whoops, a remnant from when I was trying to work out why things wouldn't link up
- thanks.

https://codereview.chromium.org/2715543003/diff/240001/ui/accessibility/platf...
File ui/accessibility/platform/ax_platform_node_mac.mm (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/accessibility/platf...
ui/accessibility/platform/ax_platform_node_mac.mm:635: AX_EXPORT AXPlatformNode*
AXPlatformNode::FromNativeViewAccessible(
On 2017/03/06 01:43:09, tapted wrote:
> remove AX_EXPORT 

Done.

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility.cc (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.cc:125: return
view_->parent()->GetNativeViewAccessible();
On 2017/03/06 01:43:09, tapted wrote:
> I think this is where we need GetForView? Except GetParent() here will always
> return nullptr on ChromeOS. (so what happens if we don't define GetForView on
> ChromeOS at all?)

Unfortunately I think we need this for GetChildCount() in the next CL
(https://crrev.com/2119413004/). The issue is that we need a NVA instance to
check if the View is ignored. Alternatively, I think we could check this
manually by inlining IsAccessibilityFocusableWhenEnabled() (see next CL again)
and also checking view_->GetAccessibleNodeData(), and then I think it would be
possible.

Or we could leave GetChildCount(), ChildAtIndex(), HitTestSync, and GetParent()
unimplemented for CrOS?

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.cc:256: return
view->native_view_accessibility_.get();
On 2017/03/06 01:43:09, tapted wrote:
> The CL description needs to be clear why this is needed too (I think it's
> related to skipping over parents?)

Updated the description with why we need a fallback with friend
NativeViewAccessibility for CrOS - let me know if that's not what you meant?

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility.h (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.h:94: static
NativeViewAccessibility* Create(View* view);
On 2017/03/06 01:43:09, tapted wrote:
> Do we need this? (i.e. Why can't we use the current `Create` that returns a
> unique_ptr -- it looks like only CreateForView calls it.
> 
> Note if we don't need a version that returns a raw pointer, we probably don't
> need to rename Create to CreateForView either)

You're right, it looks like we don't need this any more :O Previously I had it
there to avoid some code duplication, but now CreateForView is short we can get
rid of it - thanks! Fixed, leaving it just as Create() now.

[tapted, 2017-03-13 04:07:02]

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility.cc (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.cc:125: return
view_->parent()->GetNativeViewAccessible();
On 2017/03/13 03:03:25, Patti Lor wrote:
> On 2017/03/06 01:43:09, tapted wrote:
> > I think this is where we need GetForView? Except GetParent() here will
always
> > return nullptr on ChromeOS. (so what happens if we don't define GetForView
on
> > ChromeOS at all?)
> 
> Unfortunately I think we need this for GetChildCount() in the next CL
> (https://crrev.com/2119413004/). The issue is that we need a NVA instance to
> check if the View is ignored. Alternatively, I think we could check this
> manually by inlining IsAccessibilityFocusableWhenEnabled() (see next CL again)
> and also checking view_->GetAccessibleNodeData(), and then I think it would be
> possible.
> 
> Or we could leave GetChildCount(), ChildAtIndex(), HitTestSync, and
GetParent()
> unimplemented for CrOS?

It's possible we need GetChildCount on CrOS.. (but also maybe not: only
AXPlatformBase calls it, and none exists on CrOS).

But ChildAtIndex(), HitTestSync() and GetParent() must all just return nullptr
on CrOS.



Essentially... it seems NativeViewAccessibility doesn't need to (and shouldn't)
implement ui::AXPlatformNodeDelegate on ChromeOS.

Perhaps we want NativeViewAccessibility and PlatformNativeViewAccessibility that
inherits form NVA.

https://codereview.chromium.org/2715543003/diff/340001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility_auralinux.h (right):

https://codereview.chromium.org/2715543003/diff/340001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility_auralinux.h:1: // Copyright
2015 The Chromium Authors. All rights reserved.
The changes to the 6 native_view_accessibility_foo.h/cc classes should be
deferred to a separate CL. They are good changes, but they distract from the
main/controversial bit of this CL.

[Patti Lor, 2017-03-17 05:15:15]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 05:15:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-03-17 05:35:52]

Description was changed from

==========
Views a11y: Implement method to retrieve a NativeViewAccessibility from a View.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View is required. While this is
possible using AXPlatformNode::FromNativeViewAccessible() on Windows, it is not
implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac, which then allows a NativeViewAccessibility instance to be retrieved for
any given View. Since ChromeOS doesn't have a gfx::NativeViewAccessible, this
isn't possible, so instead friend NativeViewAccessibility from View in order to
access View's NativeViewAccessibility member directly.

BUG=610589
==========

to

==========
Views a11y: Implement AXPlatformNode::FromNativeViewAccessible() for Mac/Linux.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View is required. While this is
possible using AXPlatformNode::FromNativeViewAccessible() on Windows, it is not
implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac, which makes it possible to retrieve a NativeViewAccessibility instance for
any given View.

BUG=610589
==========

[Patti Lor, 2017-03-17 05:51:40]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 05:51:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 06:19:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 06:19:14]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Patti Lor, 2017-03-17 06:30:02]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 06:30:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Patti Lor, 2017-03-17 06:36:34]

Patchset #13 (id:400001) has been deleted

[commit-bot: I haz the power, 2017-03-17 06:41:11]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Patti Lor, 2017-03-17 06:45:44]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 06:46:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 08:52:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-17 08:52:51]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2017-03-19 23:10:26]

Thanks for doing the refactor Trent :) PTAL!

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility.cc (right):

https://codereview.chromium.org/2715543003/diff/240001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility.cc:125: return
view_->parent()->GetNativeViewAccessible();
On 2017/03/13 04:07:02, tapted wrote:
> On 2017/03/13 03:03:25, Patti Lor wrote:
> > On 2017/03/06 01:43:09, tapted wrote:
> > > I think this is where we need GetForView? Except GetParent() here will
> always
> > > return nullptr on ChromeOS. (so what happens if we don't define GetForView
> on
> > > ChromeOS at all?)
> > 
> > Unfortunately I think we need this for GetChildCount() in the next CL
> > (https://crrev.com/2119413004/). The issue is that we need a NVA instance to
> > check if the View is ignored. Alternatively, I think we could check this
> > manually by inlining IsAccessibilityFocusableWhenEnabled() (see next CL
again)
> > and also checking view_->GetAccessibleNodeData(), and then I think it would
be
> > possible.
> > 
> > Or we could leave GetChildCount(), ChildAtIndex(), HitTestSync, and
> GetParent()
> > unimplemented for CrOS?
> 
> It's possible we need GetChildCount on CrOS.. (but also maybe not: only
> AXPlatformBase calls it, and none exists on CrOS).
> 
> But ChildAtIndex(), HitTestSync() and GetParent() must all just return nullptr
> on CrOS.
> 
> 
> 
> Essentially... it seems NativeViewAccessibility doesn't need to (and
shouldn't)
> implement ui::AXPlatformNodeDelegate on ChromeOS.
> 
> Perhaps we want NativeViewAccessibility and PlatformNativeViewAccessibility
that
> inherits form NVA.

Thanks for your giant refactor Trent! It took a bit of time to rebase but
everything seems to work and seems a lot nicer too :))

https://codereview.chromium.org/2715543003/diff/340001/ui/views/accessibility...
File ui/views/accessibility/native_view_accessibility_auralinux.h (right):

https://codereview.chromium.org/2715543003/diff/340001/ui/views/accessibility...
ui/views/accessibility/native_view_accessibility_auralinux.h:1: // Copyright
2015 The Chromium Authors. All rights reserved.
On 2017/03/13 04:07:02, tapted wrote:
> The changes to the 6 native_view_accessibility_foo.h/cc classes should be
> deferred to a separate CL. They are good changes, but they distract from the
> main/controversial bit of this CL.

Reverted!

[Patti Lor, 2017-03-22 02:51:22]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-22 02:52:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 04:53:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 04:53:53]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[tapted, 2017-03-23 04:24:32]

lgtm (although I can't decide whether we want to test this separately to the
stuff coming in https://codereview.chromium.org/2119413004  - we'd need some
friend declarations)

https://codereview.chromium.org/2715543003/diff/440001/ui/accessibility/platf...
File ui/accessibility/platform/ax_platform_node.cc (right):

https://codereview.chromium.org/2715543003/diff/440001/ui/accessibility/platf...
ui/accessibility/platform/ax_platform_node.cc:24: #if defined(OS_ANDROID)
I think this #if block can just be removed now (i.e. delete the
FromNativeViewAccessible that returns nullptr)

[Patti Lor, 2017-03-24 02:32:25]

The CQ bit was checked by patricialor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 02:32:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-24 04:09:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-24 04:09:54]

Dry run: This issue passed the CQ dry run.

[Patti Lor, 2017-03-24 04:33:59]

patricialor@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[Patti Lor, 2017-03-24 04:34:00]

Hi dmazzoni, PTAL! Thanks.

https://codereview.chromium.org/2715543003/diff/440001/ui/accessibility/platf...
File ui/accessibility/platform/ax_platform_node.cc (right):

https://codereview.chromium.org/2715543003/diff/440001/ui/accessibility/platf...
ui/accessibility/platform/ax_platform_node.cc:24: #if defined(OS_ANDROID)
On 2017/03/23 04:24:32, tapted (OOO until 2017-03-30) wrote:
> I think this #if block can just be removed now (i.e. delete the
> FromNativeViewAccessible that returns nullptr)

Oo, ok. Thanks, done!

[Patti Lor, 2017-03-30 03:07:23]

On 2017/03/24 04:34:00, Patti Lor wrote:
> Hi dmazzoni, PTAL! Thanks.
> 
>
https://codereview.chromium.org/2715543003/diff/440001/ui/accessibility/platf...
> File ui/accessibility/platform/ax_platform_node.cc (right):
> 
>
https://codereview.chromium.org/2715543003/diff/440001/ui/accessibility/platf...
> ui/accessibility/platform/ax_platform_node.cc:24: #if defined(OS_ANDROID)
> On 2017/03/23 04:24:32, tapted (OOO until 2017-03-30) wrote:
> > I think this #if block can just be removed now (i.e. delete the
> > FromNativeViewAccessible that returns nullptr)
> 
> Oo, ok. Thanks, done!

Ping for dmazzoni@?

[dmazzoni, 2017-03-30 15:38:59]

lgtm

[Patti Lor, 2017-03-31 03:22:06]

The CQ bit was checked by patricialor@chromium.org

[Patti Lor, 2017-03-31 03:22:06]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/2715543003/#ps460001
(title: "Try removing default impl for FromNativeViewAccessible again.")

[commit-bot: I haz the power, 2017-03-31 03:22:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 04:22:05]

CQ is committing da patch.

Bot data: {"patchset_id": 460001, "attempt_start_ts": 1490930526149130,
"parent_rev": "1d18c5d69956016f711094666eeb9592b540004e", "commit_rev":
"54bfa63a7eedc669fc481aa985d1f5979e695a1a"}

[commit-bot: I haz the power, 2017-03-31 04:22:56]

Description was changed from

==========
Views a11y: Implement AXPlatformNode::FromNativeViewAccessible() for Mac/Linux.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View is required. While this is
possible using AXPlatformNode::FromNativeViewAccessible() on Windows, it is not
implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac, which makes it possible to retrieve a NativeViewAccessibility instance for
any given View.

BUG=610589
==========

to

==========
Views a11y: Implement AXPlatformNode::FromNativeViewAccessible() for Mac/Linux.

In order to implement the ability to exclude unnecessary accessibility elements
from the accessibility tree cross-platform, a method to retrieve the
NativeViewAccessibility instance for a given View is required. While this is
possible using AXPlatformNode::FromNativeViewAccessible() on Windows, it is not
implemented on any other platform.

This patch implements AXPlatformNode::FromNativeViewAccessible() for Linux and
Mac, which makes it possible to retrieve a NativeViewAccessibility instance for
any given View.

BUG=610589

Review-Url: https://codereview.chromium.org/2715543003
Cr-Commit-Position: refs/heads/master@{#461026}
Committed:
https://chromium.googlesource.com/chromium/src/+/54bfa63a7eedc669fc481aa985d1...
==========

[commit-bot: I haz the power, 2017-03-31 04:22:57]

Committed patchset #15 (id:460001) as
https://chromium.googlesource.com/chromium/src/+/54bfa63a7eedc669fc481aa985d1...