[chromedriver] Create and use Get/Set WindowRect from chrome

Modify session_commands Get/Set Window Position/Size
to use Get/Set WindowRect from chrome
Combine getWindowSize and getWindowPosition to getWindowRect.
Change window_commands::ExecuteGetWindowRect to use it instead.

Change-Id: Ifcc70bfaaab6c89908e404bac03decb8a6adacd7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2141612
Commit-Queue: Shengfa Lin <shengfa@google.com>
Reviewed-by: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757561}

chrome/test/chromedriver/chrome/chrome.h

@@ -27,7 +27,14 @@ class Chrome {
     kPrompt,
   };
 
-  virtual ~Chrome() {}
+  struct WindowRect {
+    int x;
+    int y;
+    int width;
+    int height;
+  };
+
+  virtual ~Chrome() = default;
 
   virtual Status GetAsDesktop(ChromeDesktopImpl** desktop) = 0;
 
@@ -53,30 +60,13 @@ class Chrome {
                            WindowType type,
                            std::string* window_handle) = 0;
 
-  // Gets the size of the specified WebView.
-  virtual Status GetWindowSize(const std::string& id,
-                               int* width,
-                               int* height) = 0;
+  // Gets the rect of the specified WebView
+  virtual Status GetWindowRect(const std::string& id, WindowRect* rect) = 0;
 
   // Sets the rect of the specified WebView
   virtual Status SetWindowRect(const std::string& target_id,
                                const base::DictionaryValue& params) = 0;
 
-  // Sets the size of the specified WebView.
-  virtual Status SetWindowSize(const std::string& target_id,
-                               int width,
-                               int height) = 0;
-
-  // Gets the on-screen position of the specified WebView.
-  virtual Status GetWindowPosition(const std::string& target_id,
-                                   int* x,
-                                   int* y) = 0;
-
-  // Sets the on-screen position of the specified WebView.
-  virtual Status SetWindowPosition(const std::string& target_id,
-                                   int x,
-                                   int y) = 0;
-
   // Maximizes specified WebView.
   virtual Status MaximizeWindow(const std::string& target_id) = 0;
 

chrome/test/chromedriver/chrome/chrome_impl.cc

@@ -162,33 +162,20 @@ Status ChromeImpl::GetWindow(const std::string& target_id, Window* window) {
   return ParseWindow(std::move(result), window);
 }
 
