Improve omnibox accessibility on Windows.

views/widget/native_widget_win.cc

Index: views/widget/native_widget_win.cc
===================================================================
--- views/widget/native_widget_win.cc	(revision 107097)
+++ views/widget/native_widget_win.cc	(working copy)
@@ -380,8 +380,6 @@
       ownership_(Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET),
       can_update_layered_window_(true),
       restore_focus_when_enabled_(false),
-      accessibility_view_events_index_(-1),
-      accessibility_view_events_(kMaxAccessibilityViewEvents),
       previous_cursor_(NULL),
       fullscreen_(false),
       force_hidden_count_(0),
@@ -447,31 +445,16 @@
   SetInitialFocus();
 }
 
-View* NativeWidgetWin::GetAccessibilityViewEventAt(int id) {
-  // Convert from MSAA child id.
-  id = -(id + 1);
-  DCHECK(id >= 0 && id < kMaxAccessibilityViewEvents);
-  return accessibility_view_events_[id];
+View* NativeWidgetWin::GetAccessibilityViewFromChildId(long child_id) {
+  long unique_id = -child_id;

[sky, 2011/10/25 20:08:45]

Why do you negate this?

[dmazzoni, 2011/10/26 16:46:13]

It's explained in SendNativeAccessibilityEvent; I added a comment pointing
there.

+  base::hash_map<long, View*>::iterator iter =
+      accessibility_id_view_map_.find(unique_id);
+  if (iter != accessibility_id_view_map_.end())

[sky, 2011/10/25 20:08:45]

nit: convert to a return statement.

+    return iter->second;
+  else
+    return NULL;
 }
 
-int NativeWidgetWin::AddAccessibilityViewEvent(View* view) {
-  accessibility_view_events_index_ =
-      (accessibility_view_events_index_ + 1) % kMaxAccessibilityViewEvents;
-  accessibility_view_events_[accessibility_view_events_index_] = view;
-
-  // Convert to MSAA child id.
-  return -(accessibility_view_events_index_ + 1);
-}
-
-void NativeWidgetWin::ClearAccessibilityViewEvent(View* view) {
-  for (std::vector<View*>::iterator it = accessibility_view_events_.begin();
-      it != accessibility_view_events_.end();
-      ++it) {
-    if (*it == view)
-      *it = NULL;
-  }
-}
-
 void NativeWidgetWin::PushForceHidden() {
   if (force_hidden_count_++ == 0)
     Hide();
@@ -592,7 +575,18 @@
   if (drop_target_.get())
     drop_target_->ResetTargetViewIfEquals(view);
 
-  ClearAccessibilityViewEvent(view);
+  // Remove this view from the map from accessibility unique IDs to views.
+  IAccessible* accessible = view->GetNativeViewAccessible();
+  IAccessible2* accessible2 = NULL;
+  HRESULT hr = accessible->QueryInterface(

[sky, 2011/10/25 20:08:45]

How expensive is this? Would it be better to maintain the inverse mapping
(view->id)?

[dmazzoni, 2011/10/26 16:46:13]

It couldn't be that expensive - it's called thousands of times per second when
accessibility is enabled.

Maintaining view->id would be just as ugly as maintaining id->view, we'd have to
update it every time the view hierarchy changed.

Since I changed it to search the view hierarchy, QueryInterface will now only be
called once per accessibility notification (these are rare, one or two per user
event like a click), and additional calls will only happen if assistive
technology is running and triggers GetViewByAccessibleId.

+      IID_IAccessible2, reinterpret_cast<void**>(&accessible2));
+  if (hr == S_OK) {
+    long unique_id;
+    hr = accessible2->get_uniqueID(&unique_id);
+    if (hr == S_OK) {

[sky, 2011/10/25 20:08:45]

nit: no {}

+      accessibility_id_view_map_.erase(unique_id);
+    }
+  }
 }
 
 void NativeWidgetWin::SetNativeWindowProperty(const char* name, void* value) {
@@ -623,14 +617,34 @@
 void NativeWidgetWin::SendNativeAccessibilityEvent(
     View* view,
     ui::AccessibilityTypes::Event event_type) {
-  // Now call the Windows-specific method to notify MSAA clients of this
-  // event.  The widget gives us a temporary unique child ID to associate
-  // with this view so that clients can call get_accChild in
-  // NativeViewAccessibilityWin to retrieve the IAccessible associated
-  // with this view.
-  int child_id = AddAccessibilityViewEvent(view);
-  ::NotifyWinEvent(NativeViewAccessibilityWin::MSAAEvent(event_type),
-                   GetNativeView(), OBJID_CLIENT, child_id);
+  DWORD native_event = NativeViewAccessibilityWin::MSAAEvent(event_type);
+  gfx::NativeView native_view = this->GetNativeView();
+
+  // Get the unique ID from the view's IAccessible2 interface.
+  IAccessible* accessible = view->GetNativeViewAccessible();
+  IAccessible2* accessible2 = NULL;
+  HRESULT hr = accessible->QueryInterface(
+      IID_IAccessible2, reinterpret_cast<void**>(&accessible2));
+  if (hr != S_OK) {
+    // If this is reached, you're probably trying to fire a notification on a
+    // views::NativeViewHost that has only a default IAccessible. You have to
+    // either call ::NotifyWinEvent directly, or override
+    // GetNativeViewAccessible for that view.
+    NOTREACHED();
+    return;
+  }
+  long unique_id;
+  hr = accessible2->get_uniqueID(&unique_id);
+  DCHECK_EQ(hr, S_OK);
+  if (!accessibility_id_view_map_[unique_id])

[sky, 2011/10/25 20:08:45]

Can you reset the map all the time? Or maybe DCHECK that NULL or == view.

+    accessibility_id_view_map_[unique_id] = view;
+
+  // Send the negation of the unique id as the child id parameter to
+  // NotifyWinEvent. The client will call get_accChild on the top-level
+  // NativeViewAccessibilityWin object, where positive child ids are
+  // interpreted as 1-based child indices, and negative child ids are
+  // interpreted as unique ids.
+  ::NotifyWinEvent(native_event, native_view, OBJID_CLIENT, -unique_id);
 }
 
 void NativeWidgetWin::SetMouseCapture() {