Refactor all accessibility code out of webkit/glue.

Refactor all accessibility code out of webkit/glue.

Moves the data definitions into content/common and the serialization code
into content/renderer.

There are no code changes here, this is just a refactoring.

BUG=132129
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=141768

[dmazzoni, 2012-06-11 20:26:06]

No code changes, just refactoring.

jam: content
tony: webkit/glue

dtseng: fyi

FYI, I promise I'll move more accessibility code into the content/
namespace...but I don't want to do it in this same change, because it will touch
even more files and it's tricky because ATL doesn't allow COM objects to be in a
namespace.

[tony, 2012-06-11 20:56:05]

LGTM.  Splitting the serializing code into a separate file seems cleaner too.

[jam, 2012-06-12 00:00:42]

http://codereview.chromium.org/10544099/diff/9001/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility.h (right):

http://codereview.chromium.org/10544099/diff/9001/content/browser/accessibili...
content/browser/accessibility/browser_accessibility.h:27: using
content::AccessibilityNodeData;
this is against the google style guide per
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Namesp...

http://codereview.chromium.org/10544099/diff/9001/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_manager.h (right):

http://codereview.chromium.org/10544099/diff/9001/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_manager.h:23: using
content::AccessibilityNodeData;
ditto

http://codereview.chromium.org/10544099/diff/9001/content/common/accessibilit...
File content/common/accessibility_node_data.h (right):

http://codereview.chromium.org/10544099/diff/9001/content/common/accessibilit...
content/common/accessibility_node_data.h:22: public:
nit: unnecessary

http://codereview.chromium.org/10544099/diff/9001/content/common/accessibilit...
content/common/accessibility_node_data.h:256: public:
ditto

http://codereview.chromium.org/10544099/diff/9001/content/renderer/accessibil...
File content/renderer/accessibility_node_serializer.h (right):

http://codereview.chromium.org/10544099/diff/9001/content/renderer/accessibil...
content/renderer/accessibility_node_serializer.h:14: class
AccessibilityNodeSerializer {
nit: why a class instead of just having this be
content::SerializeAccessibilityNode?

http://codereview.chromium.org/10544099/diff/9001/content/renderer/accessibil...
content/renderer/accessibility_node_serializer.h:16: enum IncludeChildren {
nit: i realize you're just moving this, and in WebKit enums are used for
booleans. but in chrome this isn't the convention, and the function below looks
just as readable to me as 

 26    static void Serialize(const WebKit::WebAccessibilityObject& src,
 27                         AccessibilityNodeData* dst,
 28                         bool include_children,
 29                         bool include_line_breaks);

[dmazzoni, 2012-06-12 18:35:33]

http://codereview.chromium.org/10544099/diff/9001/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility.h (right):

http://codereview.chromium.org/10544099/diff/9001/content/browser/accessibili...
content/browser/accessibility/browser_accessibility.h:27: using
content::AccessibilityNodeData;
On 2012/06/12 00:00:42, John Abd-El-Malek wrote:
> this is against the google style guide per
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Namesp...

Done throughout, got rid of "using" in header files.

http://codereview.chromium.org/10544099/diff/9001/content/common/accessibilit...
File content/common/accessibility_node_data.h (right):

http://codereview.chromium.org/10544099/diff/9001/content/common/accessibilit...
content/common/accessibility_node_data.h:22: public:
On 2012/06/12 00:00:42, John Abd-El-Malek wrote:
> nit: unnecessary

Done.

http://codereview.chromium.org/10544099/diff/9001/content/common/accessibilit...
content/common/accessibility_node_data.h:256: public:
On 2012/06/12 00:00:42, John Abd-El-Malek wrote:
> ditto

Done.

http://codereview.chromium.org/10544099/diff/9001/content/renderer/accessibil...
File content/renderer/accessibility_node_serializer.h (right):

http://codereview.chromium.org/10544099/diff/9001/content/renderer/accessibil...
content/renderer/accessibility_node_serializer.h:14: class
AccessibilityNodeSerializer {
On 2012/06/12 00:00:42, John Abd-El-Malek wrote:
> nit: why a class instead of just having this be
> content::SerializeAccessibilityNode?

Mainly to encapsulate the enums, so I'm fine with making this a global function.
Done.

http://codereview.chromium.org/10544099/diff/9001/content/renderer/accessibil...
content/renderer/accessibility_node_serializer.h:16: enum IncludeChildren {
On 2012/06/12 00:00:42, John Abd-El-Malek wrote:
> nit: i realize you're just moving this, and in WebKit enums are used for
> booleans. but in chrome this isn't the convention, and the function below
looks
> just as readable to me as 
> 
>  26    static void Serialize(const WebKit::WebAccessibilityObject& src,
>  27                         AccessibilityNodeData* dst,
>  28                         bool include_children,
>  29                         bool include_line_breaks);

The style guide specifically suggests using enums instead of booleans, in
particular when there are multiple boolean arguments to the same function and
it'd be easy to get them mixed up.

http://dev.chromium.org/developers/coding-style#Code_Formatting

However, in this case it turns out we're not using include_line_breaks anymore,
so I got rid of that argument and switched include_children to a bool.

[jam, 2012-06-12 21:01:51]

lgtm

http://codereview.chromium.org/10544099/diff/9001/content/renderer/accessibil...
File content/renderer/accessibility_node_serializer.h (right):

http://codereview.chromium.org/10544099/diff/9001/content/renderer/accessibil...
content/renderer/accessibility_node_serializer.h:16: enum IncludeChildren {
On 2012/06/12 18:35:33, Dominic Mazzoni wrote:
> On 2012/06/12 00:00:42, John Abd-El-Malek wrote:
> > nit: i realize you're just moving this, and in WebKit enums are used for
> > booleans. but in chrome this isn't the convention, and the function below
> looks
> > just as readable to me as 
> > 
> >  26    static void Serialize(const WebKit::WebAccessibilityObject& src,
> >  27                         AccessibilityNodeData* dst,
> >  28                         bool include_children,
> >  29                         bool include_line_breaks);
> 
> The style guide specifically suggests using enums instead of booleans, in
> particular when there are multiple boolean arguments to the same function and
> it'd be easy to get them mixed up.
> 
> http://dev.chromium.org/developers/coding-style#Code_Formatting

That's the first time I hear this, and this change wasn't discussed publicly as
far as I can tell. I'll start another thread about this.

> 
> However, in this case it turns out we're not using include_line_breaks
anymore,
> so I got rid of that argument and switched include_children to a bool.

[commit-bot: I haz the power, 2012-06-12 21:03:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/10544099/6007

[commit-bot: I haz the power, 2012-06-12 22:55:20]

Change committed as 141768