Only block alert() requests from blocked popups; not all popups.

Add two unit tests to make sure we do the right thing; required
adding a bunch of stuff to the automation layer.

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

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

4 files changed, 96 insertions, 86 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 8e572ab..b9bd059 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -801,6 +801,8 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) {
                         WaitForNavigation)
     IPC_MESSAGE_HANDLER(AutomationMsg_SetIntPreferenceRequest,
                         SetIntPreference)
+    IPC_MESSAGE_HANDLER(AutomationMsg_ShowingAppModalDialogRequest,
+                        GetShowingAppModalDialog)
   IPC_END_MESSAGE_MAP()
 }
 
@@ -1063,6 +1065,12 @@ void AutomationProvider::GetBrowserWindowCount(const IPC::Message& message) {
       message.routing_id(), static_cast<int>(BrowserList::size())));
 }
 
+void AutomationProvider::GetShowingAppModalDialog(const IPC::Message& message) {
+  Send(new AutomationMsg_ShowingAppModalDialogResponse(
+           message.routing_id(),
+           static_cast<bool>(BrowserList::IsShowingAppModalDialog())));
+}
+
 void AutomationProvider::GetBrowserWindow(const IPC::Message& message,
                                           int index) {
   int handle = 0;
diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h
index aaeb5bc..cb9ff00 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -105,6 +105,7 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
                  const std::string value,
                  int handle);
   void GetBrowserWindowCount(const IPC::Message& message);
+  void GetShowingAppModalDialog(const IPC::Message& message);
   void GetBrowserWindow(const IPC::Message& message, int index);
   void GetLastActiveBrowserWindow(const IPC::Message& message);
   void GetActiveWindow(const IPC::Message& message);
@@ -411,4 +412,3 @@ class TestingAutomationProvider : public AutomationProvider,
   void OnRemoveProvider();  // Called via PostTask
 };
 #endif  // CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_H_
-
diff --git a/chrome/browser/views/constrained_window_impl_interactive_uitest.cc b/chrome/browser/views/constrained_window_impl_interactive_uitest.cc
index e528f1d..3e6dc88 100644
--- a/chrome/browser/views/constrained_window_impl_interactive_uitest.cc
+++ b/chrome/browser/views/constrained_window_impl_interactive_uitest.cc
@@ -19,43 +19,53 @@
 
 #include "generated_resources.h"
 
-const int kRightCloseButtonOffset = 55;
-const int kBottomCloseButtonOffset = 20;
-
 class InteractiveConstrainedWindowTest : public UITest {
  protected:
   InteractiveConstrainedWindowTest() {
     show_window_ = true;
   }
-};
 
-TEST_F(InteractiveConstrainedWindowTest, TestOpenAndResizeTo) {
-  scoped_ptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
-  ASSERT_TRUE(browser.get());
+  virtual void SetUp() {
+    UITest::SetUp();
 
-  scoped_ptr<WindowProxy> window(
-      automation()->GetWindowForBrowser(browser.get()));
-  ASSERT_TRUE(window.get());
+    browser_.reset(automation()->GetBrowserWindow(0));
+    ASSERT_TRUE(browser_.get());
 
-  scoped_ptr<TabProxy> tab(browser->GetTab(0));
-  ASSERT_TRUE(tab.get());
+    window_.reset(automation()->GetWindowForBrowser(browser_.get()));
+    ASSERT_TRUE(window_.get());
 
-  std::wstring filename(test_data_directory_);
-  file_util::AppendToPath(&filename, L"constrained_files");
-  file_util::AppendToPath(&filename,
-                          L"constrained_window_onload_resizeto.html");
-  ASSERT_TRUE(tab->NavigateToURL(net::FilePathToFileURL(filename)));
+    tab_.reset(browser_->GetTab(0));
+    ASSERT_TRUE(tab_.get());
+  }
 
-  gfx::Rect tab_view_bounds;
-  ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER,
-                                    &tab_view_bounds, true));
+  void NavigateMainTabTo(const std::wstring& file_name) {
+    std::wstring filename(test_data_directory_);
+    file_util::AppendToPath(&filename, L"constrained_files");
+    file_util::AppendToPath(&filename, file_name);
+    ASSERT_TRUE(tab_->NavigateToURL(net::FilePathToFileURL(filename)));
+  }
 
-  // Simulate a click of the actual link to force user_gesture to be
-  // true; if we don't, the resulting popup will be constrained, which
-  // isn't what we want to test.
-  POINT link_point(tab_view_bounds.CenterPoint().ToPOINT());
-  ASSERT_TRUE(window->SimulateOSClick(link_point,
-                                      views::Event::EF_LEFT_BUTTON_DOWN));
+  void SimulateClickInCenterOf(const scoped_ptr<WindowProxy>& window) {
+    gfx::Rect tab_view_bounds;
+    ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER,
+                                      &tab_view_bounds, true));
+
+    // Simulate a click of the actual link to force user_gesture to be
+    // true; if we don't, the resulting popup will be constrained, which
+    // isn't what we want to test.
+    POINT link_point(tab_view_bounds.CenterPoint().ToPOINT());
+    ASSERT_TRUE(window->SimulateOSClick(link_point,
+                                        views::Event::EF_LEFT_BUTTON_DOWN));
+  }
+
+  scoped_ptr<BrowserProxy> browser_;
+  scoped_ptr<WindowProxy> window_;
+  scoped_ptr<TabProxy> tab_;
+};
+
+TEST_F(InteractiveConstrainedWindowTest, TestOpenAndResizeTo) {
+  NavigateMainTabTo(L"constrained_window_onload_resizeto.html");
+  SimulateClickInCenterOf(window_);
 
   ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, 1000));
 
@@ -74,12 +84,7 @@ TEST_F(InteractiveConstrainedWindowTest, TestOpenAndResizeTo) {
   ASSERT_EQ(300, rect.width());
   ASSERT_EQ(320, rect.height());
 
-  // Send a click to the popup window to test resizeTo.
-  ASSERT_TRUE(popup_window->GetViewBounds(VIEW_ID_TAB_CONTAINER,
-                                          &tab_view_bounds, true));
-  POINT popup_link_point(tab_view_bounds.CenterPoint().ToPOINT());
-  ASSERT_TRUE(popup_window->SimulateOSClick(
-                  popup_link_point, views::Event::EF_LEFT_BUTTON_DOWN));
+  SimulateClickInCenterOf(popup_window);
 
   // No idea how to wait here other then sleeping. This timeout used to be
   // lower, then we started hitting it before it was done. :(
@@ -120,32 +125,8 @@ bool ParseCountOutOfTitle(const std::wstring& title, int* output)
 // Tests that in the window.open() equivalent of a fork bomb, we stop building
 // windows.
 TEST_F(InteractiveConstrainedWindowTest, DontSpawnEndlessPopups) {
-  scoped_ptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
-  ASSERT_TRUE(browser.get());
-
-  scoped_ptr<WindowProxy> window(
-      automation()->GetWindowForBrowser(browser.get()));
-  ASSERT_TRUE(window.get());
-
-  scoped_ptr<TabProxy> tab(browser->GetTab(0));
-  ASSERT_TRUE(tab.get());
-
-  std::wstring filename(test_data_directory_);
-  file_util::AppendToPath(&filename, L"constrained_files");
-  file_util::AppendToPath(&filename,
-                          L"infinite_popups.html");
-  ASSERT_TRUE(tab->NavigateToURL(net::FilePathToFileURL(filename)));
-
-  gfx::Rect tab_view_bounds;
-  ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER,
-                                    &tab_view_bounds, true));
-
-  // Simulate a click of the actual link to force user_gesture to be
-  // true; if we don't, the resulting popup will be constrained, which
-  // isn't what we want to test.
-  POINT link_point(tab_view_bounds.CenterPoint().ToPOINT());
-  ASSERT_TRUE(window->SimulateOSClick(link_point,
-                                      views::Event::EF_LEFT_BUTTON_DOWN));
+  NavigateMainTabTo(L"infinite_popups.html");
+  SimulateClickInCenterOf(window_);
 
   ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, 1000));
 
@@ -194,33 +175,11 @@ TEST_F(InteractiveConstrainedWindowTest, DontSpawnEndlessPopups) {
   }
 }
 
