Make BrowserAccessibilityManager cross platform. Step 1....

Make BrowserAccessibilityManager cross platform. Step 1.
1. Move windows browser accessibility source and headers into chrome\browser\accessibility
2. Rename BrowserAccessibilityManager to BrowserAccessibilityManagerWin and BrowserAccessibility to BrowserAccessibilityWin
3. Add base classes that will eventually contain common browser accessibility code.

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


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

[dmazzoni, 2010-09-30 13:31:43]

I think you created some new files that you forgot to add to this changelist.

Also, do you want to move the unittest to the accessibility/ directory too?

[David Tseng, 2010-09-30 19:22:53]

Awesome.  Looking at BrowserAccessibilityManager in detail, I think
almost all of it has relevance in the Mac port.  I think the next
steps would be to:
 strip BrowserAccessibilityManager(Win) of its notion of child id's
and bubble that resulting class into BrowserAccessibilityManager.  A
renderer_id_to_browserAccessibility map would be very handy as opposed
to keeping two maps.



On 9/30/10, ctguil@chromium.org <ctguil@chromium.org> wrote:
> http://codereview.chromium.org/3591003/show
>

[dmazzoni, 2010-10-01 17:14:36]

LGTM.

http://codereview.chromium.org/3591003/diff/52001/53003
File chrome/browser/accessibility/browser_accessibility.h (right):

http://codereview.chromium.org/3591003/diff/52001/53003#newcode15
chrome/browser/accessibility/browser_accessibility.h:15: // Class implementing
the Accessible interface for the Browser-Renderer
This description could be a little more informative, for example it could
mention that this object represents one node in the tree, corresponding directly
to an AccessibilityObject in WebKit...

Mention that this is the cross-platform base class, with an implementation for
each platform that handles platform-specific accessibility APIs.

http://codereview.chromium.org/3591003/diff/52001/53007
File chrome/browser/accessibility/browser_accessibility_manager_win.h (right):

http://codereview.chromium.org/3591003/diff/52001/53007#newcode24
chrome/browser/accessibility/browser_accessibility_manager_win.h:24: class
BrowserAccessibilityFactory {
Should this be a BrowserAccessibilityWinFactory?

http://codereview.chromium.org/3591003/diff/52001/53010
File chrome/browser/accessibility/browser_accessibility_win_unittest.cc (right):

http://codereview.chromium.org/3591003/diff/52001/53010#newcode242
chrome/browser/accessibility/browser_accessibility_win_unittest.cc:242: new
BrowserAccessibilityManagerWin(
Can you use BrowserAccessibilityManager::Create instead? Or would it be too
tricky to move the factory up to browser_accessibility_manager.h?

[Chris Guillory, 2010-10-01 17:42:23]

http://codereview.chromium.org/3591003/diff/52001/53003
File chrome/browser/accessibility/browser_accessibility.h (right):

http://codereview.chromium.org/3591003/diff/52001/53003#newcode15
chrome/browser/accessibility/browser_accessibility.h:15: // Class implementing
the Accessible interface for the Browser-Renderer
On 2010/10/01 17:14:37, Dominic Mazzoni wrote:
> This description could be a little more informative, for example it could
> mention that this object represents one node in the tree, corresponding
directly
> to an AccessibilityObject in WebKit...
> 
> Mention that this is the cross-platform base class, with an implementation for
> each platform that handles platform-specific accessibility APIs.
> 

Done.

http://codereview.chromium.org/3591003/diff/52001/53007
File chrome/browser/accessibility/browser_accessibility_manager_win.h (right):

http://codereview.chromium.org/3591003/diff/52001/53007#newcode24
chrome/browser/accessibility/browser_accessibility_manager_win.h:24: class
BrowserAccessibilityFactory {
On 2010/10/01 17:14:37, Dominic Mazzoni wrote:
> Should this be a BrowserAccessibilityWinFactory?

Done.

http://codereview.chromium.org/3591003/diff/52001/53010
File chrome/browser/accessibility/browser_accessibility_win_unittest.cc (right):

http://codereview.chromium.org/3591003/diff/52001/53010#newcode242
chrome/browser/accessibility/browser_accessibility_win_unittest.cc:242: new
BrowserAccessibilityManagerWin(
I think factory is one of the things that should move up. I'll add this as a
todo for now.

[dmazzoni, 2010-10-01 17:43:34]

LGTM.

On Fri, Oct 1, 2010 at 10:42 AM, <ctguil@chromium.org> wrote:

>
> http://codereview.chromium.org/3591003/diff/52001/53003
> File chrome/browser/accessibility/browser_accessibility.h (right):
>
> http://codereview.chromium.org/3591003/diff/52001/53003#newcode15
> chrome/browser/accessibility/browser_accessibility.h:15: // Class
> implementing the Accessible interface for the Browser-Renderer
> On 2010/10/01 17:14:37, Dominic Mazzoni wrote:
>
>> This description could be a little more informative, for example it
>>
> could
>
>> mention that this object represents one node in the tree,
>>
> corresponding directly
>
>> to an AccessibilityObject in WebKit...
>>
>
>  Mention that this is the cross-platform base class, with an
>>
> implementation for
>
>> each platform that handles platform-specific accessibility APIs.
>>
>
>
> Done.
>
>
> http://codereview.chromium.org/3591003/diff/52001/53007
> File chrome/browser/accessibility/browser_accessibility_manager_win.h
> (right):
>
> http://codereview.chromium.org/3591003/diff/52001/53007#newcode24
> chrome/browser/accessibility/browser_accessibility_manager_win.h:24:
> class BrowserAccessibilityFactory {
> On 2010/10/01 17:14:37, Dominic Mazzoni wrote:
>
>> Should this be a BrowserAccessibilityWinFactory?
>>
>
> Done.
>
>
> http://codereview.chromium.org/3591003/diff/52001/53010
> File chrome/browser/accessibility/browser_accessibility_win_unittest.cc
> (right):
>
> http://codereview.chromium.org/3591003/diff/52001/53010#newcode242
> chrome/browser/accessibility/browser_accessibility_win_unittest.cc:242:
> new BrowserAccessibilityManagerWin(
> I think factory is one of the things that should move up. I'll add this
> as a todo for now.
>
>
> http://codereview.chromium.org/3591003/show
>