Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(709)

Issue 132243004: Revert of Enable all ESC-Fullscreen tests (Chrome Apps) for all platforms. (Closed)

Created:
6 years, 11 months ago by alph
Modified:
6 years, 11 months ago
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -70 lines) Patch
M chrome/browser/apps/app_window_interactive_uitest.cc View 10 chunks +31 lines, -58 lines 0 comments Download
M chrome/test/data/extensions/platform_apps/leave_fullscreen/main.js View 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/test/data/extensions/platform_apps/prevent_leave_fullscreen/main.js View 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
alph
Created Revert of Enable all ESC-Fullscreen tests (Chrome Apps) for all platforms.
6 years, 11 months ago (2014-01-09 19:39:50 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alph@chromium.org/132243004/1
6 years, 11 months ago (2014-01-09 19:42:21 UTC) #2
commit-bot: I haz the power
6 years, 11 months ago (2014-01-09 19:42:24 UTC) #3
Failed to apply patch for chrome/browser/apps/app_window_interactive_uitest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/apps/app_window_interactive_uitest.cc
  Hunk #1 FAILED at 7.
  Hunk #2 FAILED at 18.
  Hunk #3 FAILED at 59.
  Hunk #4 FAILED at 92.
  Hunk #5 FAILED at 109.
  Hunk #6 FAILED at 131.
  Hunk #7 FAILED at 155.
  Hunk #8 FAILED at 175.
  Hunk #9 FAILED at 198.
  Hunk #10 FAILED at 220.
  10 out of 10 hunks FAILED -- saving rejects to file
chrome/browser/apps/app_window_interactive_uitest.cc.rej

Patch:       chrome/browser/apps/app_window_interactive_uitest.cc
Index: chrome/browser/apps/app_window_interactive_uitest.cc
diff --git a/chrome/browser/apps/app_window_interactive_uitest.cc
b/chrome/browser/apps/app_window_interactive_uitest.cc
index
c6adbcd7d2c51d02d2b2d354345c43e6d73f1d2e..b44bf1412ce31988ac1687e48fdf313e05859cda
100644
--- a/chrome/browser/apps/app_window_interactive_uitest.cc
+++ b/chrome/browser/apps/app_window_interactive_uitest.cc
@@ -7,7 +7,7 @@
 #include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/test/base/interactive_test_utils.h"
 
-using apps::NativeAppWindow;
+using namespace apps;
 
 // Helper class that has to be created in the stack to check if the fullscreen
 // setting of a NativeWindow has changed since the creation of the object.
@@ -18,7 +18,7 @@
         initial_fullscreen_state_(window_->IsFullscreen()) {}
 
   void Wait() {
-    while (initial_fullscreen_state_ == window_->IsFullscreen())
+    while (initial_fullscreen_state_ != window_->IsFullscreen())
       content::RunAllPendingInMessageLoop();
   }
 
@@ -59,26 +59,37 @@
       false,
       false);
   }
-
-  // This method will wait until the application is able to ack a key event.
-  void WaitUntilKeyFocus() {
-    ExtensionTestMessageListener key_listener("KeyReceived", false);
-
-    while (!key_listener.was_satisfied()) {
-      ASSERT_TRUE(SimulateKeyPress(ui::VKEY_Z));
-      content::RunAllPendingInMessageLoop();
-    }
-  }
 };
 
-IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest, ESCLeavesFullscreenWindow) {
+#if defined(OS_LINUX) && defined(USE_AURA)
+// These tests do not work on Linux Aura because when the window is raised, the
+// content is not focused thus do not get the key events.
+// See http://crbug.com/324346
+#define MAYBE_ESCDoesNotLeaveFullscreenWindow \
+  DISABLED_ESCDoesNotLeaveFullscreenWindow
+#define MAYBE_ESCDoesNotLeaveFullscreenDOM
DISABLED_ESCDoesNotLeaveFullscreenDOM
+// These tests are failing on Linux Aura for unknown reasons.
+#define MAYBE_ESCLeavesFullscreenWindow DISABLED_ESCLeavesFullscreenWindow
+#define MAYBE_ESCLeavesFullscreenDOM DISABLED_ESCLeavesFullscreenDOM
+#elif defined(OS_MACOSX)
+// These tests are highly flaky on MacOS.
+#define MAYBE_ESCLeavesFullscreenWindow DISABLED_ESCLeavesFullscreenWindow
+#define MAYBE_ESCLeavesFullscreenDOM DISABLED_ESCLeavesFullscreenDOM
+#define MAYBE_ESCDoesNotLeaveFullscreenWindow \
+  DISABLED_ESCDoesNotLeaveFullscreenWindow
+#define MAYBE_ESCDoesNotLeaveFullscreenDOM
DISABLED_ESCDoesNotLeaveFullscreenDOM
+#else
+#define MAYBE_ESCLeavesFullscreenWindow ESCLeavesFullscreenWindow
+#define MAYBE_ESCLeavesFullscreenDOM ESCLeavesFullscreenDOM
+#define MAYBE_ESCDoesNotLeaveFullscreenWindow ESCDoesNotLeaveFullscreenWindow
+#define MAYBE_ESCDoesNotLeaveFullscreenDOM ESCDoesNotLeaveFullscreenDOM
+#endif
+
+IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest,
+                       MAYBE_ESCLeavesFullscreenWindow) {
   ExtensionTestMessageListener launched_listener("Launched", true);
   LoadAndLaunchPlatformApp("leave_fullscreen");
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
-
-  // We start by making sure the window is actually focused.
-  ASSERT_TRUE(ui_test_utils::ShowAndFocusNativeWindow(
-                  GetFirstShellWindow()->GetNativeWindow()));
 
   // When receiving the reply, the application will try to go fullscreen using
   // the Window API but there is no synchronous way to know if that actually
@@ -92,12 +103,6 @@
     fs_changed.Wait();
   }
 
