Added battery level and time to the status tray's accessible name.

chrome/browser/ui/views/accessibility/accessibility_event_router_views.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/accessibility/accessibility_event_router_views.h"
 
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/memory/singleton.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/accessibility/accessibility_extension_api.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "content/public/browser/notification_service.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/controls/tree/tree_view.h"
 #include "ui/views/focus/view_storage.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 using views::FocusManager;
 
 AccessibilityEventRouterViews::AccessibilityEventRouterViews()
-    : most_recent_profile_(NULL) {
+    : most_recent_profile_(NULL),
+      most_recent_view_id_(
+          views::ViewStorage::GetInstance()->CreateStorageID()) {
   // Register for notification when profile is destroyed to ensure that all
   // observers are detatched at that time.
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
                  content::NotificationService::AllSources());
 }
 
 AccessibilityEventRouterViews::~AccessibilityEventRouterViews() {
 }
 
 // static
@@ skipping 116 matching lines @@
   most_recent_profile_ = profile;
 
   if (type == ui::AX_EVENT_MENU_START ||
       type == ui::AX_EVENT_MENU_POPUP_START ||
       type == ui::AX_EVENT_MENU_END ||
       type == ui::AX_EVENT_MENU_POPUP_END) {
     SendMenuNotification(view, type, profile);
     return;
   }
 
+  // If this view is not focusable for accessibility, find the closest
+  // ancestor that is, so that the correct accessible name is being read.
+  views::View* temp_view = view;
+  while (temp_view->parent() && !view->IsAccessibilityFocusable()) {
+    temp_view = temp_view->parent();
+    if (temp_view->IsAccessibilityFocusable())
+      view = temp_view;

[sky, 2014/07/14 15:57:19]

Don't you want to break here?

[evy, 2014/07/14 16:03:47]

Well, I have to check it in the while condition in case the original view is
already focusable, and the while condition should cause it to break on the next
iteration. 

Do you think it would be more clear to have the explicit break? If so, I can add
it.

[dmazzoni, 2014/07/14 17:46:00]

It may not be an actual problem in this case since IsAccessibilityFocusable is a
quick function call, but I'd say it's usually better style to write a loop that
doesn't test the loop condition twice.

[sky, 2014/07/14 21:11:14]

Are you saying you want:

while (temp_view && !temp_view->IsAF())
  temp_view = temp_view->parent();
if (temp_view)
  view = temp_view;

For clarity (and ease of testing) you could move to a standalone function so
that this code becomes something like:
view = FindFirstAccessibleAncestor(view);

?

[evy, 2014/07/14 22:19:16]

Yeah, sorry this function is way more complicated than it needs to be because I
originally wrote it for the highest accessible view and it's still structured
for that.
It's only a three line function, but I think it's nice to have it in its own
function still.

+  }
+
+  // Since multiple items could share a highest focusable view, these items
+  // could all dispatch the same accessibility hover events, which isn't
+  // necessary.
+  if (type == ui::AX_EVENT_HOVER &&
+      views::ViewStorage::GetInstance()->RetrieveView(most_recent_view_id_) ==

[dmazzoni, 2014/07/14 17:46:00]

Optional: add "using views::ViewStorage;" at the top of the file and replace
views::ViewStorage with just ViewStorage, will make this slightly more readable
- especially since we're using it in two different places within this file.

[evy, 2014/07/14 22:19:16]

Done.

+          view)
+    return;

[dmazzoni, 2014/07/14 17:46:00]

Nit: { } around return because conditional is multi-line

[Daniel Erat, 2014/07/14 18:04:14]

i don't feel strongly about this one way or another, but the rule that i think
i've typically heard (and then repeated myself) is "curly brackets if the
statement _body_ is multi-line, but not if only the condition is"

[evy, 2014/07/14 22:19:16]

I've been taught that it's nice to have curly braces just in case you add
something later and forget to add the braces - so I think I'll add them.

+  views::ViewStorage::GetInstance()->StoreView(most_recent_view_id_, view);
+
   ui::AXViewState state;
   view->GetAccessibleState(&state);
 
   if (type == ui::AX_EVENT_ALERT &&
       !(state.role == ui::AX_ROLE_ALERT ||
         state.role == ui::AX_ROLE_WINDOW)) {
     SendAlertControlNotification(view, type, profile);
     return;
   }
 
@@ skipping 369 matching lines @@
     return base::UTF16ToUTF8(state.name);
 
   for (int i = 0; i < view->child_count(); ++i) {
     views::View* child = view->child_at(i);
     std::string result = RecursiveGetStaticText(child);
     if (!result.empty())
       return result;
   }
   return std::string();
 }