Remedy irratic focus behavior when accessibility clients set focus.

BUG=none
TEST=manually on Mac.
TBR=tony@chromium.org, avi@chromium.org

Review URL: http://codereview.chromium.org/10069003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132928 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 129 insertions, 130 deletions

diff --git a/content/browser/accessibility/browser_accessibility_manager.cc b/content/browser/accessibility/browser_accessibility_manager.cc
index 4d81dae..9c5329d 100644
--- a/content/browser/accessibility/browser_accessibility_manager.cc
+++ b/content/browser/accessibility/browser_accessibility_manager.cc
@@ -152,7 +152,8 @@ void BrowserAccessibilityManager::OnAccessibilityNotifications(
       continue;
     }
 
-    if (param.notification_type == AccessibilityNotificationFocusChanged) {
+    int notification_type = param.notification_type;
+    if (notification_type == AccessibilityNotificationFocusChanged) {
       SetFocus(node, false);
 
       // Don't send a native focus event if the window itself doesn't
@@ -162,10 +163,10 @@ void BrowserAccessibilityManager::OnAccessibilityNotifications(
     }
 
     // Send the notification event to the operating system.
-    NotifyAccessibilityEvent(param.notification_type, node);
+    NotifyAccessibilityEvent(notification_type, node);
 
     // Set initial focus when a page is loaded.
-    if (param.notification_type == AccessibilityNotificationLoadComplete) {
+    if (notification_type == AccessibilityNotificationLoadComplete) {
       if (!focus_)
         SetFocus(root_, false);
       if (!delegate_ || delegate_->HasFocus())
@@ -293,9 +294,10 @@ void BrowserAccessibilityManager::UpdateNode(
   // If the only reference to the focused node is focus_ itself, then the
   // focused node is no longer in the tree, so set the focus to the root.
   if (focus_ && focus_->ref_count() == 1) {
-    SetFocus(root_, true);
-    if (!delegate_ || delegate_->HasFocus())
-      NotifyAccessibilityEvent(AccessibilityNotificationFocusChanged, focus_);
+    SetFocus(root_, false);
+
+    if (delegate_ && delegate_->HasFocus())
+      NotifyAccessibilityEvent(AccessibilityNotificationBlur, focus_);
   }
 }
 
diff --git a/content/browser/accessibility/browser_accessibility_manager_mac.mm b/content/browser/accessibility/browser_accessibility_manager_mac.mm
index f9d6730..92da9d5 100644
--- a/content/browser/accessibility/browser_accessibility_manager_mac.mm
+++ b/content/browser/accessibility/browser_accessibility_manager_mac.mm
@@ -40,6 +40,9 @@ void BrowserAccessibilityManagerMac::NotifyAccessibilityEvent(
     case AccessibilityNotificationAlert:
       // Not used on Mac.
       return;
+    case AccessibilityNotificationBlur:
+      // A no-op on Mac.
+      return;
     case AccessibilityNotificationCheckStateChanged:
       // Not used on Mac.
       return;
@@ -91,7 +94,7 @@ void BrowserAccessibilityManagerMac::NotifyAccessibilityEvent(
     case AccessibilityNotificationTextRemoved:
       // Not used on Mac.
       return;
-    case AccessibilityNotificationValueChangedD:
+    case AccessibilityNotificationValueChanged:
       event_id = NSAccessibilityValueChangedNotification;
       break;
   }
diff --git a/content/browser/accessibility/browser_accessibility_manager_win.cc b/content/browser/accessibility/browser_accessibility_manager_win.cc
index 3901d93..2670ee1 100644
--- a/content/browser/accessibility/browser_accessibility_manager_win.cc
+++ b/content/browser/accessibility/browser_accessibility_manager_win.cc
@@ -65,6 +65,11 @@ void BrowserAccessibilityManagerWin::NotifyAccessibilityEvent(
     case AccessibilityNotificationActiveDescendantChanged:
       event_id = IA2_EVENT_ACTIVE_DESCENDANT_CHANGED;
       break;
+    case AccessibilityNotificationBlur:
+      // Equivalent to focus on the root.
+      event_id = EVENT_OBJECT_FOCUS;
+      node = GetRoot();
+      break;
     case AccessibilityNotificationCheckStateChanged:
       event_id = EVENT_OBJECT_STATECHANGE;
       break;
@@ -77,7 +82,7 @@ void BrowserAccessibilityManagerWin::NotifyAccessibilityEvent(
     case AccessibilityNotificationLoadComplete:
       event_id = IA2_EVENT_DOCUMENT_LOAD_COMPLETE;
       break;
-    case AccessibilityNotificationValueChangedD:
+    case AccessibilityNotificationValueChanged:
       event_id = EVENT_OBJECT_VALUECHANGE;
       break;
     case AccessibilityNotificationSelectedTextChanged:
diff --git a/content/common/accessibility_messages.h b/content/common/accessibility_messages.h
index 23bb4f0..c7763e1 100644
--- a/content/common/accessibility_messages.h
+++ b/content/common/accessibility_messages.h
@@ -32,6 +32,9 @@ enum AccessibilityNotification {
   // An alert appeared.
   AccessibilityNotificationAlert,
 
+  // A node has lost focus.
+  AccessibilityNotificationBlur,
+
   // The node checked state has changed.
   AccessibilityNotificationCheckStateChanged,
 
@@ -84,7 +87,7 @@ enum AccessibilityNotification {
   AccessibilityNotificationTextRemoved,
 
   // The node value has changed.
-  AccessibilityNotificationValueChangedD,
+  AccessibilityNotificationValueChanged,
 };
 
 #endif  // CONTENT_COMMON_ACCESSIBILITY_MESSAGES_H_
diff --git a/content/renderer/renderer_accessibility.cc b/content/renderer/renderer_accessibility.cc
index bfb22f1..666b06e 100644
--- a/content/renderer/renderer_accessibility.cc
+++ b/content/renderer/renderer_accessibility.cc
@@ -8,6 +8,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/renderer/render_view_impl.h"
 #include "content/renderer/renderer_accessibility.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebAccessibilityNotification.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebAccessibilityObject.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
@@ -74,7 +75,7 @@ bool WebAccessibilityNotificationToAccessibilityNotification(
       *type = AccessibilityNotificationSelectedTextChanged;
       break;
     case WebKit::WebAccessibilityNotificationValueChanged:
-      *type = AccessibilityNotificationValueChangedD;
+      *type = AccessibilityNotificationValueChanged;
       break;
     default:
       NOTREACHED();
@@ -134,7 +135,7 @@ void RendererAccessibility::FocusedNodeChanged(const WebNode& node) {
     // TODO(dmazzoni): Make WebKit send this notification instead.
     PostAccessibilityNotification(
         document.accessibilityObject(),
-        WebKit::WebAccessibilityNotificationFocusedUIElementChanged);
+        AccessibilityNotificationBlur);
   }
 }
 
@@ -161,6 +162,18 @@ void RendererAccessibility::DidFinishLoad(WebKit::WebFrame* frame) {
 void RendererAccessibility::PostAccessibilityNotification(
     const WebAccessibilityObject& obj,
     WebAccessibilityNotification notification) {
+  AccessibilityNotification temp;
+  if (!WebAccessibilityNotificationToAccessibilityNotification(
+      notification, &temp)) {
+    return;
+  }
+
+  PostAccessibilityNotification(obj, temp);
+}
+
+void RendererAccessibility::PostAccessibilityNotification(
+    const WebKit::WebAccessibilityObject& obj,
+    AccessibilityNotification notification) {
   if (!WebAccessibilityObject::accessibilityEnabled())
     return;
 
@@ -184,20 +197,15 @@ void RendererAccessibility::PostAccessibilityNotification(
   }
 
   // Add the accessibility object to our cache and ensure it's valid.
-  Notification acc_notification;
+  AccessibilityHostMsg_NotificationParams acc_notification;
   acc_notification.id = obj.axID();
-  acc_notification.type = notification;
-
-  AccessibilityNotification temp;
-  if (!WebAccessibilityNotificationToAccessibilityNotification(
-      notification, &temp)) {
-    return;
-  }
+  acc_notification.notification_type = notification;
 
   // Discard duplicate accessibility notifications.
   for (uint32 i = 0; i < pending_notifications_.size(); ++i) {
     if (pending_notifications_[i].id == acc_notification.id &&
-        pending_notifications_[i].type == acc_notification.type) {
+        pending_notifications_[i].notification_type ==
+            acc_notification.notification_type) {
       return;
     }
   }
@@ -228,16 +236,17 @@ void RendererAccessibility::SendPendingAccessibilityNotifications() {
   // Make a copy of the notifications, because it's possible that
   // actions inside this loop will cause more notifications to be
   // queued up.
-  std::vector<Notification> src_notifications = pending_notifications_;
+  std::vector<AccessibilityHostMsg_NotificationParams> src_notifications =
+      pending_notifications_;
   pending_notifications_.clear();
 
   // Generate a notification message from each WebKit notification.
   std::vector<AccessibilityHostMsg_NotificationParams> notification_msgs;
 
-  // Loop over each WebKit notification and generate a notification message
-  // from it.
+  // Loop over each notification and generate an updated notification message.
   for (size_t i = 0; i < src_notifications.size(); ++i) {
-    Notification& notification = src_notifications[i];
+    AccessibilityHostMsg_NotificationParams& notification =
+        src_notifications[i];
 
     // TODO(dtseng): Come up with a cleaner way of deciding to include children.
     int root_id = document.accessibilityObject().axID();
@@ -256,8 +265,8 @@ void RendererAccessibility::SendPendingAccessibilityNotifications() {
            obj.axID() != root_id) {
       obj = obj.parentObject();
       includes_children = true;
-      if (notification.type ==
-          WebKit::WebAccessibilityNotificationChildrenChanged) {
+      if (notification.notification_type ==
+          AccessibilityNotificationChildrenChanged) {
         notification.id = obj.axID();
       }
     }
@@ -303,11 +312,10 @@ void RendererAccessibility::SendPendingAccessibilityNotifications() {
     }
 
     AccessibilityHostMsg_NotificationParams notification_msg;
-    BuildAccessibilityTree(obj, includes_children, &notification_msg.acc_tree);
-    WebAccessibilityNotificationToAccessibilityNotification(
-        notification.type, &notification_msg.notification_type);
+    notification_msg.notification_type = notification.notification_type;
     notification_msg.id = notification.id;
     notification_msg.includes_children = includes_children;
+    BuildAccessibilityTree(obj, includes_children, &notification_msg.acc_tree);
     if (obj.axID() == root_id) {
       DCHECK_EQ(notification_msg.acc_tree.role,
                 WebAccessibility::ROLE_WEB_AREA);
@@ -320,10 +328,11 @@ void RendererAccessibility::SendPendingAccessibilityNotifications() {
 
 #ifndef NDEBUG
     if (logging_) {
-      LOG(INFO) << "Accessibility update: "
-                << notification_msg.acc_tree.DebugString(true,
-                                                         routing_id(),
-                                                         notification.type);
+      LOG(INFO) << "Accessibility update: \n"
+          << "routing id=" << routing_id()
+          << " notification="
+          << AccessibilityNotificationToString(notification.notification_type)
+          << "\n" << notification_msg.acc_tree.DebugString(true);
     }
 #endif
   }
@@ -549,12 +558,12 @@ void RendererAccessibility::OnSetFocus(int acc_obj_id) {
 }
 
 bool RendererAccessibility::ShouldIncludeChildren(
-    const RendererAccessibility::Notification& notification) {
-  WebKit::WebAccessibilityNotification type = notification.type;
-  if (type == WebKit::WebAccessibilityNotificationChildrenChanged ||
-      type == WebKit::WebAccessibilityNotificationLoadComplete ||
-      type == WebKit::WebAccessibilityNotificationLiveRegionChanged ||
-      type == WebKit::WebAccessibilityNotificationSelectedChildrenChanged) {
+    const AccessibilityHostMsg_NotificationParams& notification) {
+  AccessibilityNotification type = notification.notification_type;
+  if (type == AccessibilityNotificationChildrenChanged ||
+      type == AccessibilityNotificationLoadComplete ||
+      type == AccessibilityNotificationLiveRegionChanged ||
+      type == AccessibilityNotificationSelectedChildrenChanged) {
     return true;
   }
   return false;
@@ -620,3 +629,56 @@ void RendererAccessibility::BuildAccessibilityTree(
   // Find all editable text nodes and add them as children.
   RecursiveAddEditableTextNodesToTree(src, dst);
 }
+
+#ifndef NDEBUG
+const std::string RendererAccessibility::AccessibilityNotificationToString(
+    AccessibilityNotification notification) {
+  switch (notification) {
+    case AccessibilityNotificationActiveDescendantChanged:
+      return "active descendant changed";
+    case AccessibilityNotificationBlur:
+      return "blur";
+    case AccessibilityNotificationAlert:
+      return "alert";
+    case AccessibilityNotificationCheckStateChanged:
+      return "check state changed";
+    case AccessibilityNotificationChildrenChanged:
+      return "children changed";
+    case AccessibilityNotificationFocusChanged:
+      return "focus changed";
+    case AccessibilityNotificationLayoutComplete:
+      return "layout complete";
+    case AccessibilityNotificationLiveRegionChanged:
+      return "live region changed";
+    case AccessibilityNotificationLoadComplete:
+      return "load complete";
+    case AccessibilityNotificationMenuListValueChanged:
+      return "menu list changed";
+    case AccessibilityNotificationObjectShow:
+      return "object show";
+    case AccessibilityNotificationObjectHide:
+      return "object hide";
+    case AccessibilityNotificationRowCountChanged:
+      return "row count changed";
+    case AccessibilityNotificationRowCollapsed:
+      return "row collapsed";
+    case AccessibilityNotificationRowExpanded:
+      return "row expanded";
+    case AccessibilityNotificationScrolledToAnchor:
+      return "scrolled to anchor";
+    case AccessibilityNotificationSelectedChildrenChanged:
+      return "selected children changed";
+    case AccessibilityNotificationSelectedTextChanged:
+      return "selected text changed";
+    case AccessibilityNotificationTextInserted:
+      return "text inserted";
+    case AccessibilityNotificationTextRemoved:
+      return "text removed";
+    case AccessibilityNotificationValueChanged:
+      return "value changed";
+    default:
+      NOTREACHED();
+  }
+  return "";
+}
+#endif
diff --git a/content/renderer/renderer_accessibility.h b/content/renderer/renderer_accessibility.h
index e7dd1dc..af39da9 100644
--- a/content/renderer/renderer_accessibility.h
+++ b/content/renderer/renderer_accessibility.h
@@ -12,7 +12,6 @@
 #include "base/memory/weak_ptr.h"
 #include "content/common/accessibility_messages.h"
 #include "content/public/renderer/render_view_observer.h"
-#include "third_party/WebKit/Source/WebKit/chromium/public/WebAccessibilityNotification.h"
 
 class RenderViewImpl;
 
@@ -43,22 +42,20 @@ class RendererAccessibility : public content::RenderViewObserver {
   virtual void DidFinishLoad(WebKit::WebFrame* frame) OVERRIDE;
 
   // Called when an accessibility notification occurs in WebKit.
-  virtual void PostAccessibilityNotification(
+  void PostAccessibilityNotification(
       const WebKit::WebAccessibilityObject& obj,
       WebKit::WebAccessibilityNotification notification);
 
  private:
-  // One accessibility notification from WebKit. These are queued up and
-  // used to send tree updates and notification messages from the
-  // renderer to the browser.
-  struct Notification {
-   public:
-    // The id of the accessibility object.
-    int32 id;
+  // Post an accessibility notification to be sent to the browser process.
+  void PostAccessibilityNotification(
+      const WebKit::WebAccessibilityObject& obj,
+      AccessibilityNotification notification);
 
-    // The accessibility notification type.
-    WebKit::WebAccessibilityNotification type;
-  };
+#ifndef NDEBUG
+  const std::string AccessibilityNotificationToString(
+      AccessibilityNotification notification);
+#endif
 
   // In order to keep track of what nodes the browser knows about, we keep a
   // representation of the browser tree - just IDs and parent/child
@@ -94,7 +91,8 @@ class RendererAccessibility : public content::RenderViewObserver {
 
   // Whether or not this notification typically needs to send
   // updates to its children, too.
-  bool ShouldIncludeChildren(const Notification& notification);
+  bool ShouldIncludeChildren(
+      const AccessibilityHostMsg_NotificationParams& notification);
 
   // Returns the main top-level document for this page, or NULL if there's
   // no view or frame.
@@ -122,7 +120,7 @@ class RendererAccessibility : public content::RenderViewObserver {
 
   // Notifications from WebKit are collected until they are ready to be
   // sent to the browser.
-  std::vector<Notification> pending_notifications_;
+  std::vector<AccessibilityHostMsg_NotificationParams> pending_notifications_;
 
   // Our representation of the browser tree.
   BrowserTreeNode* browser_root_;
diff --git a/webkit/glue/webaccessibility.cc b/webkit/glue/webaccessibility.cc
index 42c2d4d..b6b8f4c 100644
--- a/webkit/glue/webaccessibility.cc
+++ b/webkit/glue/webaccessibility.cc
@@ -25,19 +25,11 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebVector.h"
 
-#ifndef NDEBUG
-#include "third_party/WebKit/Source/WebKit/chromium/public/WebAccessibilityNotification.h"
-#endif
-
 using base::DoubleToString;
 using base::IntToString;
 using WebKit::WebAccessibilityRole;
 using WebKit::WebAccessibilityObject;
 
-#ifndef NDEBUG
-using WebKit::WebAccessibilityNotification;
-#endif
-
 namespace {
 
 std::string IntVectorToString(const std::vector<int>& items) {
@@ -340,73 +332,9 @@ WebAccessibility::~WebAccessibility() {
 }
 
 #ifndef NDEBUG
-std::string WebAccessibility::DebugString(bool recursive,
-                                          int render_routing_id,
-                                          int notification) const {
+std::string WebAccessibility::DebugString(bool recursive) const {
   std::string result;
   static int indent = 0;
-
-  if (render_routing_id != 0) {
-    WebKit::WebAccessibilityNotification notification_type =
-        static_cast<WebKit::WebAccessibilityNotification>(notification);
-    result += "routing id=";
-    result += IntToString(render_routing_id);
-    result += " notification=";
-
-    switch (notification_type) {
-      case WebKit::WebAccessibilityNotificationActiveDescendantChanged:
-        result += "active descendant changed";
-        break;
-      case WebKit::WebAccessibilityNotificationCheckedStateChanged:
-        result += "check state changed";
-        break;
-      case WebKit::WebAccessibilityNotificationChildrenChanged:
-        result += "children changed";
-        break;
-      case WebKit::WebAccessibilityNotificationFocusedUIElementChanged:
-        result += "focus changed";
-        break;
-      case WebKit::WebAccessibilityNotificationLayoutComplete:
-        result += "layout complete";
-        break;
-      case WebKit::WebAccessibilityNotificationLiveRegionChanged:
-        result += "live region changed";
-        break;
-      case WebKit::WebAccessibilityNotificationLoadComplete:
-        result += "load complete";
-        break;
-      case WebKit::WebAccessibilityNotificationMenuListValueChanged:
-        result += "menu list changed";
-        break;
-      case WebKit::WebAccessibilityNotificationRowCountChanged:
-        result += "row count changed";
-        break;
-      case WebKit::WebAccessibilityNotificationRowCollapsed:
-        result += "row collapsed";
-        break;
-      case WebKit::WebAccessibilityNotificationRowExpanded:
-        result += "row expanded";
-        break;
-      case WebKit::WebAccessibilityNotificationScrolledToAnchor:
-        result += "scrolled to anchor";
-        break;
-      case WebKit::WebAccessibilityNotificationSelectedChildrenChanged:
-        result += "selected children changed";
-        break;
-      case WebKit::WebAccessibilityNotificationSelectedTextChanged:
-        result += "selected text changed";
-        break;
-      case WebKit::WebAccessibilityNotificationValueChanged:
-        result += "value changed";
-        break;
-      case WebKit::WebAccessibilityNotificationInvalid:
-        result += "invalid notification";
-        break;
-      default:
-        NOTREACHED();
-    }
-  }
-
   result += "\n";
   for (int i = 0; i < indent; ++i)
     result += "  ";
@@ -763,7 +691,7 @@ std::string WebAccessibility::DebugString(bool recursive,
     result += "\n";
     ++indent;
     for (size_t i = 0; i < children.size(); ++i)
-      result += children[i].DebugString(true, 0, 0);
+      result += children[i].DebugString(true);
     --indent;
   }
 
diff --git a/webkit/glue/webaccessibility.h b/webkit/glue/webaccessibility.h
index 2b5f928..fd4362e 100644
--- a/webkit/glue/webaccessibility.h
+++ b/webkit/glue/webaccessibility.h
@@ -264,9 +264,7 @@ struct WEBKIT_GLUE_EXPORT WebAccessibility {
             bool include_children);
 
 #ifndef NDEBUG
-  std::string DebugString(bool recursive,
-                          int render_routing_id,
-                          int notification_type) const;
+  std::string DebugString(bool recursive) const;
 #endif
 
  private: