Refactor content/ to use ui::AXNodeData instead of blink.

Refactor content/ to use ui::AXNodeData instead of blink.

This is a huge change, but it's all refactoring / renaming.
There should be no new logic.

BUG=316726
R=dtseng@chromium.org, jam@chromium.org, tsepez@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243187

[dmazzoni, 2014-01-06 17:16:43]

Trybots pass! Please review soon, it's challenging to maintain a patch this
large.

[David Tseng, 2014-01-06 17:49:45]

Taking a look now.


On Fri, Jan 3, 2014 at 4:17 PM, <dmazzoni@chromium.org> wrote:

> Reviewers: David Tseng, aboxhall,
>
> Description:
> Refactor content/ to use ui::AXNodeData instead of blink.
>
> This is a huge change, but it's all refactoring / renaming.
> There should be no new logic.
>
> BUG=316726
>
> Please review this at https://codereview.chromium.org/116293005/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+2658, -2610 lines):
>   D content/common/accessibility_node_data.h
>   D content/common/accessibility_node_data.cc
>   M content/browser/DEPS
>   M content/browser/accessibility/accessibility_win_browsertest.cc
>   M content/browser/accessibility/browser_accessibility.h
>   M content/browser/accessibility/browser_accessibility.cc
>   M content/browser/accessibility/browser_accessibility_android.cc
>   M content/browser/accessibility/browser_accessibility_cocoa.h
>   M content/browser/accessibility/browser_accessibility_cocoa.mm
>   M content/browser/accessibility/browser_accessibility_gtk.cc
>   M content/browser/accessibility/browser_accessibility_mac_unittest.mm
>   M content/browser/accessibility/browser_accessibility_manager.h
>   M content/browser/accessibility/browser_accessibility_manager.cc
>   M content/browser/accessibility/browser_accessibility_manager_android.h
>   M content/browser/accessibility/browser_accessibility_manager_android.cc
>   M content/browser/accessibility/browser_accessibility_manager_gtk.h
>   M content/browser/accessibility/browser_accessibility_manager_gtk.cc
>   M content/browser/accessibility/browser_accessibility_manager_mac.h
>   M content/browser/accessibility/browser_accessibility_manager_mac.mm
>   M content/browser/accessibility/browser_accessibility_manager_
> unittest.cc
>   M content/browser/accessibility/browser_accessibility_manager_win.h
>   M content/browser/accessibility/browser_accessibility_manager_win.cc
>   M content/browser/accessibility/browser_accessibility_win.h
>   M content/browser/accessibility/browser_accessibility_win.cc
>   M content/browser/accessibility/browser_accessibility_win_unittest.cc
>   M content/browser/accessibility/cross_platform_accessibility_
> browsertest.cc
>   M content/browser/accessibility/dump_accessibility_tree_browsertest.cc
>   M content/browser/renderer_host/render_view_host_impl.h
>   M content/browser/renderer_host/render_view_host_impl.cc
>   M content/common/DEPS
>   M content/common/accessibility_messages.h
>   M content/content_browser.gypi
>   M content/content_common.gypi
>   M content/content_renderer.gypi
>   M content/renderer/DEPS
>   M content/renderer/accessibility/accessibility_node_serializer.h
>   M content/renderer/accessibility/accessibility_node_serializer.cc
>   A content/renderer/accessibility/blink_ax_enum_conversion.h
>   A content/renderer/accessibility/blink_ax_enum_conversion.cc
>   M content/renderer/accessibility/renderer_accessibility.h
>   M content/renderer/accessibility/renderer_accessibility.cc
>   M content/renderer/accessibility/renderer_accessibility_browsertest.cc
>   M content/renderer/accessibility/renderer_accessibility_complete.h
>   M content/renderer/accessibility/renderer_accessibility_complete.cc
>   M content/renderer/accessibility/renderer_accessibility_focus_only.cc
>   M content/test/DEPS
>   M content/test/accessibility_browser_test_utils.h
>   M content/test/accessibility_browser_test_utils.cc
>   M ui/accessibility/ax_enums.h
>   M ui/accessibility/ax_node_data.h
>   M ui/accessibility/ax_node_data.cc
>   M ui/accessibility/ax_tree.h
>   M ui/accessibility/ax_tree.cc
>   M ui/accessibility/ax_tree_unittest.cc
>
>
> To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2014-01-06 18:13:54]

