Allow nested calls to AccessibilityEventRouterGtk::AddRootWidget and

AccessibilityEventRouterGtk::RemoveRootWidget via reference counting,
which stops ExtensionApiTest, FLAKY_Tabs from crashing on chromeos.

BUG=56479
TEST=ExtensionApiTest.FLAKY_Tabs no longer crashes, and added new AccessibilityEventRouterGtkTest.

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

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

4 files changed, 76 insertions, 15 deletions

diff --git a/chrome/browser/extensions/extension_tabs_apitest.cc b/chrome/browser/extensions/extension_tabs_apitest.cc
index 793376d..d8c1bc3 100644
--- a/chrome/browser/extensions/extension_tabs_apitest.cc
+++ b/chrome/browser/extensions/extension_tabs_apitest.cc
@@ -9,13 +9,9 @@
 #include "chrome/browser/profile.h"
 #include "chrome/common/pref_names.h"
 
-// Tabs started crashing on CrOS and hanging browser tests
-// http://crbug.com/56479
-#if defined(OS_CHROMEOS)
-#define MAYBE_Tabs DISABLED_Tabs
 // Tabs is flaky on chromeos, windows, linux views and linux dbg.
 // http://crbug.com/48920
-#elif defined(OS_LINUX) || defined(OS_WIN)
+#if defined(OS_LINUX) || defined(OS_WIN) || defined(OS_CHROMEOS)
 #define MAYBE_Tabs FLAKY_Tabs
 #elif defined(OS_MACOSX)
 // Tabs appears to timeout, or maybe crash on mac.
