Commit cb19f7f0 authored by dmazzoni's avatar dmazzoni Committed by Commit bot

Fix automation API bounding boxes on high-dpi devices

This regressed in r460412 (https://codereview.chromium.org/2762373002)
when we switched from taking the frame-relative coordinates of each web
object and adding a frame offset, to always just walking up the whole
accessibility tree to find the global bounds of an object.

The reason that failed is because the coordinates we get from the web
have the scale factor applied, but coordinates we get from the desktop
do not. So we need to "unapply" the device scale factor just once,
at the point we walk from the web tree to the desktop tree.

This regressed once before last year. This time I figured out a way to add
a regression test for it.

BUG=726081

Review-Url: https://codereview.chromium.org/2911553002
Cr-Commit-Position: refs/heads/master@{#474967}
parent ec3056b0
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#include "ui/accessibility/ax_tree.h" #include "ui/accessibility/ax_tree.h"
#include "ui/accessibility/ax_tree_serializer.h" #include "ui/accessibility/ax_tree_serializer.h"
#include "ui/accessibility/tree_generator.h" #include "ui/accessibility/tree_generator.h"
#include "ui/display/display_switches.h"
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
#include "ash/accelerators/accelerator_controller.h" #include "ash/accelerators/accelerator_controller.h"
...@@ -298,4 +299,22 @@ IN_PROC_BROWSER_TEST_F(AutomationApiTest, HitTest) { ...@@ -298,4 +299,22 @@ IN_PROC_BROWSER_TEST_F(AutomationApiTest, HitTest) {
<< message_; << message_;
} }
#if defined(OS_CHROMEOS)
class AutomationApiTestWithDeviceScaleFactor : public AutomationApiTest {
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
AutomationApiTest::SetUpCommandLine(command_line);
command_line->AppendSwitchASCII(switches::kForceDeviceScaleFactor, "2.0");
}
};
IN_PROC_BROWSER_TEST_F(AutomationApiTestWithDeviceScaleFactor, LocationScaled) {
StartEmbeddedTestServer();
ASSERT_TRUE(RunPlatformAppTest("automation/tests/location_scaled"))
<< message_;
}
#endif // defined(OS_CHROMEOS)
} // namespace extensions } // namespace extensions
...@@ -128,6 +128,10 @@ static gfx::Rect ComputeGlobalNodeBounds(TreeCache* cache, ...@@ -128,6 +128,10 @@ static gfx::Rect ComputeGlobalNodeBounds(TreeCache* cache,
if (node->data().transform) if (node->data().transform)
node->data().transform->TransformRect(&bounds); node->data().transform->TransformRect(&bounds);
// Walk up to this node's container. This may cross a tree
// boundary, in which case GetParent() modifies |cache|, so we
// save the old cache temporarily.
TreeCache* previous_cache = cache;
ui::AXNode* container = ui::AXNode* container =
cache->tree.GetFromId(node->data().offset_container_id); cache->tree.GetFromId(node->data().offset_container_id);
if (!container) { if (!container) {
...@@ -141,6 +145,16 @@ static gfx::Rect ComputeGlobalNodeBounds(TreeCache* cache, ...@@ -141,6 +145,16 @@ static gfx::Rect ComputeGlobalNodeBounds(TreeCache* cache,
if (!container || container == node) if (!container || container == node)
break; break;
// All trees other than the desktop tree are scaled by the device
// scale factor. When crossing out of another tree into the desktop
// tree, unscale the bounds by the device scale factor.
if (previous_cache->tree_id != api::automation::kDesktopTreeID &&
cache->tree_id == api::automation::kDesktopTreeID) {
float scale_factor = cache->owner->GetDeviceScaleFactor();
if (scale_factor > 0)
bounds.Scale(1.0 / scale_factor);
}
gfx::RectF container_bounds = container->data().location; gfx::RectF container_bounds = container->data().location;
bounds.Offset(container_bounds.x(), container_bounds.y()); bounds.Offset(container_bounds.x(), container_bounds.y());
...@@ -154,17 +168,6 @@ static gfx::Rect ComputeGlobalNodeBounds(TreeCache* cache, ...@@ -154,17 +168,6 @@ static gfx::Rect ComputeGlobalNodeBounds(TreeCache* cache,
node = container; node = container;
} }
// All trees other than the desktop tree are scaled by the device
// scale factor. Unscale them so they're all in consistent units.
if (cache->tree_id != api::automation::kDesktopTreeID) {
float scale_factor = cache->owner->context()
->GetRenderFrame()
->GetRenderView()
->GetDeviceScaleFactor();
if (scale_factor > 0)
bounds.Scale(1.0 / scale_factor);
}
return gfx::ToEnclosingRect(bounds); return gfx::ToEnclosingRect(bounds);
} }
...@@ -1093,6 +1096,10 @@ ui::AXNode* AutomationInternalCustomBindings::GetParent( ...@@ -1093,6 +1096,10 @@ ui::AXNode* AutomationInternalCustomBindings::GetParent(
return nullptr; return nullptr;
} }
float AutomationInternalCustomBindings::GetDeviceScaleFactor() const {
return context()->GetRenderFrame()->GetRenderView()->GetDeviceScaleFactor();
}
void AutomationInternalCustomBindings::RouteTreeIDFunction( void AutomationInternalCustomBindings::RouteTreeIDFunction(
const std::string& name, const std::string& name,
TreeIDFunction callback) { TreeIDFunction callback) {
......
...@@ -59,6 +59,8 @@ class AutomationInternalCustomBindings : public ObjectBackedNativeHandler, ...@@ -59,6 +59,8 @@ class AutomationInternalCustomBindings : public ObjectBackedNativeHandler,
return ObjectBackedNativeHandler::context(); return ObjectBackedNativeHandler::context();
} }
float GetDeviceScaleFactor() const;
private: private:
// ObjectBackedNativeHandler overrides: // ObjectBackedNativeHandler overrides:
void Invalidate() override; void Invalidate() override;
......
<html>
<head>
<title>Page with some absolute-positioned buttons</title>
<style>
body {
margin: 0;
padding: 0;
}
button {
position: absolute;
width: 200px;
height: 100px;
outline: 1px solid #000;
}
.first {
left: 0px;
top: 0px;
}
.second {
left: 225px;
top: 25px;
}
</style>
</head>
<body>
<button class="first">First</button>
<button class="second">Second</button>
</body>
</html>
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
var assertTrue = chrome.test.assertTrue;
var EventType = chrome.automation.EventType;
// These tests are run with a device scale factor of 2.0, but all of the
// coordinates we get from the automation API should be with no device
// scale factor applied.
//
// Note that internally, the device scale factor is applied to web trees,
// but not to the desktop tree, so we want to make sure that that's properly
// taken into account when converting from relative coordinates to global
// device-independent coordinates.
var allTests = [
function testLocationInWebView() {
rootNode.addEventListener(EventType.LOAD_COMPLETE, function() {
var firstButton = rootNode.find({ attributes: { name: 'First' } });
var secondButton = rootNode.find({ attributes: { name: 'Second' } });
if (firstButton && secondButton) {
var firstRect = firstButton.location;
var secondRect = secondButton.location;
// The first button should be at (50, 150) with a size of (200, 100).
// Allow one pixel off for rounding errors.
assertTrue(Math.abs(firstRect.left - 50) <= 1);
assertTrue(Math.abs(firstRect.top - 150) <= 1);
assertTrue(Math.abs(firstRect.width - 200) <= 1);
assertTrue(Math.abs(firstRect.height - 100) <= 1);
// The second button should be exactly 225 x 25 pixels offset from
// the first button.
assertTrue(Math.abs(secondRect.left - firstRect.left - 225) <= 1);
assertTrue(Math.abs(secondRect.top - firstRect.top - 25) <= 1);
assertTrue(Math.abs(secondRect.width - 200) <= 1);
assertTrue(Math.abs(secondRect.height - 100) <= 1);
chrome.test.succeed();
}
}, false);
chrome.app.window.create('buttons.html', {
'innerBounds': {
'left': 50,
'top': 150,
'width': 400,
'height': 400
}
});
}
];
chrome.automation.getDesktop(function(rootNodeArg) {
window.rootNode = rootNodeArg;
chrome.test.runTests(allTests);
});
{
"name": "chrome.automation.scaleFactorTests",
"version": "0.1",
"manifest_version": 2,
"description": "Tests for the Automation API with a device scale factor.",
"automation": {"desktop": true},
"app": {
"background": {
"scripts": ["location_scaled.js"]
}
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment