[ChromeDriver] Add get window size functionality for Android

Making GetWindowSize return the desired values when running ChromeDriver
on Android. Added a test (testAndroidGetWindowSize) to run_py_tests.py
to test this functionality, which should be the same as running a
script to get window.innerWidth / window.innerHeight.

Bug: chromedriver:1005
Change-Id: Ib30b84e657ef9fb2ab1b5e7a5f5dd9cf452ae98f
Reviewed-on: https://chromium-review.googlesource.com/1121335
Commit-Queue: Cole Winstanley <cwinstanley@google.com>
Reviewed-by: John Chen <johnchen@chromium.org>
Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572328}

chrome/test/chromedriver/chrome/chrome.h

@@ -36,6 +36,16 @@ class Chrome {
   // Return the WebView for the given id.
   virtual Status GetWebViewById(const std::string& id, WebView** web_view) = 0;
 
+  // Gets the size of the specified WebView.
+  virtual Status GetWindowSize(const std::string& 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;
+
   // Closes the specified WebView.
   virtual Status CloseWebView(const std::string& id) = 0;
 

chrome/test/chromedriver/chrome/chrome_android_impl.cc

@@ -12,6 +12,7 @@
 #include "chrome/test/chromedriver/chrome/devtools_event_listener.h"
 #include "chrome/test/chromedriver/chrome/devtools_http_client.h"
 #include "chrome/test/chromedriver/chrome/status.h"
+#include "chrome/test/chromedriver/chrome/web_view_impl.h"
 
 ChromeAndroidImpl::ChromeAndroidImpl(
     std::unique_ptr<DevToolsHttpClient> http_client,
@@ -36,6 +37,32 @@ std::string ChromeAndroidImpl::GetOperatingSystemName() {
   return "ANDROID";
 }
 
+Status ChromeAndroidImpl::GetWindow(const std::string& target_id,
+                                    Window* window) {
+  WebView* web_view = nullptr;
+  Status status = GetWebViewById(target_id, &web_view);
+  if (status.IsError())
+    return status;
+
+  std::unique_ptr<base::Value> result;
+  std::string expression =
+      "[window.screenX, window.screenY, window.outerWidth * "
+      "window.devicePixelRatio, window.outerHeight * window.devicePixelRatio]";
+  status = web_view->EvaluateScript(target_id, expression, &result);
+  if (status.IsError())
+    return status;
+
+  window->left = result->GetList()[0].GetInt();
+  window->top = result->GetList()[1].GetInt();
+  window->width = result->GetList()[2].GetInt();
+  window->height = result->GetList()[3].GetInt();
+  // Android does not use Window.id or have window states
+  window->id = 0;
+  window->state = "";
+
+  return status;
+}
+
 bool ChromeAndroidImpl::HasTouchScreen() const {
   const BrowserInfo* browser_info = GetBrowserInfo();
   if (browser_info->browser_name == "webview")

chrome/test/chromedriver/chrome/chrome_android_impl.h

@@ -34,6 +34,9 @@ class ChromeAndroidImpl : public ChromeImpl {
   bool HasTouchScreen() const override;
   Status QuitImpl() override;
 
+ protected:
+  Status GetWindow(const std::string& target_id, Window* window) override;
+
  private:
   std::unique_ptr<Device> device_;
 };

chrome/test/chromedriver/chrome/chrome_desktop_impl.cc

@@ -231,32 +231,6 @@ void ChromeDesktopImpl::SetNetworkConnection(
   network_connection_ = network_connection;
 }
 
-Status ChromeDesktopImpl::GetWindowPosition(const std::string& target_id,
-                                            int* x,
-                                            int* y) {
-  Window window;
-  Status status = GetWindow(target_id, &window);
-  if (status.IsError())
-    return status;
-
-  *x = window.left;
-  *y = window.top;
-  return Status(kOk);
-}
-
-Status ChromeDesktopImpl::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 ChromeDesktopImpl::SetWindowRect(const std::string& target_id,
                                         const base::DictionaryValue& params) {
   Window window;
@@ -409,55 +383,6 @@ Status ChromeDesktopImpl::FullScreenWindow(const std::string& target_id) {
   return SetWindowBounds(window.id, std::move(bounds));
 }
 
-Status ChromeDesktopImpl::ParseWindowBounds(
-    std::unique_ptr<base::DictionaryValue> params,
-    Window* window) {
-  const base::Value* value = nullptr;
-  const base::DictionaryValue* bounds_dict = nullptr;
-  if (!params->Get("bounds", &value) || !value->GetAsDictionary(&bounds_dict))
-    return Status(kUnknownError, "no window bounds in response");
-
-  if (!bounds_dict->GetString("windowState", &window->state))
-    return Status(kUnknownError, "no window state in window bounds");
-
-  if (!bounds_dict->GetInteger("left", &window->left))
-    return Status(kUnknownError, "no left offset in window bounds");
-  if (!bounds_dict->GetInteger("top", &window->top))
-    return Status(kUnknownError, "no top offset in window bounds");
-  if (!bounds_dict->GetInteger("width", &window->width))
-    return Status(kUnknownError, "no width in window bounds");
-  if (!bounds_dict->GetInteger("height", &window->height))
-    return Status(kUnknownError, "no height in window bounds");
-
-  return Status(kOk);
-}
-
-Status ChromeDesktopImpl::ParseWindow(
-    std::unique_ptr<base::DictionaryValue> params,
-    Window* window) {
-  if (!params->GetInteger("windowId", &window->id))
-    return Status(kUnknownError, "no window id in response");
-
-  return ParseWindowBounds(std::move(params), window);
-}
-
-Status ChromeDesktopImpl::GetWindow(const std::string& target_id,
-                                    Window* window) {
-  Status status = devtools_websocket_client_->ConnectIfNecessary();
-  if (status.IsError())
-    return status;
-
-  base::DictionaryValue params;
-  params.SetString("targetId", target_id);
-  std::unique_ptr<base::DictionaryValue> result;
-  status = devtools_websocket_client_->SendCommandAndGetResult(
-      "Browser.getWindowForTarget", params, &result);
-  if (status.IsError())
-    return status;
-
-  return ParseWindow(std::move(result), window);
-}
-
 Status ChromeDesktopImpl::GetWindowBounds(int window_id, Window* window) {
   Status status = devtools_websocket_client_->ConnectIfNecessary();
   if (status.IsError())

chrome/test/chromedriver/chrome/chrome_desktop_impl.h

@@ -12,7 +12,6 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/process/process.h"
-#include "base/values.h"
 #include "chrome/test/chromedriver/chrome/chrome_impl.h"
 #include "chrome/test/chromedriver/chrome/scoped_temp_dir_with_retry.h"
 
@@ -66,8 +65,6 @@ class ChromeDesktopImpl : public ChromeImpl {
   int GetNetworkConnection() const;
   void SetNetworkConnection(int network_connection);
 
-  Status GetWindowPosition(const std::string& target_id, int* x, int* y);
-  Status GetWindowSize(const std::string& target_id, int* width, int* height);
   Status SetWindowRect(const std::string& target_id,
                        const base::DictionaryValue& params);
   Status SetWindowPosition(const std::string& target_id, int x, int y);
@@ -77,20 +74,6 @@ class ChromeDesktopImpl : public ChromeImpl {
   Status FullScreenWindow(const std::string& target_id);
 
  private:
-  struct Window {
-    int id;
-    std::string state;
-    int left;
-    int top;
-    int width;
-    int height;
-  };
-  Status ParseWindowBounds(std::unique_ptr<base::DictionaryValue> params,
-                           Window* window);
-  Status ParseWindow(std::unique_ptr<base::DictionaryValue> params,
-                     Window* window);
-
-  Status GetWindow(const std::string& target_id, Window* window);
   Status GetWindowBounds(int window_id, Window* window);
   Status SetWindowBounds(int window_id,
                          std::unique_ptr<base::DictionaryValue> bounds);

chrome/test/chromedriver/chrome/chrome_impl.cc

@@ -122,6 +122,79 @@ Status ChromeImpl::GetWebViewById(const std::string& id, WebView** web_view) {
   return Status(kUnknownError, "web view not found");
 }
 
+Status ChromeImpl::GetWindow(const std::string& target_id, Window* window) {
+  Status status = devtools_websocket_client_->ConnectIfNecessary();
+  if (status.IsError())
+    return status;
+
+  base::DictionaryValue params;
+  params.SetString("targetId", target_id);
+  std::unique_ptr<base::DictionaryValue> result;
+  status = devtools_websocket_client_->SendCommandAndGetResult(
+      "Browser.getWindowForTarget", params, &result);
+  if (status.IsError())
+    return status;
+
+  return ParseWindow(std::move(result), window);
+}
+
+Status ChromeImpl::GetWindowPosition(const std::string& target_id,
+                                     int* x,
+                                     int* y) {
+  Window window;
+  Status status = GetWindow(target_id, &window);
+  if (status.IsError())
+    return status;
+
+  *x = window.left;
+  *y = window.top;
+  return Status(kOk);
+}
+
+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::ParseWindow(std::unique_ptr<base::DictionaryValue> params,
+                               Window* window) {
+  if (!params->GetInteger("windowId", &window->id))
+    return Status(kUnknownError, "no window id in response");
+
+  return ParseWindowBounds(std::move(params), window);
+}
+
+Status ChromeImpl::ParseWindowBounds(
+    std::unique_ptr<base::DictionaryValue> params,
+    Window* window) {
+  const base::Value* value = nullptr;
+  const base::DictionaryValue* bounds_dict = nullptr;
+  if (!params->Get("bounds", &value) || !value->GetAsDictionary(&bounds_dict))
+    return Status(kUnknownError, "no window bounds in response");
+
+  if (!bounds_dict->GetString("windowState", &window->state))
+    return Status(kUnknownError, "no window state in window bounds");
+
+  if (!bounds_dict->GetInteger("left", &window->left))
+    return Status(kUnknownError, "no left offset in window bounds");
+  if (!bounds_dict->GetInteger("top", &window->top))
+    return Status(kUnknownError, "no top offset in window bounds");
+  if (!bounds_dict->GetInteger("width", &window->width))
+    return Status(kUnknownError, "no width in window bounds");
+  if (!bounds_dict->GetInteger("height", &window->height))
+    return Status(kUnknownError, "no height in window bounds");
+
+  return Status(kOk);
+}
+
 Status ChromeImpl::CloseWebView(const std::string& id) {
   Status status = devtools_http_client_->CloseWebView(id);
   if (status.IsError())

chrome/test/chromedriver/chrome/chrome_impl.h

@@ -12,6 +12,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/memory/linked_ptr.h"
+#include "base/values.h"
 #include "chrome/test/chromedriver/chrome/chrome.h"
 
 struct BrowserInfo;
@@ -26,7 +27,6 @@ class WebViewsInfo;
 class ChromeImpl : public Chrome {
  public:
   ~ChromeImpl() override;
-
   // Overridden from Chrome:
   Status GetAsDesktop(ChromeDesktopImpl** desktop) override;
   const BrowserInfo* GetBrowserInfo() const override;
@@ -36,6 +36,8 @@ class ChromeImpl : public Chrome {
   Status GetWebViewIds(std::list<std::string>* web_view_ids,
                        bool w3c_compliant) override;
   Status GetWebViewById(const std::string& id, WebView** web_view) override;
+  Status GetWindowSize(const std::string& id, int* width, int* height) override;
+  Status GetWindowPosition(const std::string& id, int* x, int* y) override;
   Status CloseWebView(const std::string& id) override;
   Status ActivateWebView(const std::string& id) override;
   Status SetAcceptInsecureCerts() override;
@@ -53,6 +55,20 @@ class ChromeImpl : public Chrome {
 
   virtual Status QuitImpl() = 0;
 
+  struct Window {
+    int id;
+    std::string state;
+    int left;
+    int top;
+    int width;
+    int height;
+  };
+  virtual Status GetWindow(const std::string& target_id, Window* window);
+  Status ParseWindow(std::unique_ptr<base::DictionaryValue> params,
+                     Window* window);
+  Status ParseWindowBounds(std::unique_ptr<base::DictionaryValue> params,
+                           Window* window);
+
   bool quit_;
   std::unique_ptr<DevToolsHttpClient> devtools_http_client_;
   std::unique_ptr<DevToolsClient> devtools_websocket_client_;

chrome/test/chromedriver/chrome/stub_chrome.cc

@@ -36,6 +36,18 @@ Status StubChrome::GetWebViewById(const std::string& id, WebView** web_view) {
   return Status(kOk);
 }
 
+Status StubChrome::GetWindowSize(const std::string& id,
+                                 int* width,
+                                 int* height) {
+  return Status(kOk);
+}
+
+Status StubChrome::GetWindowPosition(const std::string& target_id,
+                                     int* x,
+                                     int* y) {
+  return Status(kOk);
+}
+
 Status StubChrome::CloseWebView(const std::string& id) {
   return Status(kOk);
 }

chrome/test/chromedriver/chrome/stub_chrome.h

@@ -29,6 +29,10 @@ class StubChrome : public Chrome {
   Status GetWebViewIds(std::list<std::string>* web_view_ids,
                        bool w3c_compliant) override;
   Status GetWebViewById(const std::string& id, WebView** web_view) override;
+  Status GetWindowSize(const std::string& id, int* width, int* height) override;
+  Status GetWindowPosition(const std::string& target_id,
+                           int* x,
+                           int* y) override;
   Status CloseWebView(const std::string& id) override;
   Status ActivateWebView(const std::string& id) override;
   Status SetAcceptInsecureCerts() override;

chrome/test/chromedriver/session_commands.cc

@@ -750,34 +750,21 @@ Status ExecuteSetNetworkConnection(Session* session,
   return Status(kOk);
 }
 
+// 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,
                             const base::DictionaryValue& params,
                             std::unique_ptr<base::Value>* value) {
-  ChromeDesktopImpl* desktop = NULL;
-  Status status = session->chrome->GetAsDesktop(&desktop);
-  if (status.IsError())
-    return status;
-
   int x, y;
   int width, height;
 
-  if (desktop->GetBrowserInfo()->build_no >= kBrowserWindowDevtoolsBuildNo) {
-    status = desktop->GetWindowPosition(session->window, &x, &y);
-    if (status.IsError())
-      return status;
-    status = desktop->GetWindowSize(session->window, &width, &height);
-  } else {
-    AutomationExtension* extension = NULL;
-    status =
-        desktop->GetAutomationExtension(&extension, session->w3c_compliant);
-    if (status.IsError())
-      return status;
+  Status status = session->chrome->GetWindowPosition(session->window, &x, &y);
+  if (status.IsError())
+    return status;
+  status = session->chrome->GetWindowSize(session->window, &width, &height);
 
-    status = extension->GetWindowPosition(&x, &y);
-    if (status.IsError())
-      return status;
-    status = extension->GetWindowSize(&width, &height);
-  }
   if (status.IsError())
     return status;
 
@@ -793,24 +780,9 @@ Status ExecuteGetWindowRect(Session* session,
 Status ExecuteGetWindowPosition(Session* session,
                                 const base::DictionaryValue& params,
                                 std::unique_ptr<base::Value>* value) {
-  ChromeDesktopImpl* desktop = NULL;
-  Status status = session->chrome->GetAsDesktop(&desktop);
-  if (status.IsError())
-    return status;
-
   int x, y;
+  Status status = session->chrome->GetWindowPosition(session->window, &x, &y);
 
-  if (desktop->GetBrowserInfo()->build_no >= kBrowserWindowDevtoolsBuildNo) {
-    status = desktop->GetWindowPosition(session->window, &x, &y);
-  } else {
-    AutomationExtension* extension = NULL;
-    status =
-        desktop->GetAutomationExtension(&extension, session->w3c_compliant);
-    if (status.IsError())
-      return status;
-
-    status = extension->GetWindowPosition(&x, &y);
-  }
   if (status.IsError())
     return status;
 
@@ -850,24 +822,10 @@ Status ExecuteSetWindowPosition(Session* session,
 Status ExecuteGetWindowSize(Session* session,
                             const base::DictionaryValue& params,
                             std::unique_ptr<base::Value>* value) {
-  ChromeDesktopImpl* desktop = NULL;
-  Status status = session->chrome->GetAsDesktop(&desktop);
-  if (status.IsError())
-    return status;
-
   int width, height;
 
-  if (desktop->GetBrowserInfo()->build_no >= kBrowserWindowDevtoolsBuildNo) {
-    status = desktop->GetWindowSize(session->window, &width, &height);
-  } else {
-    AutomationExtension* extension = NULL;
-    status =
-        desktop->GetAutomationExtension(&extension, session->w3c_compliant);
-    if (status.IsError())
-      return status;
-
-    status = extension->GetWindowSize(&width, &height);
-  }
+  Status status =
+      session->chrome->GetWindowSize(session->window, &width, &height);
   if (status.IsError())
     return status;
 

chrome/test/chromedriver/test/run_py_tests.py

@@ -180,7 +180,7 @@ _ANDROID_NEGATIVE_FILTER['chrome'] = (
         'ChromeDownloadDirTest.*',
         # https://crbug.com/274650
         'ChromeDriverTest.testCloseWindow',
-        # https://bugs.chromium.org/p/chromedriver/issues/detail?id=298
+        # Most window operations don't make sense on Android.
         'ChromeDriverTest.testWindowFullScreen',
         'ChromeDriverTest.testWindowPosition',
         'ChromeDriverTest.testWindowSize',
@@ -1923,6 +1923,24 @@ class ChromeDriverAndroidTest(ChromeDriverBaseTest):
     orientation = self._driver.GetScreenOrientation()
     self.assertEqual(orientation['orientation'], 'LANDSCAPE')
 
+  def testAndroidGetWindowSize(self):
+    self._driver = self.CreateDriver()
+    size = self._driver.GetWindowSize()
+
+    script_size = self._driver.ExecuteScript(
+      "return [window.outerWidth * window.devicePixelRatio,"
+      "window.outerHeight * window.devicePixelRatio]")
+    self.assertEquals(size, script_size)
+
+    script_inner = self._driver.ExecuteScript(
+      "return [window.innerWidth, window.innerHeight]")
+    self.assertLessEqual(script_inner[0], size[0])
+    self.assertLessEqual(script_inner[1], size[1])
+    # Sanity check: screen dimensions in the range 2-20000px
+    self.assertLessEqual(size[0], 20000)
+    self.assertLessEqual(size[1], 20000)
+    self.assertGreaterEqual(size[0], 2)
+    self.assertGreaterEqual(size[1], 2)
 
 class ChromeDownloadDirTest(ChromeDriverBaseTest):