Icons are loaded asynchronously for browser actions, so the

test must wait on the extension being updated if no icon is
found. 

I think this should fix the flakiness in the test.

BUG=None
TEST=Fixing a browser test (hopefully).

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

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

5 files changed, 38 insertions, 6 deletions

diff --git a/chrome/browser/extensions/browser_action_test_util.h b/chrome/browser/extensions/browser_action_test_util.h
index df3e2cf..ab04013 100644
--- a/chrome/browser/extensions/browser_action_test_util.h
+++ b/chrome/browser/extensions/browser_action_test_util.h
@@ -24,7 +24,12 @@ class BrowserActionTestUtil {
   // Returns the number of browser action currently visible.
   int VisibleBrowserActions();
 
-  // Returns whether the browser action at |index| has a non-null icon.
+  // Waits for a browser action at |index| to get updated.
+  void WaitForBrowserActionUpdated(int index);
+
+  // Returns whether the browser action at |index| has a non-null icon. Note
+  // that the icon is loaded asynchronously, in which case you can wait for it
+  // to load by calling WaitForBrowserActionUpdated.
   bool HasIcon(int index);
 
   // Simulates a user click on the browser action button at |index|.
diff --git a/chrome/browser/extensions/browser_action_test_util_views.cc b/chrome/browser/extensions/browser_action_test_util_views.cc
index 69b034c..4d94ce3 100644
--- a/chrome/browser/extensions/browser_action_test_util_views.cc
+++ b/chrome/browser/extensions/browser_action_test_util_views.cc
@@ -34,6 +34,12 @@ int BrowserActionTestUtil::VisibleBrowserActions() {
   return GetContainer(browser_)->VisibleBrowserActions();
 }
 
+void BrowserActionTestUtil::WaitForBrowserActionUpdated(int index) {
+  ui_test_utils::WaitForBrowserActionUpdated(
+      GetContainer(browser_)->GetBrowserActionViewAt(index)->
+          button()->extension()->browser_action());
+}
+
 bool BrowserActionTestUtil::HasIcon(int index) {
   return !GetContainer(browser_)->GetBrowserActionViewAt(index)->
       button()->icon().empty();
diff --git a/chrome/browser/views/browser_actions_container_unittest.cc b/chrome/browser/views/browser_actions_container_unittest.cc
index b8b6a062..c8a4c72 100644
--- a/chrome/browser/views/browser_actions_container_unittest.cc
+++ b/chrome/browser/views/browser_actions_container_unittest.cc
@@ -23,6 +23,16 @@ class BrowserActionsContainerTest : public ExtensionBrowserTest {
     return browser_actions_bar_.get();
   }
 
+  // Make sure extension with index |extension_index| has an icon.
+  void EnsureExtensionHasIcon(int extension_index) {
+    if (!browser_actions_bar_->HasIcon(extension_index)) {
+      // The icon is loaded asynchronously and a notification is then sent to
+      // observers. So we wait on it.
+      browser_actions_bar_->WaitForBrowserActionUpdated(extension_index);
+    }
+    EXPECT_TRUE(browser_actions_bar()->HasIcon(extension_index));
+  }
+
  private:
   scoped_ptr<BrowserActionTestUtil> browser_actions_bar_;
 };
@@ -46,7 +56,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsContainerTest, Basic) {
                                           .AppendASCII("browser_action")
                                           .AppendASCII("basics")));
   EXPECT_EQ(1, browser_actions_bar()->NumberOfBrowserActions());
-  EXPECT_TRUE(browser_actions_bar()->HasIcon(0));
+  EnsureExtensionHasIcon(0);
 
   // Unload the extension.
   std::string id = browser_actions_bar()->GetExtensionId(0);
@@ -65,7 +75,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsContainerTest, Visibility) {
                                           .AppendASCII("browser_action")
                                           .AppendASCII("basics")));
   EXPECT_EQ(1, browser_actions_bar()->NumberOfBrowserActions());
-  EXPECT_TRUE(browser_actions_bar()->HasIcon(0));
+  EnsureExtensionHasIcon(0);
   EXPECT_EQ(1, browser_actions_bar()->VisibleBrowserActions());
   std::string idA = browser_actions_bar()->GetExtensionId(0);
 
@@ -74,7 +84,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsContainerTest, Visibility) {
                                           .AppendASCII("browser_action")
                                           .AppendASCII("add_popup")));
   EXPECT_EQ(2, browser_actions_bar()->NumberOfBrowserActions());
-  EXPECT_TRUE(browser_actions_bar()->HasIcon(1));
+  EnsureExtensionHasIcon(0);
   EXPECT_EQ(2, browser_actions_bar()->VisibleBrowserActions());
   std::string idB = browser_actions_bar()->GetExtensionId(1);
 
@@ -85,7 +95,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsContainerTest, Visibility) {
                                           .AppendASCII("browser_action")
                                           .AppendASCII("remove_popup")));
   EXPECT_EQ(3, browser_actions_bar()->NumberOfBrowserActions());
-  EXPECT_TRUE(browser_actions_bar()->HasIcon(2));
+  EnsureExtensionHasIcon(2);
   EXPECT_EQ(3, browser_actions_bar()->VisibleBrowserActions());
   std::string idC = browser_actions_bar()->GetExtensionId(2);
 
diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index fb1ec65..4ea680f 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_service.h"
 #if defined(TOOLKIT_VIEWS)
@@ -419,6 +420,12 @@ void WaitForTabParented() {
   TabParentedNotificationObserver new_tab_observer;
 }
 
+void WaitForBrowserActionUpdated(ExtensionAction* browser_action) {
+  SimpleNotificationObserver<ExtensionAction>
+      observer(NotificationType::EXTENSION_BROWSER_ACTION_UPDATED,
+               browser_action);
+}
+
 void WaitForLoadStop(NavigationController* controller) {
   SimpleNotificationObserver<NavigationController>
       new_tab_observer(NotificationType::LOAD_STOP, controller);
diff --git a/chrome/test/ui_test_utils.h b/chrome/test/ui_test_utils.h
index 462a9ec..e466610 100644
--- a/chrome/test/ui_test_utils.h
+++ b/chrome/test/ui_test_utils.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -22,6 +22,7 @@ class AppModalDialog;
 class Browser;
 class CommandLine;
 class DownloadManager;
+class ExtensionAction;
 class FilePath;
 class GURL;
 class MessageLoop;
@@ -67,6 +68,9 @@ void WaitForNewTab(Browser* browser);
 // Waits for a tab to be parented.
 void WaitForTabParented();
 
+// Waits for a |browser_action| to be updated.
+void WaitForBrowserActionUpdated(ExtensionAction* browser_action);
+
 // Waits for a load stop for the specified |controller|.
 void WaitForLoadStop(NavigationController* controller);