diff options
8 files changed, 53 insertions, 74 deletions
diff --git a/chrome/browser/extensions/extension_tabs_apitest.cc b/chrome/browser/extensions/extension_tabs_apitest.cc index 9a9ce46..be3f795 100644 --- a/chrome/browser/extensions/extension_tabs_apitest.cc +++ b/chrome/browser/extensions/extension_tabs_apitest.cc @@ -13,10 +13,12 @@ // Tabs appears to timeout, or maybe crash on mac. // http://crbug.com/53779 #define MAYBE_Tabs FAILS_Tabs -#else -// It's flaky on win and linux. +#elif defined(OS_WIN) +// It's flaky on win. // http://crbug.com/58269 #define MAYBE_Tabs FLAKY_Tabs +#else +#define MAYBE_Tabs Tabs #endif IN_PROC_BROWSER_TEST_F(ExtensionApiTest, MAYBE_Tabs) { diff --git a/chrome/browser/gtk/accessibility_event_router_gtk.cc b/chrome/browser/gtk/accessibility_event_router_gtk.cc index 2beb476..0de0f91 100644 --- a/chrome/browser/gtk/accessibility_event_router_gtk.cc +++ b/chrome/browser/gtk/accessibility_event_router_gtk.cc @@ -287,25 +287,24 @@ void AccessibilityEventRouterGtk::RemoveRootWidget(GtkWidget* root_widget) { } } -void AccessibilityEventRouterGtk::IgnoreWidget(GtkWidget* widget) { - widget_info_map_[widget].ignore = true; -} - -void AccessibilityEventRouterGtk::SetWidgetName( +void AccessibilityEventRouterGtk::AddWidgetNameOverride( GtkWidget* widget, std::string name) { widget_info_map_[widget].name = name; + widget_info_map_[widget].refcount++; } -void AccessibilityEventRouterGtk::RemoveWidget(GtkWidget* widget) { +void AccessibilityEventRouterGtk::RemoveWidgetNameOverride(GtkWidget* widget) { DCHECK(widget_info_map_.find(widget) != widget_info_map_.end()); - widget_info_map_.erase(widget); + widget_info_map_[widget].refcount--; + if (widget_info_map_[widget].refcount == 0) { + widget_info_map_.erase(widget); + } } void AccessibilityEventRouterGtk::FindWidget( GtkWidget* widget, Profile** profile, bool* is_accessible) { *is_accessible = false; - // First see if it's a descendant of a root widget. for (base::hash_map<GtkWidget*, RootWidgetInfo>::const_iterator iter = root_widget_info_map_.begin(); iter != root_widget_info_map_.end(); @@ -317,16 +316,6 @@ void AccessibilityEventRouterGtk::FindWidget( break; } } - if (!*is_accessible) - return; - - // Now make sure it's not marked as a widget to be ignored. - base::hash_map<GtkWidget*, WidgetInfo>::const_iterator iter = - widget_info_map_.find(widget); - if (iter != widget_info_map_.end() && iter->second.ignore) { - *is_accessible = false; - return; - } } std::string AccessibilityEventRouterGtk::GetWidgetName(GtkWidget* widget) { diff --git a/chrome/browser/gtk/accessibility_event_router_gtk.h b/chrome/browser/gtk/accessibility_event_router_gtk.h index 4e5f368..2cc2cf9 100644 --- a/chrome/browser/gtk/accessibility_event_router_gtk.h +++ b/chrome/browser/gtk/accessibility_event_router_gtk.h @@ -63,13 +63,15 @@ class AccessibilityEventRouterGtk { // Internal information about a particular widget to override the // information we get directly from gtk. struct WidgetInfo { - WidgetInfo() : ignore(false) { } + WidgetInfo() : refcount(0) { } + + // The number of times that AddWidgetNameOverride has been called on this + // widget. When RemoveWidget has been called an equal number of + // times and the refcount reaches zero, this entry will be deleted. + int refcount; // If nonempty, will use this name instead of the widget's label. std::string name; - - // If true, will ignore this widget and not send accessibility events. - bool ignore; }; // Internal information about a root widget @@ -92,21 +94,23 @@ class AccessibilityEventRouterGtk { // Start sending accessibility events for this widget and all of its // descendants. Notifications will go to the specified profile. + // Uses reference counting, so it's safe to call this twice on the + // same widget, as long as each call is paired with a call to + // RemoveRootWidget. void AddRootWidget(GtkWidget* root_widget, Profile* profile); // Stop sending accessibility events for this widget and all of its // descendants. void RemoveRootWidget(GtkWidget* root_widget); - // Don't send any events for this widget. - void IgnoreWidget(GtkWidget* widget); - // Use the following string as the name of this widget, instead of the - // gtk label associated with the widget. - void SetWidgetName(GtkWidget* widget, std::string name); + // gtk label associated with the widget. Must be paired with a call to + // RemoveWidgetNameOverride. + void AddWidgetNameOverride(GtkWidget* widget, std::string name); - // Forget all information about this widget. - void RemoveWidget(GtkWidget* widget); + // Forget widget name override. Must be paired with a call to + // AddWidgetNameOverride (uses reference counting). + void RemoveWidgetNameOverride(GtkWidget* widget); // // The following methods are only for use by gtk signal handlers. diff --git a/chrome/browser/gtk/accessible_widget_helper_gtk.cc b/chrome/browser/gtk/accessible_widget_helper_gtk.cc index ede764f..6a25d94 100644 --- a/chrome/browser/gtk/accessible_widget_helper_gtk.cc +++ b/chrome/browser/gtk/accessible_widget_helper_gtk.cc @@ -29,8 +29,10 @@ AccessibleWidgetHelper::~AccessibleWidgetHelper() { if (root_widget_) accessibility_event_router_->RemoveRootWidget(root_widget_); - for (unsigned int i = 0; i < managed_widgets_.size(); i++) { - accessibility_event_router_->RemoveWidget(managed_widgets_[i]); + for (std::set<GtkWidget*>::iterator it = managed_widgets_.begin(); + it != managed_widgets_.end(); + ++it) { + accessibility_event_router_->RemoveWidgetNameOverride(*it); } } @@ -44,20 +46,21 @@ void AccessibleWidgetHelper::SendOpenWindowNotification( Details<AccessibilityWindowInfo>(&info)); } -void AccessibleWidgetHelper::IgnoreWidget(GtkWidget* widget) { - accessibility_event_router_->IgnoreWidget(widget); - managed_widgets_.push_back(widget); -} - void AccessibleWidgetHelper::SetWidgetName( GtkWidget* widget, std::string name) { - accessibility_event_router_->SetWidgetName(widget, name); - managed_widgets_.push_back(widget); + if (managed_widgets_.find(widget) != managed_widgets_.end()) { + // AccessibilityEventRouterGtk reference-counts its widgets, but we + // don't. In order to avoid a memory leak, tell the event router + // to deref first, so the resulting refcount is unchanged after we + // call SetWidgetName. + accessibility_event_router_->RemoveWidgetNameOverride(widget); + } + + accessibility_event_router_->AddWidgetNameOverride(widget, name); + managed_widgets_.insert(widget); } void AccessibleWidgetHelper::SetWidgetName( GtkWidget* widget, int string_id) { - std::string name = l10n_util::GetStringUTF8(string_id); - accessibility_event_router_->SetWidgetName(widget, name); - managed_widgets_.push_back(widget); + SetWidgetName(widget, l10n_util::GetStringUTF8(string_id)); } diff --git a/chrome/browser/gtk/accessible_widget_helper_gtk.h b/chrome/browser/gtk/accessible_widget_helper_gtk.h index 3713dd9..2b06fac 100644 --- a/chrome/browser/gtk/accessible_widget_helper_gtk.h +++ b/chrome/browser/gtk/accessible_widget_helper_gtk.h @@ -8,8 +8,8 @@ #include <gtk/gtk.h> +#include <set> #include <string> -#include <vector> #include "base/basictypes.h" #include "base/singleton.h" @@ -28,8 +28,7 @@ class Profile; // events for all of its descendants. // // Most controls have default behavior for accessibility; when this needs -// to be augmented, call one of the methods below to ignore a particular -// widget or change its details. +// to be augmented, call one of the methods below to change its details. // // All of the information managed by this class is registered with the // (global) AccessibilityEventRouterGtk and unregistered when this object is @@ -48,9 +47,6 @@ class AccessibleWidgetHelper { // goes out of scope. void SendOpenWindowNotification(const std::string& window_title); - // Do not send accessibility events for this widget - void IgnoreWidget(GtkWidget* widget); - // Use the following string as the name of this widget, instead of the // gtk label associated with the widget. void SetWidgetName(GtkWidget* widget, std::string name); @@ -64,7 +60,7 @@ class AccessibleWidgetHelper { Profile* profile_; GtkWidget* root_widget_; std::string window_title_; - std::vector<GtkWidget*> managed_widgets_; + std::set<GtkWidget*> managed_widgets_; }; #endif // CHROME_BROWSER_GTK_ACCESSIBLE_WIDGET_HELPER_GTK_H_ diff --git a/chrome/browser/views/accessibility_event_router_views_unittest.cc b/chrome/browser/views/accessibility_event_router_views_unittest.cc index 700499d..7e51ab2 100644 --- a/chrome/browser/views/accessibility_event_router_views_unittest.cc +++ b/chrome/browser/views/accessibility_event_router_views_unittest.cc @@ -106,6 +106,7 @@ TEST_F(AccessibilityEventRouterViewsTest, TestFocusNotification) { const char kButton1ASCII[] = "Button1"; const char kButton2ASCII[] = "Button2"; const char kButton3ASCII[] = "Button3"; + const char kButton3NewASCII[] = "Button3"; // Create a window and layout. views::Widget* window = CreateWidget(); @@ -148,9 +149,8 @@ TEST_F(AccessibilityEventRouterViewsTest, TestFocusNotification) { // Create an AccessibleViewHelper for this window, which will send // accessibility notifications for all events that happen in child views. - // Tell it to ignore button 3. AccessibleViewHelper accessible_view_helper(root_view, &profile); - accessible_view_helper.IgnoreView(button3); + accessible_view_helper.SetViewName(button3, std::string(kButton3NewASCII)); // Advance focus to the next button and test that we got the // expected notification with the name of button 2. @@ -160,14 +160,14 @@ TEST_F(AccessibilityEventRouterViewsTest, TestFocusNotification) { EXPECT_EQ(1, focus_event_count_); EXPECT_EQ(kButton2ASCII, last_control_name_); - // Advance to button 3. We expect no notification because we told it - // to ignore this view. + // Advance to button 3. Expect the new accessible name we assigned. focus_manager->AdvanceFocus(false); - EXPECT_EQ(1, focus_event_count_); + EXPECT_EQ(2, focus_event_count_); + EXPECT_EQ(kButton3NewASCII, last_control_name_); // Advance to button 1 and check the notification. focus_manager->AdvanceFocus(false); - EXPECT_EQ(2, focus_event_count_); + EXPECT_EQ(3, focus_event_count_); EXPECT_EQ(kButton1ASCII, last_control_name_); } diff --git a/chrome/browser/views/accessible_view_helper.cc b/chrome/browser/views/accessible_view_helper.cc index aa86ef9..65caf36 100644 --- a/chrome/browser/views/accessible_view_helper.cc +++ b/chrome/browser/views/accessible_view_helper.cc @@ -39,8 +39,10 @@ AccessibleViewHelper::~AccessibleViewHelper() { if (view_tree_) { accessibility_event_router_->RemoveViewTree(view_tree_); for (std::vector<views::View*>::iterator iter = managed_views_.begin(); - iter != managed_views_.end(); ++iter) + iter != managed_views_.end(); + ++iter) { accessibility_event_router_->RemoveView(*iter); + } } } @@ -54,20 +56,6 @@ void AccessibleViewHelper::SendOpenWindowNotification( Details<AccessibilityWindowInfo>(&info)); } -void AccessibleViewHelper::IgnoreView(views::View* view) { - if (!view_tree_) - return; - - accessibility_event_router_->IgnoreView(view); - managed_views_.push_back(view); - - #if defined(OS_LINUX) - gfx::NativeView native_view = GetNativeView(view); - if (native_view) - widget_helper_->IgnoreWidget(native_view); - #endif -} - void AccessibleViewHelper::SetViewName(views::View* view, std::string name) { if (!view_tree_) return; diff --git a/chrome/browser/views/accessible_view_helper.h b/chrome/browser/views/accessible_view_helper.h index e3a0f93..3309c0f 100644 --- a/chrome/browser/views/accessible_view_helper.h +++ b/chrome/browser/views/accessible_view_helper.h @@ -55,9 +55,6 @@ class AccessibleViewHelper { // goes out of scope. void SendOpenWindowNotification(const std::string& window_title); - // Will not send accessibility events for this view. - void IgnoreView(views::View* view); - // Uses the following string as the name of this view, instead of // view->GetAccessibleName(). void SetViewName(views::View* view, std::string name); |