diff --git a/chrome/browser/gtk/accessibility_event_router_gtk.cc b/chrome/browser/gtk/accessibility_event_router_gtk.cc
index 6d04cbd..2beb476 100644
--- a/chrome/browser/gtk/accessibility_event_router_gtk.cc
+++ b/chrome/browser/gtk/accessibility_event_router_gtk.cc
@@ -274,13 +274,17 @@ void AccessibilityEventRouterGtk::RemoveEventListeners() {
 
 void AccessibilityEventRouterGtk::AddRootWidget(
     GtkWidget* root_widget, Profile* profile) {
-  root_widget_profile_map_[root_widget] = profile;
+  root_widget_info_map_[root_widget].refcount++;
+  root_widget_info_map_[root_widget].profile = profile;
 }
 
 void AccessibilityEventRouterGtk::RemoveRootWidget(GtkWidget* root_widget) {
-  DCHECK(root_widget_profile_map_.find(root_widget) !=
-         root_widget_profile_map_.end());
-  root_widget_profile_map_.erase(root_widget);
+  DCHECK(root_widget_info_map_.find(root_widget) !=
+         root_widget_info_map_.end());
+  root_widget_info_map_[root_widget].refcount--;
+  if (root_widget_info_map_[root_widget].refcount == 0) {
+    root_widget_info_map_.erase(root_widget);
+  }
 }
 
 void AccessibilityEventRouterGtk::IgnoreWidget(GtkWidget* widget) {
@@ -302,14 +306,14 @@ void AccessibilityEventRouterGtk::FindWidget(
   *is_accessible = false;
 
   // First see if it's a descendant of a root widget.
-  for (base::hash_map<GtkWidget*, Profile*>::const_iterator iter =
-           root_widget_profile_map_.begin();
-       iter != root_widget_profile_map_.end();
+  for (base::hash_map<GtkWidget*, RootWidgetInfo>::const_iterator iter =
+           root_widget_info_map_.begin();
+       iter != root_widget_info_map_.end();
        ++iter) {
-    if (gtk_widget_is_ancestor(widget, iter->first)) {
+    if (widget == iter->first || gtk_widget_is_ancestor(widget, iter->first)) {
       *is_accessible = true;
       if (profile)
-        *profile = iter->second;
+        *profile = iter->second.profile;
       break;
     }
   }
diff --git a/chrome/browser/gtk/accessibility_event_router_gtk.h b/chrome/browser/gtk/accessibility_event_router_gtk.h
index e260937..4e5f368 100644
--- a/chrome/browser/gtk/accessibility_event_router_gtk.h
+++ b/chrome/browser/gtk/accessibility_event_router_gtk.h
@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/gtest_prod_util.h"
 #include "base/hash_tables.h"
 #include "base/singleton.h"
 #include "base/task.h"
@@ -71,6 +72,21 @@ class AccessibilityEventRouterGtk {
     bool ignore;
   };
 
+  // Internal information about a root widget
+  struct RootWidgetInfo {
+    RootWidgetInfo() : refcount(0), profile(NULL) { }
+
+    // The number of times that AddRootWidget has been called on this
+    // widget. When RemoveRootWidget has been called an equal number of
+    // times and the refcount reaches zero, this entry will be deleted.
+    int refcount;
+
+    // The profile associated with this root widget; accessibility
+    // notifications for any descendant of this root widget will get routed
+    // to this profile.
+    Profile* profile;
+  };
+
   // Get the single instance of this class.
   static AccessibilityEventRouterGtk* GetInstance();
 
@@ -158,7 +174,7 @@ class AccessibilityEventRouterGtk {
 
   // The set of all root widgets; only descendants of these will generate
   // accessibility notifications.
-  base::hash_map<GtkWidget*, Profile*> root_widget_profile_map_;
+  base::hash_map<GtkWidget*, RootWidgetInfo> root_widget_info_map_;
 
   // Extra information about specific widgets.
   base::hash_map<GtkWidget*, WidgetInfo> widget_info_map_;
@@ -180,6 +196,9 @@ class AccessibilityEventRouterGtk {
   // Used to schedule invocations of StartListening() and to defer handling
   // of some events until the next time through the event loop.
   ScopedRunnableMethodFactory<AccessibilityEventRouterGtk> method_factory_;
+
+  friend class AccessibilityEventRouterGtkTest;
+  FRIEND_TEST_ALL_PREFIXES(AccessibilityEventRouterGtkTest, AddRootWidgetTwice);
 };
 
 #endif  // CHROME_BROWSER_GTK_ACCESSIBILITY_EVENT_ROUTER_GTK_H_
diff --git a/chrome/browser/gtk/accessibility_event_router_gtk_unittest.cc b/chrome/browser/gtk/accessibility_event_router_gtk_unittest.cc
new file mode 100644
index 0000000..7264da7
--- /dev/null
+++ b/chrome/browser/gtk/accessibility_event_router_gtk_unittest.cc
@@ -0,0 +1,42 @@
+// Copyright (c) 2010 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/gtk/accessibility_event_router_gtk.h"
+#include "chrome/test/testing_profile.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class AccessibilityEventRouterGtkTest : public testing::Test {
+ protected:
+  AccessibilityEventRouterGtkTest() { }
+};
+
+TEST_F(AccessibilityEventRouterGtkTest, AddRootWidgetTwice) {
+  AccessibilityEventRouterGtk* event_router =
+      AccessibilityEventRouterGtk::GetInstance();
+  TestingProfile profile;
+
+  GtkWidget* widget = gtk_window_new(GTK_WINDOW_TOPLEVEL);
+
+  bool found = false;
+  event_router->FindWidget(widget, NULL, &found);
+  EXPECT_FALSE(found);
+
+  event_router->AddRootWidget(widget, &profile);
+  event_router->FindWidget(widget, NULL, &found);
+  EXPECT_TRUE(found);
+
+  event_router->AddRootWidget(widget, &profile);
+  event_router->FindWidget(widget, NULL, &found);
+  EXPECT_TRUE(found);
+
+  event_router->RemoveRootWidget(widget);
+  event_router->FindWidget(widget, NULL, &found);
+  EXPECT_TRUE(found);
+
+  event_router->RemoveRootWidget(widget);
+  event_router->FindWidget(widget, NULL, &found);
+  EXPECT_FALSE(found);
+
+  gtk_widget_destroy(widget);
+};