Consolidate notifiers used by different automation

IPC messages.
Make OpenIncognito synchronuous.

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

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

6 files changed, 199 insertions, 66 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 9e87744..fe442ba 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -415,16 +415,25 @@ class TabClosedNotificationObserver : public TabStripNotificationObserver {
                 NotificationType::TAB_CLOSING,
             automation,
             routing_id),
-         reply_message_(reply_message) {
+         reply_message_(reply_message),
+         for_browser_command_(false) {
   }
 
   virtual void ObserveTab(NavigationController* controller) {
-    AutomationMsg_CloseTab::WriteReplyParams(reply_message_, true);
+    if (for_browser_command_)
+      AutomationMsg_WindowExecuteCommand::WriteReplyParams(reply_message_,
+                                                           true);
+    else
+      AutomationMsg_CloseTab::WriteReplyParams(reply_message_, true);
     automation_->Send(reply_message_);
   }
 
+  void set_for_browser_command(bool for_browser_command) {
+    for_browser_command_ = for_browser_command;
+  }
  protected:
   IPC::Message* reply_message_;
+  bool for_browser_command_;
 };
 
 class BrowserOpenedNotificationObserver : public NotificationObserver {
@@ -432,7 +441,8 @@ class BrowserOpenedNotificationObserver : public NotificationObserver {
   BrowserOpenedNotificationObserver(AutomationProvider* automation,
                                     IPC::Message* reply_message)
       : automation_(automation),
-        reply_message_(reply_message) {
+        reply_message_(reply_message),
+        for_browser_command_(false) {
     registrar_.Add(this, NotificationType::BROWSER_OPENED,
                    NotificationService::AllSources());
   }
@@ -444,6 +454,9 @@ class BrowserOpenedNotificationObserver : public NotificationObserver {
                        const NotificationSource& source,
                        const NotificationDetails& details) {
     if (type == NotificationType::BROWSER_OPENED) {
+      if (for_browser_command_)
+        AutomationMsg_WindowExecuteCommand::WriteReplyParams(reply_message_,
+                                                             true);
       automation_->Send(reply_message_);
       delete this;
     } else {
@@ -451,10 +464,14 @@ class BrowserOpenedNotificationObserver : public NotificationObserver {
     }
   }
 
+  void set_for_browser_command(bool for_browser_command) {
+    for_browser_command_ = for_browser_command;
+  }
  private:
   AutomationProvider* automation_;
   IPC::Message* reply_message_;
   NotificationRegistrar registrar_;
+  bool for_browser_command_;
 };
 
 class BrowserClosedNotificationObserver : public NotificationObserver {
@@ -465,7 +482,8 @@ class BrowserClosedNotificationObserver : public NotificationObserver {
                                     IPC::Message* reply_message)
       : automation_(automation),
         routing_id_(routing_id),
-        reply_message_(reply_message) {
+        reply_message_(reply_message),
+        for_browser_command_(false) {
     NotificationService::current()->AddObserver(this,
         NotificationType::BROWSER_CLOSED, Source<Browser>(browser));
   }
@@ -476,17 +494,25 @@ class BrowserClosedNotificationObserver : public NotificationObserver {
     DCHECK(type == NotificationType::BROWSER_CLOSED);
     Details<bool> close_app(details);
     DCHECK(reply_message_ != NULL);
-    AutomationMsg_CloseBrowser::WriteReplyParams(reply_message_, true,
-                                                 *(close_app.ptr()));
+    if (for_browser_command_)
+      AutomationMsg_WindowExecuteCommand::WriteReplyParams(reply_message_,
+                                                           true);
+    else
+      AutomationMsg_CloseBrowser::WriteReplyParams(reply_message_, true,
+                                                   *(close_app.ptr()));
     automation_->Send(reply_message_);
     reply_message_ = NULL;
     delete this;
   }
 
+  void set_for_browser_command(bool for_browser_command) {
+    for_browser_command_ = for_browser_command;
+  }
  private:
   AutomationProvider* automation_;
   int32 routing_id_;
   IPC::Message* reply_message_;
+  bool for_browser_command_;
 };
 
 namespace {
@@ -500,6 +526,8 @@ struct CommandNotification {
 const struct CommandNotification command_notifications[] = {
   {IDC_DUPLICATE_TAB, NotificationType::TAB_PARENTED},
   {IDC_NEW_TAB, NotificationType::TAB_PARENTED},
+  // Returns as soon as the restored tab is created. To further wait until
+  // the content page is loaded, use WaitForTabToBeRestored.
   {IDC_RESTORE_TAB, NotificationType::TAB_PARENTED}
 };
 
@@ -507,21 +535,50 @@ const struct CommandNotification command_notifications[] = {
 
 class ExecuteBrowserCommandObserver : public NotificationObserver {
  public:
-  ExecuteBrowserCommandObserver(AutomationProvider* automation,
-                                IPC::Message* reply_message)
-      : automation_(automation),
-        reply_message_(reply_message) {
-  }
-
   ~ExecuteBrowserCommandObserver() {
   }
 
-  bool Register(int command) {
-    if (!GetNotificationType(command, &notification_type_))
-      return false;
-    registrar_.Add(this, notification_type_,
-                   NotificationService::AllSources());
-    return true;
+  static bool CreateAndRegisterObserver(AutomationProvider* automation,
+                                        Browser* browser,
+                                        int command,
+                                        IPC::Message* reply_message) {
+    bool result = true;
+    switch (command) {
+      case IDC_NEW_WINDOW:
+      case IDC_NEW_INCOGNITO_WINDOW: {
+        BrowserOpenedNotificationObserver* observer =
+            new BrowserOpenedNotificationObserver(automation,
+                                                  reply_message);
+        observer->set_for_browser_command(true);
+        break;
+      }
+      case IDC_CLOSE_WINDOW: {
+        BrowserClosedNotificationObserver* observer =
+            new BrowserClosedNotificationObserver(browser, automation,
+                                                  reply_message->routing_id(),
+                                                  reply_message);
+        observer->set_for_browser_command(true);
+        break;
+      }
+      case IDC_CLOSE_TAB: {
+        TabClosedNotificationObserver* observer =
+            new TabClosedNotificationObserver(browser, automation,
+                                              reply_message->routing_id(),
+                                              true, reply_message);
+        observer->set_for_browser_command(true);
+        break;
+      }
+      default: {
+        ExecuteBrowserCommandObserver* observer =
+            new ExecuteBrowserCommandObserver(automation, reply_message);
+        if (!observer->Register(command)) {
+          delete observer;
+          result = false;
+        }
+        break;
+      }
+    }
+    return result;
   }
 
   virtual void Observe(NotificationType type,
@@ -538,6 +595,20 @@ class ExecuteBrowserCommandObserver : public NotificationObserver {
   }
 
  private:
+  ExecuteBrowserCommandObserver(AutomationProvider* automation,
+                                IPC::Message* reply_message)
+      : automation_(automation),
+        reply_message_(reply_message) {
+  }
+
+  bool Register(int command) {
+    if (!GetNotificationType(command, &notification_type_))
+      return false;
+    registrar_.Add(this, notification_type_,
+                   NotificationService::AllSources());
+    return true;
+  }
+
   bool GetNotificationType(int command, NotificationType::Type* type) {
     if (!type)
       return false;
@@ -1423,13 +1494,11 @@ void AutomationProvider::ExecuteBrowserCommand(
     Browser* browser = browser_tracker_->GetResource(handle);
     if (browser->command_updater()->SupportsCommand(command) &&
         browser->command_updater()->IsCommandEnabled(command)) {
-      ExecuteBrowserCommandObserver* observer =
-          new ExecuteBrowserCommandObserver(this, reply_message);
-      if (observer->Register(command))
+      if (ExecuteBrowserCommandObserver::CreateAndRegisterObserver(
+          this, browser, command, reply_message)) {
         browser->ExecuteCommand(command);
-      else
-        delete observer;
-      return;
+        return;
+      }
     }
   }
   AutomationMsg_WindowExecuteCommand::WriteReplyParams(reply_message, false);
diff --git a/chrome/test/automated_ui_tests/automated_ui_test_base.cc b/chrome/test/automated_ui_tests/automated_ui_test_base.cc
index 5820275..2a16b9b 100644
--- a/chrome/test/automated_ui_tests/automated_ui_test_base.cc
+++ b/chrome/test/automated_ui_tests/automated_ui_test_base.cc
@@ -39,9 +39,7 @@ bool AutomatedUITestBase::CloseActiveTab() {
   }
 
   if (tab_count > 1) {
-    scoped_ptr<TabProxy> tab(GetActiveTab());
-    // Wait until tab is closed.
-    return tab->Close(true);
+    return RunCommand(IDC_CLOSE_TAB);
   } else if (tab_count == 1) {
     // Synchronously close the window if it is not the last window.
     return CloseActiveWindow();
@@ -77,6 +75,10 @@ bool AutomatedUITestBase::DuplicateTab() {
   return RunCommand(IDC_DUPLICATE_TAB);
 }
 
+bool AutomatedUITestBase::GoOffTheRecord() {
+  return RunCommand(IDC_NEW_INCOGNITO_WINDOW);
+}
+
 bool AutomatedUITestBase::OpenAndActivateNewBrowserWindow(
     BrowserProxy** previous_browser) {
   if (!automation()->OpenNewBrowserWindow(SW_SHOWNORMAL)) {
diff --git a/chrome/test/automated_ui_tests/automated_ui_test_base.h b/chrome/test/automated_ui_tests/automated_ui_test_base.h
index e454bec..80268bc 100644
--- a/chrome/test/automated_ui_tests/automated_ui_test_base.h
+++ b/chrome/test/automated_ui_tests/automated_ui_test_base.h
@@ -38,6 +38,9 @@ class AutomatedUITestBase : public UITest {
   // Returns true if a duplicated tab is added.
   bool DuplicateTab();
 
+  // Opens an OffTheRecord browser window.
+  bool GoOffTheRecord();
+
   // Opens a new tab in the active window using an accelerator.
   // Returns true if a new tab is successfully opened.
   bool NewTab();
diff --git a/chrome/test/automated_ui_tests/automated_ui_test_test.cc b/chrome/test/automated_ui_tests/automated_ui_test_test.cc
index afaa97a..e09996f 100644
--- a/chrome/test/automated_ui_tests/automated_ui_test_test.cc
+++ b/chrome/test/automated_ui_tests/automated_ui_test_test.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/app/chrome_dll_resource.h"
 #include "chrome/test/automated_ui_tests/automated_ui_test_base.h"
 #include "chrome/test/automation/browser_proxy.h"
 #include "chrome/test/automation/tab_proxy.h"
@@ -48,6 +49,50 @@ TEST_F(AutomatedUITestBase, RestoreTab) {
   ASSERT_EQ(2, tab_count);
 }
 
+TEST_F(AutomatedUITestBase, CloseTab) {
+  int num_browser_windows;
+  int tab_count;
+  NewTab();
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(1, num_browser_windows);
+  active_browser()->GetTabCount(&tab_count);
+  ASSERT_EQ(2, tab_count);
+
+  ASSERT_TRUE(OpenAndActivateNewBrowserWindow(NULL));
+  NewTab();
+  NewTab();
+  active_browser()->GetTabCount(&tab_count);
+  ASSERT_EQ(3, tab_count);
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(2, num_browser_windows);
+
+  ASSERT_TRUE(CloseActiveTab());
+  active_browser()->GetTabCount(&tab_count);
+  ASSERT_EQ(2, tab_count);
+  ASSERT_TRUE(CloseActiveTab());
+  active_browser()->GetTabCount(&tab_count);
+  ASSERT_EQ(1, tab_count);
+  num_browser_windows = 0;
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(2, num_browser_windows);
+
+  // The browser window is closed by closing this tab.
+  ASSERT_TRUE(CloseActiveTab());
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(1, num_browser_windows);
+  // Active_browser_ is now the first created window.
+  active_browser()->GetTabCount(&tab_count);
+  ASSERT_EQ(2, tab_count);
+  ASSERT_TRUE(CloseActiveTab());
+  active_browser()->GetTabCount(&tab_count);
+  ASSERT_EQ(1, tab_count);
+
+  // The last tab should not be closed.
+  ASSERT_FALSE(CloseActiveTab());
+  active_browser()->GetTabCount(&tab_count);
+  ASSERT_EQ(1, tab_count);
+}
+
 TEST_F(AutomatedUITestBase, OpenBrowserWindow) {
   int num_browser_windows;
   int tab_count;
@@ -131,46 +176,68 @@ TEST_F(AutomatedUITestBase, CloseBrowserWindow) {
   ASSERT_FALSE(CloseActiveWindow());
 }
 
-TEST_F(AutomatedUITestBase, CloseTab) {
+TEST_F(AutomatedUITestBase, IncognitoWindow) {
   int num_browser_windows;
-  int tab_count;
-  NewTab();
+  int num_normal_browser_windows;
   automation()->GetBrowserWindowCount(&num_browser_windows);
   ASSERT_EQ(1, num_browser_windows);
-  active_browser()->GetTabCount(&tab_count);
-  ASSERT_EQ(2, tab_count);
+  automation()->GetNormalBrowserWindowCount(&num_normal_browser_windows);
+  ASSERT_EQ(1, num_normal_browser_windows);
 
-  ASSERT_TRUE(OpenAndActivateNewBrowserWindow(NULL));
-  NewTab();
-  NewTab();
-  active_browser()->GetTabCount(&tab_count);
-  ASSERT_EQ(3, tab_count);
+  ASSERT_TRUE(GoOffTheRecord());
+  ASSERT_TRUE(GoOffTheRecord());
   automation()->GetBrowserWindowCount(&num_browser_windows);
-  ASSERT_EQ(2, num_browser_windows);
+  ASSERT_EQ(3, num_browser_windows);
+  automation()->GetNormalBrowserWindowCount(&num_normal_browser_windows);
+  ASSERT_EQ(1, num_normal_browser_windows);
 
-  ASSERT_TRUE(CloseActiveTab());
-  active_browser()->GetTabCount(&tab_count);
-  ASSERT_EQ(2, tab_count);
-  ASSERT_TRUE(CloseActiveTab());
-  active_browser()->GetTabCount(&tab_count);
-  ASSERT_EQ(1, tab_count);
-  num_browser_windows = 0;
+  // There is only one normal window so it will not be closed.
+  ASSERT_FALSE(CloseActiveWindow());
   automation()->GetBrowserWindowCount(&num_browser_windows);
-  ASSERT_EQ(2, num_browser_windows);
+  ASSERT_EQ(3, num_browser_windows);
+  automation()->GetNormalBrowserWindowCount(&num_normal_browser_windows);
+  ASSERT_EQ(1, num_normal_browser_windows);
 
-  // The browser window is closed by closing this tab.
-  ASSERT_TRUE(CloseActiveTab());
+  set_active_browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(RunCommand(IDC_CLOSE_WINDOW));
+  set_active_browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(RunCommand(IDC_CLOSE_WINDOW));
   automation()->GetBrowserWindowCount(&num_browser_windows);
   ASSERT_EQ(1, num_browser_windows);
-  // Active_browser_ is now the first created window.
-  active_browser()->GetTabCount(&tab_count);
-  ASSERT_EQ(2, tab_count);
-  ASSERT_TRUE(CloseActiveTab());
-  active_browser()->GetTabCount(&tab_count);
-  ASSERT_EQ(1, tab_count);
+}
 
-  // The last tab should not be closed.
-  ASSERT_FALSE(CloseActiveTab());
-  active_browser()->GetTabCount(&tab_count);
-  ASSERT_EQ(1, tab_count);
+TEST_F(AutomatedUITestBase, OpenCloseBrowserWindowWithAccelerator) {
+  // Note: we don't use RunCommand(IDC_OPEN/CLOSE_WINDOW) to open/close
+  // browser window in automated ui tests. Instead we use
+  // OpenAndActivateNewBrowserWindow and CloseActiveWindow.
+  // There are other parts of UI test that use the accelerators. This is
+  // a unit test for those usage.
+  ASSERT_TRUE(RunCommand(IDC_NEW_WINDOW));
+  int num_browser_windows;
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(2, num_browser_windows);
+  ASSERT_TRUE(RunCommand(IDC_NEW_WINDOW));
+  ASSERT_TRUE(RunCommand(IDC_NEW_WINDOW));
+  ASSERT_TRUE(RunCommand(IDC_NEW_WINDOW));
+  ASSERT_TRUE(RunCommand(IDC_NEW_WINDOW));
+  ASSERT_TRUE(RunCommand(IDC_NEW_WINDOW));
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(7, num_browser_windows);
+
+  set_active_browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(RunCommand(IDC_CLOSE_WINDOW));
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(6, num_browser_windows);
+  set_active_browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(RunCommand(IDC_CLOSE_WINDOW));
+  set_active_browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(RunCommand(IDC_CLOSE_WINDOW));
+  set_active_browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(RunCommand(IDC_CLOSE_WINDOW));
+  set_active_browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(RunCommand(IDC_CLOSE_WINDOW));
+  set_active_browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(RunCommand(IDC_CLOSE_WINDOW));
+  automation()->GetBrowserWindowCount(&num_browser_windows);
+  ASSERT_EQ(1, num_browser_windows);
 }
diff --git a/chrome/test/automated_ui_tests/automated_ui_tests.cc b/chrome/test/automated_ui_tests/automated_ui_tests.cc
index 35d8dca..bd66201 100644
--- a/chrome/test/automated_ui_tests/automated_ui_tests.cc
+++ b/chrome/test/automated_ui_tests/automated_ui_tests.cc
@@ -426,10 +426,6 @@ bool AutomatedUITest::ForwardButton() {
   return RunCommandAsync(IDC_FORWARD);
 }
 
-bool AutomatedUITest::GoOffTheRecord() {
-  return RunCommandAsync(IDC_NEW_INCOGNITO_WINDOW);
-}
-
 bool AutomatedUITest::Home() {
   return RunCommandAsync(IDC_HOME);
 }
diff --git a/chrome/test/automated_ui_tests/automated_ui_tests.h b/chrome/test/automated_ui_tests/automated_ui_tests.h
index c6b847d..a58b2dc 100644
--- a/chrome/test/automated_ui_tests/automated_ui_tests.h
+++ b/chrome/test/automated_ui_tests/automated_ui_tests.h
@@ -154,10 +154,6 @@ class AutomatedUITest : public AutomatedUITestBase {
   // XML element: <Forward/>
   bool ForwardButton();
 
-  // Opens and focuses an OffTheRecord browser window.
-  // XML element: <GoOffTheRecord/>
-  bool GoOffTheRecord();
-
   // Navigates to the Home page.
   // Returns true if call to activate the accelerator is successful.
   // XML element: <Home/>