-// Make sure that we refuse to close windows when a constrained popup is displayed.
+// Make sure that we refuse to close windows when a constrained popup is
+// displayed.
 TEST_F(InteractiveConstrainedWindowTest, WindowOpenWindowClosePopup) {
-  scoped_ptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
-  ASSERT_TRUE(browser.get());
-
-  scoped_ptr<WindowProxy> window(
-      automation()->GetWindowForBrowser(browser.get()));
-  ASSERT_TRUE(window.get());
-
-  scoped_ptr<TabProxy> tab(browser->GetTab(0));
-  ASSERT_TRUE(tab.get());
-
-  std::wstring filename(test_data_directory_);
-  file_util::AppendToPath(&filename, L"constrained_files");
-  file_util::AppendToPath(&filename, L"openclose_main.html");
-  ASSERT_TRUE(tab->NavigateToURL(net::FilePathToFileURL(filename)));
-
-  gfx::Rect tab_view_bounds;
-  ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER,
-                                    &tab_view_bounds, true));
-
-  // Simulate a click of the actual link to force user_gesture to be
-  // true; if we don't, the resulting popup will be constrained, which
-  // isn't what we want to test.
-  POINT link_point(tab_view_bounds.CenterPoint().ToPOINT());
-  ASSERT_TRUE(window->SimulateOSClick(link_point,
-                                      views::Event::EF_LEFT_BUTTON_DOWN));
+  NavigateMainTabTo(L"openclose_main.html");
+  SimulateClickInCenterOf(window_);
 
   ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, 5000));
 
@@ -246,3 +205,45 @@ TEST_F(InteractiveConstrainedWindowTest, WindowOpenWindowClosePopup) {
   // Ensure we didn't close the first popup window.
   ASSERT_FALSE(automation()->WaitForWindowCountToBecome(1, 3000));
 }
+
+TEST_F(InteractiveConstrainedWindowTest, BlockAlertFromBlockedPopup) {
+  NavigateMainTabTo(L"block_alert.html");
+
+  // Wait for there to be an app modal dialog (and fail if it's shown).
+  ASSERT_FALSE(automation()->WaitForAppModalDialog(4000));
+
+  // Ensure one browser window.
+  int browser_window_count;
+  ASSERT_TRUE(automation()->GetBrowserWindowCount(&browser_window_count));
+  ASSERT_EQ(1, browser_window_count);
+
+  // Ensure one blocked popup window: the popup didn't escape.
+  scoped_ptr<ConstrainedWindowProxy> popup_notification(
+      tab_->GetConstrainedWindow(0));
+  ASSERT_TRUE(popup_notification.get());
+  std::wstring title;
+  ASSERT_TRUE(popup_notification->GetTitle(&title));
+  int popup_count = 0;
+  ASSERT_TRUE(ParseCountOutOfTitle(title, &popup_count));
+  ASSERT_EQ(1, popup_count);
+}
+
+TEST_F(InteractiveConstrainedWindowTest, ShowAlertFromNormalPopup) {
+  NavigateMainTabTo(L"show_alert.html");
+  SimulateClickInCenterOf(window_);
+
+  ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2, 5000));
+
+  scoped_ptr<BrowserProxy> popup_browser(automation()->GetBrowserWindow(1));
+  ASSERT_TRUE(popup_browser.get());
+  scoped_ptr<WindowProxy> popup_window(
+      automation()->GetWindowForBrowser(popup_browser.get()));
+  ASSERT_TRUE(popup_window.get());
+  scoped_ptr<TabProxy> popup_tab(popup_browser->GetTab(0));
+  ASSERT_TRUE(popup_tab.get());
+
+  SimulateClickInCenterOf(popup_window);
+
+  // Wait for there to be an app modal dialog.
+  ASSERT_TRUE(automation()->WaitForAppModalDialog(5000));
+}
diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc
index acd9338..e936aac 100644
--- a/chrome/browser/web_contents.cc
+++ b/chrome/browser/web_contents.cc
@@ -1082,7 +1082,8 @@ void WebContents::RunJavaScriptMessage(
   // constrained window jail).
   bool suppress_this_message = suppress_javascript_messages_;
   if (delegate())
-    suppress_this_message |= delegate()->IsPopup(this);
+    suppress_this_message |=
+        (delegate()->GetConstrainingContents(this) != NULL);
 
   *did_suppress_message = suppress_this_message;