Add a test for the mouseleave event on window.

chrome/browser/mouseleave_browsertest.cc

Index: chrome/browser/mouseleave_browsertest.cc
diff --git a/chrome/browser/mouseleave_browsertest.cc b/chrome/browser/mouseleave_browsertest.cc
index 416b081f112b51b7f0acaec195cefb78fef5393a..fae133b336b53a3fd2aaf983db5caf998bbcaae6 100644
--- a/chrome/browser/mouseleave_browsertest.cc
+++ b/chrome/browser/mouseleave_browsertest.cc
@@ -4,11 +4,17 @@
 
 #include "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/renderer_context_menu/render_view_context_menu.h"
+#include "chrome/browser/renderer_context_menu/render_view_context_menu_browsertest_util.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/app_modal/app_modal_dialog.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "ui/base/test/ui_controls.h"
@@ -19,24 +25,20 @@ class MouseLeaveTest : public InProcessBrowserTest {
  public:
   MouseLeaveTest() {}
 
-  void MouseLeaveTestCommon() {
+  void LoadTestPageAndWaitForMouseOver(content::WebContents* tab) {

[sky, 2015/03/12 15:37:37]

This should either return a boolean that it's successful, or callers should wrap
calls to it in EXPECT_NO_FATAL_FAILURE.

[Miyoung Shin, 2015/03/18 16:16:51]

Done.

+    gfx::Rect tab_view_bounds = tab->GetContainerBounds();
     GURL test_url = ui_test_utils::GetTestUrl(
         base::FilePath(), base::FilePath(FILE_PATH_LITERAL("mouseleave.html")));
 
-    content::WebContents* tab =
-        browser()->tab_strip_model()->GetActiveWebContents();
-    gfx::Rect tab_view_bounds = tab->GetContainerBounds();
-
-    gfx::Point in_content_point(
-        tab_view_bounds.x() + tab_view_bounds.width() / 2,
-        tab_view_bounds.y() + 10);
-    gfx::Point above_content_point(
-        tab_view_bounds.x() + tab_view_bounds.width() / 2,
-        tab_view_bounds.y() - 2);
+    gfx::Point in_content =
+        gfx::Point(tab_view_bounds.x() + tab_view_bounds.width() / 2,

[sky, 2015/03/12 15:37:37]

nit: use constructor that takes x,y.

[Miyoung Shin, 2015/03/18 16:16:51]

Done.

+                   tab_view_bounds.y() + 10);
+    above_content_ =
+        gfx::Point(tab_view_bounds.x() + tab_view_bounds.width() / 2,
+                   tab_view_bounds.y() - 2);
 
     // Start by moving the point just above the content.
-    ui_controls::SendMouseMove(above_content_point.x(),
-                               above_content_point.y());
+    ui_controls::SendMouseMove(above_content_.x(), above_content_.y());
 
     // Navigate to the test html page.
     base::string16 load_expected_title(base::ASCIIToUTF16("onload"));
@@ -46,19 +48,25 @@ class MouseLeaveTest : public InProcessBrowserTest {
     // next part of the test.
     EXPECT_EQ(load_expected_title, load_title_watcher.WaitAndGetTitle());
 
-    // Move the cursor to the top-center of the content, which will trigger
+    // Move the cursor to the top-center of the content which will trigger
     // a javascript onMouseOver event.
-    ui_controls::SendMouseMove(in_content_point.x(), in_content_point.y());
+    ui_controls::SendMouseMove(in_content.x(), in_content.y());
 
     // Wait on the correct intermediate title.
     base::string16 entered_expected_title(base::ASCIIToUTF16("entered"));
     content::TitleWatcher entered_title_watcher(tab, entered_expected_title);
     EXPECT_EQ(entered_expected_title, entered_title_watcher.WaitAndGetTitle());
+  }
+
+  void MouseLeaveTestCommon() {
+    content::WebContents* tab =
+        browser()->tab_strip_model()->GetActiveWebContents();
+
+    LoadTestPageAndWaitForMouseOver(tab);
 
     // Move the cursor above the content again, which should trigger
     // a javascript onMouseOut event.
-    ui_controls::SendMouseMove(above_content_point.x(),
-                               above_content_point.y());
+    ui_controls::SendMouseMove(above_content_.x(), above_content_.y());
 
     // Wait on the correct final value of the cookie.
     base::string16 left_expected_title(base::ASCIIToUTF16("left"));
@@ -66,6 +74,8 @@ class MouseLeaveTest : public InProcessBrowserTest {
     EXPECT_EQ(left_expected_title, left_title_watcher.WaitAndGetTitle());
   }
 
+  gfx::Point above_content_;

[sky, 2015/03/12 15:37:37]

Add a description.

[Miyoung Shin, 2015/03/18 16:16:51]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(MouseLeaveTest);
 };
 
@@ -95,4 +105,102 @@ IN_PROC_BROWSER_TEST_F(MouseLeaveTest, MouseDownOnBrowserCaption) {
 }
 #endif
 
+#if defined(OS_WIN)

[pkotwicz, 2015/03/12 16:51:57]

I think the MouseLeaveTest.NewWindow should be removed. Right now it is not
possible to write a correct test for a mouseleave getting dispatched when a new
window is opened. In particular, this test passes even if the code which creates
the new window is removed.

For more context see comments #29 and #37

[Miyoung Shin, 2015/03/18 16:16:51]

Done.

+// Test that a mouse leave event is triggered when opening a new window.
+#define MAYBE_NewWindow NewWindow
+#else
+// Others except OS_WIN do not send the mouseleave when opening the new window
+// TODO: Make test pass on the other OSes
+// See http://crbug.com/458492
+#define MAYBE_NewWindow DISABLED_NewWindow
+#endif
+
+IN_PROC_BROWSER_TEST_F(MouseLeaveTest, MAYBE_NewWindow) {
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  LoadTestPageAndWaitForMouseOver(tab);
+
+  // Create an new window.

[sky, 2015/03/12 15:37:37]

an->a

[Miyoung Shin, 2015/03/18 16:16:51]

Done. I removed this TC.

+  ui_test_utils::BrowserAddedObserver browser_added_observer;
+  chrome::NewWindow(browser());
+  browser_added_observer.WaitForSingleNewBrowser();

[sky, 2015/03/12 15:37:37]

Seems like you're assuming the new window is opened where the mouse is. How do
you know that'll happen?

[Miyoung Shin, 2015/03/18 16:16:51]

Done. I removed this TC.

+
+  ASSERT_EQ(2u, chrome::GetTotalBrowserCount());
+
+  content::RunAllPendingInMessageLoop();

[sky, 2015/03/12 15:37:37]

Why do you need the run all pending?

[Miyoung Shin, 2015/03/18 16:16:51]

Done. I removed this TC.

+
+  tab->GetMainFrame()->ExecuteJavaScript(base::ASCIIToUTF16("done()"));
+  const base::string16 success_title = base::ASCIIToUTF16("with mouseleave");
+  const base::string16 failure_title = base::ASCIIToUTF16("without mouseleave");
+  content::TitleWatcher done_title_watcher(tab, success_title);
+  done_title_watcher.AlsoWaitForTitle(failure_title);
+
+  EXPECT_EQ(success_title, done_title_watcher.WaitAndGetTitle());
+}
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+// Test that a mouseleave is not triggered when showing the context menu.
+// TODO: Make test pass on OS_WIN and OS_MACOSX
+// OS_WIN: http://crbug.com/450138
+// OS_MACOSX: Missing automation provider support: http://crbug.com/45892.
+#define MAYBE_ContextMenu DISABLED_ContextMenu
+#else
+#define MAYBE_ContextMenu ContextMenu
+#endif
+
+IN_PROC_BROWSER_TEST_F(MouseLeaveTest, MAYBE_ContextMenu) {
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  LoadTestPageAndWaitForMouseOver(tab);
+
+  ContextMenuWaiter menu_observer(content::NotificationService::AllSources());
+  ui_controls::SendMouseClick(ui_controls::RIGHT);
+  // Wait until the context menu is opened and closed.
+  menu_observer.WaitForMenuOpenAndClose();
+
+  content::RunAllPendingInMessageLoop();

[sky, 2015/03/12 15:37:37]

Same comment about position of context menu and why you need the run all.

[pkotwicz, 2015/03/12 16:51:57]

I suggested adding content::RunAllPendingInMessageLoop();

A test which tests that something does not occur is weird. I suggested adding
content::RunAllPendingInMessageLoop() because it gives more opportunities for
the test to fail. Calling content::RunAllPendingInMessageLoop() ensures that any
pending synthetic mouse moves get dispatched.

[sky, 2015/03/12 19:54:12]

Isn't the WaitAndGetTitle below effectively doing the waiting?

If you're saying we need to wait for mouse events before getting to 165 then the
code should do that rather than assuming RunAllPendingInMessageLoop is enough.

[pkotwicz, 2015/03/12 21:44:14]

I checked and on ChromeOS there aren't any delayed synthetic mouse moves sent to
the web contents after the context menu / JS alert has opened. I am hoping that
this test will stay valuable even if the logic which sends synthetic mouse
events in WindowEventDispatcher changes.

WaitAndGetTitle() just waits for the title set by the done() Javascript function
to be set.

[sky, 2015/03/12 22:34:06]

Certainly, but aren't those set based on mouse events, which is the whole point
of the test?
I'm still not seeing why the RunAll is needed.

[Miyoung Shin, 2015/03/16 04:34:26]

sky@, pkotwicz@,
RunAll is called in WaitForMenuOpenAndClose as well.
What do you think of removing it here ?

[Miyoung Shin, 2015/03/18 16:16:51]

Done. I removed to call content::RunAllPendingInMessageLoop().

+
+  tab->GetMainFrame()->ExecuteJavaScript(base::ASCIIToUTF16("done()"));
+  const base::string16 success_title = base::ASCIIToUTF16("without mouseleave");
+  const base::string16 failure_title = base::ASCIIToUTF16("with mouseleave");
+  content::TitleWatcher done_title_watcher(tab, success_title);
+  done_title_watcher.AlsoWaitForTitle(failure_title);
+
+  EXPECT_EQ(success_title, done_title_watcher.WaitAndGetTitle());
+}
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+// Test that a mouseleave is not triggered when showing a modal dialog.
+// Sample regression: crbug.com/394672
+// TODO: Make test pass on OS_WIN and OS_MACOSX
+// OS_WIN: http://crbug.com/450138
+// OS_MACOSX: Missing automation provider support: http://crbug.com/45892.
+#define MAYBE_ModalDialog DISABLED_ModalDialog
+#else
+#define MAYBE_ModalDialog ModalDialog
+#endif
+
+IN_PROC_BROWSER_TEST_F(MouseLeaveTest, MAYBE_ModalDialog) {
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  LoadTestPageAndWaitForMouseOver(tab);
+
+  tab->GetMainFrame()->ExecuteJavaScript(base::UTF8ToUTF16("alert()"));
+  app_modal::AppModalDialog* alert = ui_test_utils::WaitForAppModalDialog();
+  // Cancel the dialog.
+  alert->CloseModalDialog();
+
+  content::RunAllPendingInMessageLoop();

[sky, 2015/03/12 15:37:37]

And same comment here.

[Miyoung Shin, 2015/03/16 04:34:26]

This is not set based on mouse events, so I think we can remove RunAll as well.

[Miyoung Shin, 2015/03/18 16:16:51]

Done. I removed to call content::RunAllPendingInMessageLoop()

+
+  tab->GetMainFrame()->ExecuteJavaScript(base::ASCIIToUTF16("done()"));
+  const base::string16 success_title = base::ASCIIToUTF16("without mouseleave");
+  const base::string16 failure_title = base::ASCIIToUTF16("with mouseleave");
+  content::TitleWatcher done_title_watcher(tab, success_title);
+  done_title_watcher.AlsoWaitForTitle(failure_title);
+  EXPECT_EQ(success_title, done_title_watcher.WaitAndGetTitle());
+}
+
 }  // namespace