Bug 1391691 - Make WebDriver:MaximizeWindow idempotent. r=automatedtester

MozReview-Commit-ID: EJ0VQOTWysg

--HG--
extra : rebase_source : 8fa9ab528c30f11d791b0e49f36661957f19befa

testing/marionette/driver.js

@@ -3052,7 +3052,9 @@ GeckoDriver.prototype.minimizeWindow = async function(cmd, resp) {
 
 /**
  * Synchronously maximizes the user agent window as if the user pressed
- * the maximize button, or restores it if it is already maximized.
+ * the maximize button.
+ *
+ * No action is taken if the window is already maximized.
  *
  * Not supported on Fennec.
  *
@@ -3092,48 +3094,44 @@ GeckoDriver.prototype.maximizeWindow = async function(cmd, resp) {
     });
   }
 
-  let modeChangeEv;
-  await new TimedPromise(resolve => {
-    modeChangeEv = resolve;
-    win.addEventListener("sizemodechange", modeChangeEv, {once: true});
-
-    if (win.windowState == win.STATE_MAXIMIZED) {
-      win.restore();
-    } else {
+  let state = WindowState.from(win.windowState);
+  if (state != WindowState.Maximized) {
+    await new TimedPromise(resolve => {
+      win.addEventListener("sizemodechange", resolve, {once: true});
       win.maximize();
-    }
-  }, {throws: null});
-  win.removeEventListener("sizemodechange", modeChangeEv);
+    }, {throws: null});
 
-  // Transitioning into a window state is asynchronous on Linux, and we
-  // cannot rely on sizemodechange to accurately tell us when the
-  // transition has completed.
-  //
-  // To counter for this we wait for the window size to change, which
-  // it usually will.  On platforms where the transition is synchronous,
-  // the wait will have the cost of one iteration because the size will
-  // have changed as part of the transition.  Where the platform
-  // is asynchronous, the cost may be greater as we have to poll
-  // continuously until we see a change, but it ensures conformity in
-  // behaviour.
-  //
-  // Certain window managers, however, do not have a concept of maximised
-  // windows and here sizemodechange may never fire.  Indeed, if the
-  // window covers the maximum available screen real estate, the window
-  // size may also not change.  In this circumstance, which admittedly
-  // is a somewhat bizarre edge case, we assume that the timeout of
-  // waiting for sizemodechange to fire is sufficient to give the window
-  // enough time to transition itself into whatever form or shape the
-  // WM is programmed to give it.
-  await windowSizeChange();
+    // Transitioning into a window state is asynchronous on Linux,
+    // and we cannot rely on sizemodechange to accurately tell us when
+    // the transition has completed.
+    //
+    // To counter for this we wait for the window size to change, which
+    // it usually will.  On platforms where the transition is synchronous,
+    // the wait will have the cost of one iteration because the size
+    // will have changed as part of the transition.  Where the platform is
+    // asynchronous, the cost may be greater as we have to poll
+    // continuously until we see a change, but it ensures conformity in
+    // behaviour.
+    //
+    // Certain window managers, however, do not have a concept of
+    // maximised windows and here sizemodechange may never fire.  Indeed,
+    // if the window covers the maximum available screen real estate,
+    // the window size may also not change.  In this circumstance,
+    // which admittedly is a somewhat bizarre edge case, we assume that
+    // the timeout of waiting for sizemodechange to fire is sufficient
+    // to give the window enough time to transition itself into whatever
+    // form or shape the WM is programmed to give it.
+    await windowSizeChange();
+  }
 
   return this.curBrowser.rect;
 };
 
 /**
  * Synchronously sets the user agent window to full screen as if the user
- * had done "View > Enter Full Screen", or restores it if it is already
- * in full screen.
+ * had done "View > Enter Full Screen".
+ *
+ * No action is taken if the window is already in full screen mode.
  *
  * Not supported on Fennec.
  *

testing/marionette/harness/marionette_harness/tests/unit/test_window_maximize.py

@@ -45,10 +45,6 @@ class TestWindowMaximize(MarionetteTestCase):
                 "current height {current} should be greater than or equal to max height {max}"
                 .format(delta=delta, current=actual["height"], max=self.max["height"] - delta))
 
-    def assert_window_restored(self, actual):
-        self.assertEqual(self.original_size["width"], actual["width"])
-        self.assertEqual(self.original_size["height"], actual["height"])
-
     def assert_window_rect(self, rect):
         self.assertIn("width", rect)
         self.assertIn("height", rect)
@@ -66,19 +62,21 @@ class TestWindowMaximize(MarionetteTestCase):
         self.assertEqual(maximize_resp, window_rect_resp)
         self.assert_window_maximized(maximize_resp)
 
-    def test_maximize_twice_restores(self):
+    def test_maximize_twice_is_idempotent(self):
         maximized = self.marionette.maximize_window()
         self.assert_window_maximized(maximized)
 
-        restored = self.marionette.maximize_window()
-        self.assert_window_restored(restored)
+        still_maximized = self.marionette.maximize_window()
+        self.assert_window_maximized(still_maximized)
 
     def test_stress(self):
         for i in range(1, 25):
             expect_maximized = bool(i % 2)
 
-            rect = self.marionette.maximize_window()
             if expect_maximized:
+                rect = self.marionette.maximize_window()
                 self.assert_window_maximized(rect)
             else:
-                self.assert_window_restored(rect)
+                rect = self.marionette.set_window_rect(width=800, height=600)
+                self.assertEqual(800, rect["width"])
+                self.assertEqual(600, rect["height"])

testing/web-platform/meta/MANIFEST.json

@@ -626293,7 +626293,7 @@
    "support"
   ],
   "webdriver/tests/contexts/maximize_window.py": [
-   "de49d5d8cc03e863dfe7e29ca8c10678d9b45e83",
+   "905e748f0ab795a66ab41d70aebb22dbc6adbe7e",
    "wdspec"
   ],
   "webdriver/tests/contexts/resizing_and_positioning.py": [

testing/web-platform/tests/webdriver/tests/contexts/maximize_window.py

@@ -1,22 +1,28 @@
 from tests.support.inline import inline
 from tests.support.asserts import assert_error, assert_success
 
+
 alert_doc = inline("<script>window.alert()</script>")
 
+
+def maximize_window(session):
+    return session.transport.send("POST", "session/%s/window/maximize" % session.session_id)
+
+
 # 10.7.3 Maximize Window
-def test_maximize_no_browsing_context(session, create_window):
-    # Step 1
+def test_no_browsing_context(session, create_window):
+    # step 1
     session.window_handle = create_window()
     session.close()
-    result = session.transport.send("POST", "session/%s/window/maximize" % session.session_id)
-    assert_error(result, "no such window")
+    response = maximize_window(session)
+    assert_error(response, "no such window")
 
 
 def test_handle_user_prompt(session):
-    # Step 2
+    # step 2
     session.url = alert_doc
-    result = session.transport.send("POST", "session/%s/window/maximize" % session.session_id)
-    assert_error(result, "unexpected alert open")
+    response = maximize_window(session)
+    assert_error(response, "unexpected alert open")
 
 
 def test_maximize(session):
@@ -24,8 +30,8 @@ def test_maximize(session):
     assert session.window.state == "normal"
 
     # step 4
-    result = session.transport.send("POST", "session/%s/window/maximize" % session.session_id)
-    assert_success(result)
+    response = maximize_window(session)
+    assert_success(response)
 
     assert before_size != session.window.size
     assert session.window.state == "maximized"
@@ -35,13 +41,13 @@ def test_payload(session):
     before_size = session.window.size
     assert session.window.state == "normal"
 
-    result = session.transport.send("POST", "session/%s/window/maximize" % session.session_id)
+    response = maximize_window(session)
 
     # step 5
-    assert result.status == 200
-    assert isinstance(result.body["value"], dict)
+    assert response.status == 200
+    assert isinstance(response.body["value"], dict)
 
-    rect = result.body["value"]
+    rect = response.body["value"]
     assert "width" in rect
     assert "height" in rect
     assert "x" in rect
@@ -57,6 +63,14 @@ def test_payload(session):
     assert session.window.state == "maximized"
 
 
+def test_maximize_twice_is_idempotent(session):
+    assert session.window.state == "normal"
+    session.window.maximize()
+    assert session.window.state == "maximized"
+    session.window.maximize()
+    assert session.window.state == "maximized"
+
+
 def test_maximize_when_resized_to_max_size(session):
     # Determine the largest available window size by first maximising
     # the window and getting the window rect dimensions.