-Status ChromeImpl::GetWindowPosition(const std::string& target_id,
-                                     int* x,
-                                     int* y) {
+Status ChromeImpl::GetWindowRect(const std::string& target_id,
+                                 WindowRect* rect) {
   Window window;
   Status status = GetWindow(target_id, &window);
   if (status.IsError())
     return status;
 
-  *x = window.left;
-  *y = window.top;
+  rect->x = window.left;
+  rect->y = window.top;
+  rect->width = window.width;
+  rect->height = window.height;
   return Status(kOk);
 }
 
-Status ChromeImpl::SetWindowPosition(const std::string& target_id,
-                                            int x,
-                                            int y) {
-  Window window;
-  Status status = GetWindow(target_id, &window);
-  if (status.IsError())
-    return status;
-
-  auto bounds = std::make_unique<base::DictionaryValue>();
-  bounds->SetInteger("left", x);
-  bounds->SetInteger("top", y);
-  return SetWindowBounds(&window, target_id, std::move(bounds));
-}
-
 Status ChromeImpl::MaximizeWindow(const std::string& target_id) {
   Window window;
   Status status = GetWindow(target_id, &window);
@@ -259,19 +246,6 @@ Status ChromeImpl::SetWindowRect(const std::string& target_id,
   return SetWindowBounds(&window, target_id, std::move(bounds));
 }
 
-Status ChromeImpl::GetWindowSize(const std::string& target_id,
-                                 int* width,
-                                 int* height) {
-  Window window;
-  Status status = GetWindow(target_id, &window);
-  if (status.IsError())
-    return status;
-
-  *width = window.width;
-  *height = window.height;
-  return Status(kOk);
-}
-
 Status ChromeImpl::GetWindowBounds(int window_id, Window* window) {
   Status status = devtools_websocket_client_->ConnectIfNecessary();
   if (status.IsError())
@@ -385,21 +359,6 @@ Status ChromeImpl::SetWindowBounds(
   }
 }
 
-Status ChromeImpl::SetWindowSize(const std::string& target_id,
-                                        int width,
-                                        int height) {
-  Window window;
-
-  Status status = GetWindow(target_id, &window);
-  if (status.IsError())
-    return status;
-
-  auto bounds = std::make_unique<base::DictionaryValue>();
-  bounds->SetInteger("width", width);
-  bounds->SetInteger("height", height);
-  return SetWindowBounds(&window, target_id, std::move(bounds));
-}
-
 Status ChromeImpl::ParseWindow(std::unique_ptr<base::DictionaryValue> params,
                                Window* window) {
   if (!params->GetInteger("windowId", &window->id))

chrome/test/chromedriver/chrome/chrome_impl.h

@@ -38,13 +38,9 @@ class ChromeImpl : public Chrome {
   Status NewWindow(const std::string& target_id,
                    WindowType type,
                    std::string* window_handle) override;
-  Status GetWindowSize(const std::string& id, int* width, int* height) override;
-  Status SetWindowSize(const std::string& target_id,
-                       int width, int height) override;
+  Status GetWindowRect(const std::string& id, WindowRect* rect) override;
   Status SetWindowRect(const std::string& target_id,
                        const base::DictionaryValue& params) override;
-  Status GetWindowPosition(const std::string& id, int* x, int* y) override;
-  Status SetWindowPosition(const std::string& target_id, int x, int y) override;
   Status MaximizeWindow(const std::string& target_id) override;
   Status MinimizeWindow(const std::string& target_id) override;
   Status FullScreenWindow(const std::string& target_id) override;

chrome/test/chromedriver/chrome/stub_chrome.cc

@@ -42,15 +42,7 @@ Status StubChrome::NewWindow(const std::string& target_id,
   return Status(kOk);
 }
 
-Status StubChrome::GetWindowSize(const std::string& id,
-                                 int* width,
-                                 int* height) {
-  return Status(kOk);
-}
-
-Status StubChrome::SetWindowSize(const std::string& id,
-                                 int width,
-                                 int height) {
+Status StubChrome::GetWindowRect(const std::string& id, WindowRect* rect) {
   return Status(kOk);
 }
 
@@ -59,17 +51,6 @@ Status StubChrome::SetWindowRect(const std::string& target_id,
   return Status(kOk);
 }
 
-Status StubChrome::GetWindowPosition(const std::string& target_id,
-                                     int* x,
-                                     int* y) {
-  return Status(kOk);
-}
-
-Status StubChrome::SetWindowPosition(const std::string& target_id,
-                                     int x, int y) {
-  return Status(kOk);
-}
-
 Status StubChrome::MaximizeWindow(const std::string& target_id) {
   return Status(kOk);
 }

chrome/test/chromedriver/chrome/stub_chrome.h

@@ -32,14 +32,9 @@ class StubChrome : public Chrome {
   Status NewWindow(const std::string& target_id,
                    WindowType type,
                    std::string* window_handle) override;
-  Status GetWindowSize(const std::string& id, int* width, int* height) override;
-  Status SetWindowSize(const std::string& id, int width, int height) override;
+  Status GetWindowRect(const std::string& id, WindowRect* rect) override;
   Status SetWindowRect(const std::string& target_id,
                        const base::DictionaryValue& params) override;
-  Status GetWindowPosition(const std::string& target_id,
-                           int* x,
-                           int* y) override;
-  Status SetWindowPosition(const std::string& target_id, int x, int y) override;
   Status MaximizeWindow(const std::string& target_id) override;
   Status MinimizeWindow(const std::string& target_id) override;
   Status FullScreenWindow(const std::string& target_id) override;

chrome/test/chromedriver/session_commands.cc

@@ -1107,15 +1107,15 @@ Status ExecuteSetNetworkConnection(Session* session,
 Status ExecuteGetWindowPosition(Session* session,
                                 const base::DictionaryValue& params,
                                 std::unique_ptr<base::Value>* value) {
-  int x, y;
-  Status status = session->chrome->GetWindowPosition(session->window, &x, &y);
+  Chrome::WindowRect windowRect;
+  Status status = session->chrome->GetWindowRect(session->window, &windowRect);
 
   if (status.IsError())
     return status;
 
   base::DictionaryValue position;
-  position.SetInteger("x", x);
-  position.SetInteger("y", y);
+  position.SetInteger("x", windowRect.x);
+  position.SetInteger("y", windowRect.y);
   value->reset(position.DeepCopy());
   return Status(kOk);
 }
@@ -1128,24 +1128,24 @@ Status ExecuteSetWindowPosition(Session* session,
   if (!params.GetDouble("x", &x) || !params.GetDouble("y", &y))
     return Status(kInvalidArgument, "missing or invalid 'x' or 'y'");
 
-  return session->chrome->SetWindowPosition(session->window,
-                                            static_cast<int>(x),
-                                            static_cast<int>(y));
+  base::DictionaryValue rect_params;
+  rect_params.SetInteger("x", static_cast<int>(x));
+  rect_params.SetInteger("y", static_cast<int>(y));
+  return session->chrome->SetWindowRect(session->window, rect_params);
 }
 
 Status ExecuteGetWindowSize(Session* session,
                             const base::DictionaryValue& params,
                             std::unique_ptr<base::Value>* value) {
-  int width, height;
+  Chrome::WindowRect windowRect;
+  Status status = session->chrome->GetWindowRect(session->window, &windowRect);
 
-  Status status =
-      session->chrome->GetWindowSize(session->window, &width, &height);
   if (status.IsError())
     return status;
 
   base::DictionaryValue size;
-  size.SetInteger("width", width);
-  size.SetInteger("height", height);
+  size.SetInteger("width", windowRect.width);
+  size.SetInteger("height", windowRect.height);
   value->reset(size.DeepCopy());
   return Status(kOk);
 }
@@ -1159,9 +1159,10 @@ Status ExecuteSetWindowSize(Session* session,
       !params.GetDouble("height", &height))
     return Status(kInvalidArgument, "missing or invalid 'width' or 'height'");
 
-  return session->chrome->SetWindowSize(session->window,
-                                        static_cast<int>(width),
-                                        static_cast<int>(height));
+  base::DictionaryValue rect_params;
+  rect_params.SetInteger("width", static_cast<int>(width));
+  rect_params.SetInteger("height", static_cast<int>(height));
+  return session->chrome->SetWindowRect(session->window, rect_params);
 }
 
 Status ExecuteGetAvailableLogTypes(Session* session,

chrome/test/chromedriver/window_commands.cc

@@ -2107,31 +2107,21 @@ Status ExecuteTakeHeapSnapshot(Session* session,
   return web_view->TakeHeapSnapshot(value);
 }
 
-// TODO(johnchen): There is no public method in Chrome or ChromeDesktopImpl to
-// get both size and position in one call. What we're doing now is kind of
-// wasteful, since both GetWindowPosition and GetWindowSize end up getting both
-// position and size, and then discard one of the two pieces.
 Status ExecuteGetWindowRect(Session* session,
                             WebView* web_view,
                             const base::DictionaryValue& params,
                             std::unique_ptr<base::Value>* value,
                             Timeout* timeout) {
-  int x, y;
-  int width, height;
-
-  Status status = session->chrome->GetWindowPosition(session->window, &x, &y);
-  if (status.IsError())
-    return status;
-  status = session->chrome->GetWindowSize(session->window, &width, &height);
-
+  Chrome::WindowRect windowRect;
+  Status status = session->chrome->GetWindowRect(session->window, &windowRect);
   if (status.IsError())
     return status;
 
   base::DictionaryValue rect;
-  rect.SetInteger("x", x);
-  rect.SetInteger("y", y);
-  rect.SetInteger("width", width);
-  rect.SetInteger("height", height);
+  rect.SetInteger("x", windowRect.x);
+  rect.SetInteger("y", windowRect.y);
+  rect.SetInteger("width", windowRect.width);
+  rect.SetInteger("height", windowRect.height);
   value->reset(rect.DeepCopy());
   return Status(kOk);
 }