lgtm

https://codereview.chromium.org/116293005/diff/330001/content/browser/accessi...
File content/browser/accessibility/accessibility_tree_formatter_android.cc
(left):

https://codereview.chromium.org/116293005/diff/330001/content/browser/accessi...
content/browser/accessibility/accessibility_tree_formatter_android.cc:17:
#include "content/common/accessibility_node_data.h"
Was this simply unused or do you need to also include
ui/accessibility/ax_node_data.h?

https://codereview.chromium.org/116293005/diff/330001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility.cc (right):

https://codereview.chromium.org/116293005/diff/330001/content/browser/accessi...
content/browser/accessibility/browser_accessibility.cc:17: typedef
ui::AXFloatAttribute FloatAttribute;
Optional: I wonder if it wouldn't be easier to have a ui::accessibility
namespace and clean this up with "using" statements.

[dmazzoni, 2014-01-06 19:19:57]

https://codereview.chromium.org/116293005/diff/330001/content/browser/accessi...
File content/browser/accessibility/accessibility_tree_formatter_android.cc
(left):

https://codereview.chromium.org/116293005/diff/330001/content/browser/accessi...
content/browser/accessibility/accessibility_tree_formatter_android.cc:17:
#include "content/common/accessibility_node_data.h"
On 2014/01/06 18:13:55, David Tseng wrote:
> Was this simply unused or do you need to also include
> ui/accessibility/ax_node_data.h?

In this case it was unused, it was included implicitly.

https://codereview.chromium.org/116293005/diff/330001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility.cc (right):

https://codereview.chromium.org/116293005/diff/330001/content/browser/accessi...
content/browser/accessibility/browser_accessibility.cc:17: typedef
ui::AXFloatAttribute FloatAttribute;
On 2014/01/06 18:13:55, David Tseng wrote:
> Optional: I wonder if it wouldn't be easier to have a ui::accessibility
> namespace and clean this up with "using" statements.

I got rid of the typedefs. It's only saving 6 characters now, not 23.

A namespace wouldn't help much, you can't use "using" to promote enum values to
the current namespace, just individual classes and variables.

[dmazzoni, 2014-01-06 19:21:08]

+jam for OWNERS review of content/
+tsepez for OWNERS review of content/common/*messages.h

[jam, 2014-01-06 21:51:52]

lgtm

I skipped directories with accessibility in their name

https://codereview.chromium.org/116293005/diff/500001/content/browser/DEPS
File content/browser/DEPS (right):

https://codereview.chromium.org/116293005/diff/500001/content/browser/DEPS#ne...
content/browser/DEPS:10: "+ui/accessibility",
nit: since this is being added in browser/common/renderer/test, I would just put
it in content/DEPS. that's where all the other ui/ rules are

https://codereview.chromium.org/116293005/diff/500001/content/browser/rendere...
File content/browser/renderer_host/render_view_host_impl.h (right):

https://codereview.chromium.org/116293005/diff/500001/content/browser/rendere...
content/browser/renderer_host/render_view_host_impl.h:57: class AXTree;
nit: class before struct

[Tom Sepez, 2014-01-06 21:54:52]

Messages LGTM.

[dmazzoni, 2014-01-06 22:25:53]

Thanks for the fast reviews - I appreciate not having to rebase this change many
times!

https://codereview.chromium.org/116293005/diff/500001/content/browser/DEPS
File content/browser/DEPS (right):

https://codereview.chromium.org/116293005/diff/500001/content/browser/DEPS#ne...
content/browser/DEPS:10: "+ui/accessibility",
On 2014/01/06 21:51:53, jam wrote:
> nit: since this is being added in browser/common/renderer/test, I would just
put
> it in content/DEPS. that's where all the other ui/ rules are

Done.

https://codereview.chromium.org/116293005/diff/500001/content/browser/rendere...
File content/browser/renderer_host/render_view_host_impl.h (right):

https://codereview.chromium.org/116293005/diff/500001/content/browser/rendere...
content/browser/renderer_host/render_view_host_impl.h:57: class AXTree;
On 2014/01/06 21:51:53, jam wrote:
> nit: class before struct

Done.

[dmazzoni, 2014-01-06 22:31:28]

Committed patchset #8 manually as r243187 (presubmit successful).