Accessibility information from the renderer was not being returned to tools l...

Accessibility information from the renderer was not being returned to tools like screen readers, etc.
This was because the object implementing the root IAccessible interface did not know how to navigate
a native render view. To work around this, we now check if the underlying view is a native view in
our root IAccessible::get_accChild function and if yes we send the WM_GETOBJECT message to the
corresponding RenderWidgetHostViewWin class, which returns an IAccessible interface which then can
expose information from the renderer.

The other changes in the accessibility code are for some crashes I observed while testing the 
screen readers like nvda, the Windows screen reader which send a flurry of WM_GETOBJECT messages
leading to some race conditions.

We also ensure that the accessibility information for a view is torn down when the view is destroyed.

Fixes portions of http://code.google.com/p/chromium/issues/detail?id=13291

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

[jcampan, 2009-10-09 19:17:31]

http://codereview.chromium.org/261044/diff/1/7
File chrome/browser/browser_accessibility_manager.cc (right):

http://codereview.chromium.org/261044/diff/1/7#newcode41
Line 41: 
Nit: remove empty line

http://codereview.chromium.org/261044/diff/1/8
File chrome/browser/renderer_host/render_widget_host_view_win.cc (left):

http://codereview.chromium.org/261044/diff/1/8#oldcode1252
Line 1252: NOTREACHED();
Is is safe to remove this?

http://codereview.chromium.org/261044/diff/1/8
File chrome/browser/renderer_host/render_widget_host_view_win.cc (right):

http://codereview.chromium.org/261044/diff/1/8#newcode36
Line 36: #include "views/accessibility/view_accessibility.h"
Nit: should be before #include "views/focus/focus_util_win.h"

http://codereview.chromium.org/261044/diff/1/2
File views/accessibility/view_accessibility.cc (right):

http://codereview.chromium.org/261044/diff/1/2#newcode11
Line 11: L"Chrome_Uninitialize_AccessibilityInstance";
Could you use a name non Chrome specific? For example:
Views_Uninitialize_AccessibilityInstance

http://codereview.chromium.org/261044/diff/1/2#newcode14
Line 14: L"Chrome_RenderViewHostHWNDForAccessibility";
Could you use a name non Chrome specific? For example
Views_NativeViewHostHWNDForAccessibility

http://codereview.chromium.org/261044/diff/1/2#newcode553
Line 553: 
Nit: remove empty line

[ananta, 2009-10-09 20:56:34]

http://codereview.chromium.org/261044/diff/1/7
File chrome/browser/browser_accessibility_manager.cc (right):

http://codereview.chromium.org/261044/diff/1/7#newcode41
Line 41: 
On 2009/10/09 19:17:31, jcampan wrote:
> Nit: remove empty line

This file has been reverted.

http://codereview.chromium.org/261044/diff/1/8
File chrome/browser/renderer_host/render_widget_host_view_win.cc (left):

http://codereview.chromium.org/261044/diff/1/8#oldcode1252
Line 1252: NOTREACHED();
On 2009/10/09 19:17:31, jcampan wrote:
> Is is safe to remove this?
Yes. It is valid to return NULL, if we fail to get the IAccessible interface,
which could happen for instance if the renderer died, etc.

http://codereview.chromium.org/261044/diff/1/8
File chrome/browser/renderer_host/render_widget_host_view_win.cc (right):

http://codereview.chromium.org/261044/diff/1/8#newcode36
Line 36: #include "views/accessibility/view_accessibility.h"
On 2009/10/09 19:17:31, jcampan wrote:
> Nit: should be before #include "views/focus/focus_util_win.h"

Done.

http://codereview.chromium.org/261044/diff/1/2
File views/accessibility/view_accessibility.cc (right):

http://codereview.chromium.org/261044/diff/1/2#newcode11
Line 11: L"Chrome_Uninitialize_AccessibilityInstance";
On 2009/10/09 19:17:31, jcampan wrote:
> Could you use a name non Chrome specific? For example:
> Views_Uninitialize_AccessibilityInstance

Done.

http://codereview.chromium.org/261044/diff/1/2#newcode14
Line 14: L"Chrome_RenderViewHostHWNDForAccessibility";
On 2009/10/09 19:17:31, jcampan wrote:
> Could you use a name non Chrome specific? For example
> Views_NativeViewHostHWNDForAccessibility

Done.

http://codereview.chromium.org/261044/diff/1/2#newcode553
Line 553: 
On 2009/10/09 19:17:31, jcampan wrote:
> Nit: remove empty line

Done.

[Jonas Klink (Google), 2009-10-09 21:48:27]

LGTM

[jcampan, 2009-10-09 22:04:42]

http://codereview.chromium.org/261044/diff/2005/2006
File views/accessibility/view_accessibility.cc (right):

http://codereview.chromium.org/261044/diff/2005/2006#newcode10
Line 10: const wchar_t kChromeUninitializeAccessibilityInstance[] =
You still have Chrome in the const name.

http://codereview.chromium.org/261044/diff/2005/2006#newcode13
Line 13: const wchar_t kRenderViewHostPropForAccessibility[] =
You still have RenderViewHost in the const name.

[ananta, 2009-10-09 22:14:11]

http://codereview.chromium.org/261044/diff/2005/2006
File views/accessibility/view_accessibility.cc (right):

http://codereview.chromium.org/261044/diff/2005/2006#newcode10
Line 10: const wchar_t kChromeUninitializeAccessibilityInstance[] =
On 2009/10/09 22:04:42, jcampan wrote:
> You still have Chrome in the const name.

Done.

http://codereview.chromium.org/261044/diff/2005/2006#newcode13
Line 13: const wchar_t kRenderViewHostPropForAccessibility[] =
On 2009/10/09 22:04:42, jcampan wrote:
> You still have RenderViewHost in the const name.

Done.

[jcampan, 2009-10-09 22:22:13]

LGTM