Make BrowserAccessibilityManager cross platform. Step 2.

Make BrowserAccessibilityManager cross platform. Step 2. 
1. Move common logic and fields from BrowserAccessibilityMangerWin to BrowserAccessibilityManager.
2. Move common logic and fields from BrowserAccessibilityWin to BrowserAccessibility.

BUG=55264
TEST=interactive_ui_tests:AccessibilityWinBrowserTest.* 
TEST=unit_tests:BrowserAccessibilityTest.* 


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

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

[Chris Guillory, 2010-10-05 21:46:09]

Ready for an initial look. I've moved a majority of the manager logic down. I'm
updating the tests currently.

[David Tseng, 2010-10-06 17:09:57]

I'll use the headers here as a base for the mac specific port and let
you know what works well if that seems like a good strategy.

First thing that I need is could we use gfx::NativeView instead of
gfx::NativeWindow?  It doesn't matter on windows, but on mac these are
different (NSView and NSWindow respectively).
RenderWidgetHostViewCocoa implements NSView.

On 10/5/10, ctguil@chromium.org <ctguil@chromium.org> wrote:
> Reviewers: Dominic Mazzoni,
>
> Message:
> Ready for an initial look. I've moved a majority of the manager logic down.
> I'm
> updating the tests currently.
>
> Description:
> Make BrowserAccessibilityManager cross platform. Step 2. In Progress.
>
> BUG=55264
> TEST=none
>
>
> Please review this at http://codereview.chromium.org/3551015/show
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
>    M     chrome/browser/accessibility/browser_accessibility.h
>    M     chrome/browser/accessibility/browser_accessibility.cc
>    M     chrome/browser/accessibility/browser_accessibility_manager.h
>    M     chrome/browser/accessibility/browser_accessibility_manager.cc
>    M     chrome/browser/accessibility/browser_accessibility_manager_win.h
>    M     chrome/browser/accessibility/browser_accessibility_manager_win.cc
>    M     chrome/browser/accessibility/browser_accessibility_win.h
>    M     chrome/browser/accessibility/browser_accessibility_win.cc
>
>
>

[dmazzoni, 2010-10-06 17:50:33]

This is fantastic!

LGTM.

http://codereview.chromium.org/3551015/diff/17001/18002
File chrome/browser/accessibility/browser_accessibility.h (right):

http://codereview.chromium.org/3551015/diff/17001/18002#newcode82
chrome/browser/accessibility/browser_accessibility.h:82: // Release a reference
to this node.
Maybe note that this may be a no-op on platforms other than Windows?

http://codereview.chromium.org/3551015/diff/17001/18005
File chrome/browser/accessibility/browser_accessibility_manager_win.cc (right):

http://codereview.chromium.org/3551015/diff/17001/18005#newcode30
chrome/browser/accessibility/browser_accessibility_manager_win.cc:30: static
LONG next_child_id_ = -1;
Any reason not to make this a static class member? I'd prefer to keep it as a
static class member, but if not, it shouldn't have the trailing underscore.

[Chris Guillory, 2010-10-06 18:37:36]

That sounds like a very good strategy. Let's go over the base headers with
respect to the mac port sometime today. And I'll look into using
gfx::NativeView.

On Wed, Oct 6, 2010 at 10:09 AM, David Tseng <dtseng@chromium.org> wrote:

> I'll use the headers here as a base for the mac specific port and let
> you know what works well if that seems like a good strategy.
>
> First thing that I need is could we use gfx::NativeView instead of
> gfx::NativeWindow?  It doesn't matter on windows, but on mac these are
> different (NSView and NSWindow respectively).
> RenderWidgetHostViewCocoa implements NSView.
>
> On 10/5/10, ctguil@chromium.org <ctguil@chromium.org> wrote:
> > Reviewers: Dominic Mazzoni,
> >
> > Message:
> > Ready for an initial look. I've moved a majority of the manager logic
> down.
> > I'm
> > updating the tests currently.
> >
> > Description:
> > Make BrowserAccessibilityManager cross platform. Step 2. In Progress.
> >
> > BUG=55264
> > TEST=none
> >
> >
> > Please review this at http://codereview.chromium.org/3551015/show
> >
> > SVN Base: svn://chrome-svn/chrome/trunk/src/
> >
> > Affected files:
> >    M     chrome/browser/accessibility/browser_accessibility.h
> >    M     chrome/browser/accessibility/browser_accessibility.cc
> >    M     chrome/browser/accessibility/browser_accessibility_manager.h
> >    M     chrome/browser/accessibility/browser_accessibility_manager.cc
> >    M     chrome/browser/accessibility/browser_accessibility_manager_win.h
> >    M
> chrome/browser/accessibility/browser_accessibility_manager_win.cc
> >    M     chrome/browser/accessibility/browser_accessibility_win.h
> >    M     chrome/browser/accessibility/browser_accessibility_win.cc
> >
> >
> >
>

[Chris Guillory, 2010-10-06 19:15:07]

http://codereview.chromium.org/3551015/diff/17001/18002
File chrome/browser/accessibility/browser_accessibility.h (right):

http://codereview.chromium.org/3551015/diff/17001/18002#newcode82
chrome/browser/accessibility/browser_accessibility.h:82: // Release a reference
to this node.
On 2010/10/06 17:50:34, Dominic Mazzoni wrote:
> Maybe note that this may be a no-op on platforms other than Windows?

Done.

http://codereview.chromium.org/3551015/diff/17001/18005
File chrome/browser/accessibility/browser_accessibility_manager_win.cc (right):

http://codereview.chromium.org/3551015/diff/17001/18005#newcode30
chrome/browser/accessibility/browser_accessibility_manager_win.cc:30: static
LONG next_child_id_ = -1;
On 2010/10/06 17:50:34, Dominic Mazzoni wrote:
> Any reason not to make this a static class member? I'd prefer to keep it as a
> static class member, but if not, it shouldn't have the trailing underscore.
> 
> 

Done.