-  // Depending on the platform, going fullscreen might create an animation.
-  // We want to make sure that the ESC key we will send next is actually going
-  // to be received and the application might not receive key events during the
-  // animation so we should wait for the key focus to be back.
-  WaitUntilKeyFocus();
-
   // Same idea as above but for leaving fullscreen. Fullscreen mode should be
   // left when ESC is received.
   {
@@ -109,14 +114,10 @@
   }
 }
 
-IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest, ESCLeavesFullscreenDOM) {
+IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest, MAYBE_ESCLeavesFullscreenDOM)
{
   ExtensionTestMessageListener launched_listener("Launched", true);
   LoadAndLaunchPlatformApp("leave_fullscreen");
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
-
-  // We start by making sure the window is actually focused.
-  ASSERT_TRUE(ui_test_utils::ShowAndFocusNativeWindow(
-                GetFirstShellWindow()->GetNativeWindow()));
 
   launched_listener.Reply("dom");
 
@@ -131,17 +132,10 @@
   {
     FullscreenChangeWaiter fs_changed(GetFirstShellWindow()->GetBaseWindow());
 
-    WaitUntilKeyFocus();
     ASSERT_TRUE(SimulateKeyPress(ui::VKEY_A));
 
     fs_changed.Wait();
   }
-
-  // Depending on the platform, going fullscreen might create an animation.
-  // We want to make sure that the ESC key we will send next is actually going
-  // to be received and the application might not receive key events during the
-  // animation so we should wait for the key focus to be back.
-  WaitUntilKeyFocus();
 
   // Same idea as above but for leaving fullscreen. Fullscreen mode should be
   // left when ESC is received.
@@ -155,14 +149,10 @@
 }
 
 IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest,
-                       ESCDoesNotLeaveFullscreenWindow) {
+                       MAYBE_ESCDoesNotLeaveFullscreenWindow) {
   ExtensionTestMessageListener launched_listener("Launched", true);
   LoadAndLaunchPlatformApp("prevent_leave_fullscreen");
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
-
-  // We start by making sure the window is actually focused.
-  ASSERT_TRUE(ui_test_utils::ShowAndFocusNativeWindow(
-                GetFirstShellWindow()->GetNativeWindow()));
 
   // When receiving the reply, the application will try to go fullscreen using
   // the Window API but there is no synchronous way to know if that actually
@@ -175,12 +165,6 @@
 
     fs_changed.Wait();
   }
-
-  // Depending on the platform, going fullscreen might create an animation.
-  // We want to make sure that the ESC key we will send next is actually going
-  // to be received and the application might not receive key events during the
-  // animation so we should wait for the key focus to be back.
-  WaitUntilKeyFocus();
 
   ASSERT_TRUE(SimulateKeyPress(ui::VKEY_ESCAPE));
 
@@ -198,14 +182,10 @@
 }
 
 IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest,
-                       ESCDoesNotLeaveFullscreenDOM) {
+                       MAYBE_ESCDoesNotLeaveFullscreenDOM) {
   ExtensionTestMessageListener launched_listener("Launched", true);
   LoadAndLaunchPlatformApp("prevent_leave_fullscreen");
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
-
-  // We start by making sure the window is actually focused.
-  ASSERT_TRUE(ui_test_utils::ShowAndFocusNativeWindow(
-                GetFirstShellWindow()->GetNativeWindow()));
 
   launched_listener.Reply("dom");
 
@@ -220,17 +200,10 @@
   {
     FullscreenChangeWaiter fs_changed(GetFirstShellWindow()->GetBaseWindow());
 
-    WaitUntilKeyFocus();
     ASSERT_TRUE(SimulateKeyPress(ui::VKEY_A));
 
     fs_changed.Wait();
   }
-
-  // Depending on the platform, going fullscreen might create an animation.
-  // We want to make sure that the ESC key we will send next is actually going
-  // to be received and the application might not receive key events during the
-  // animation so we should wait for the key focus to be back.
-  WaitUntilKeyFocus();
 
   ASSERT_TRUE(SimulateKeyPress(ui::VKEY_ESCAPE));

Powered by Google App Engine
This is Rietveld 408576698