Commit e3a974bb authored by kenrb's avatar kenrb Committed by Commit bot

Prevent coordinate transformation when targeting the root RWHV

In some cases, an input event being targeted to the root RWHV was going
through coordinate transformation, while in other cases it was not.
This is normally fine because it is the identity transform anyway, but
it can result in rounding error when there is a non-integer device
scale factor. This was noted to cause a bug related to selections not
being cleared because of coordinates from a transformed (rounded)
event being compared against coordinates from a non-tranformed
(unrounded) input event.

This CL prevents transformation in all cases when the root is being
targeted and the event is already in the coordinate space of the root.

BUG=670253
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2567093003
Cr-Commit-Position: refs/heads/master@{#438012}
parent 2c3eccee
...@@ -543,9 +543,14 @@ bool RenderWidgetHostViewChildFrame::TransformPointToCoordSpaceForView( ...@@ -543,9 +543,14 @@ bool RenderWidgetHostViewChildFrame::TransformPointToCoordSpaceForView(
const gfx::Point& point, const gfx::Point& point,
RenderWidgetHostViewBase* target_view, RenderWidgetHostViewBase* target_view,
gfx::Point* transformed_point) { gfx::Point* transformed_point) {
if (!frame_connector_ || !local_frame_id_.is_valid() || target_view == this) if (!frame_connector_ || !local_frame_id_.is_valid())
return false; return false;
if (target_view == this) {
*transformed_point = point;
return true;
}
return frame_connector_->TransformPointToCoordSpaceForView( return frame_connector_->TransformPointToCoordSpaceForView(
point, target_view, cc::SurfaceId(frame_sink_id_, local_frame_id_), point, target_view, cc::SurfaceId(frame_sink_id_, local_frame_id_),
transformed_point); transformed_point);
......
...@@ -1657,6 +1657,11 @@ bool RenderWidgetHostViewAura::TransformPointToCoordSpaceForView( ...@@ -1657,6 +1657,11 @@ bool RenderWidgetHostViewAura::TransformPointToCoordSpaceForView(
const gfx::Point& point, const gfx::Point& point,
RenderWidgetHostViewBase* target_view, RenderWidgetHostViewBase* target_view,
gfx::Point* transformed_point) { gfx::Point* transformed_point) {
if (target_view == this) {
*transformed_point = point;
return true;
}
// In TransformPointToLocalCoordSpace() there is a Point-to-Pixel conversion, // In TransformPointToLocalCoordSpace() there is a Point-to-Pixel conversion,
// but it is not necessary here because the final target view is responsible // but it is not necessary here because the final target view is responsible
// for converting before computing the final transform. // for converting before computing the final transform.
......
...@@ -1579,6 +1579,11 @@ bool RenderWidgetHostViewMac::TransformPointToCoordSpaceForView( ...@@ -1579,6 +1579,11 @@ bool RenderWidgetHostViewMac::TransformPointToCoordSpaceForView(
const gfx::Point& point, const gfx::Point& point,
RenderWidgetHostViewBase* target_view, RenderWidgetHostViewBase* target_view,
gfx::Point* transformed_point) { gfx::Point* transformed_point) {
if (target_view == this) {
*transformed_point = point;
return true;
}
return browser_compositor_->GetDelegatedFrameHost() return browser_compositor_->GetDelegatedFrameHost()
->TransformPointToCoordSpaceForView(point, target_view, ->TransformPointToCoordSpaceForView(point, target_view,
transformed_point); transformed_point);
......
...@@ -589,6 +589,26 @@ class SitePerProcessHighDPIBrowserTest : public SitePerProcessBrowserTest { ...@@ -589,6 +589,26 @@ class SitePerProcessHighDPIBrowserTest : public SitePerProcessBrowserTest {
} }
}; };
//
// SitePerProcessNonIntegerScaleFactorBrowserTest
//
class SitePerProcessNonIntegerScaleFactorBrowserTest
: public SitePerProcessBrowserTest {
public:
const double kDeviceScaleFactor = 1.5;
SitePerProcessNonIntegerScaleFactorBrowserTest() {}
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
SitePerProcessBrowserTest::SetUpCommandLine(command_line);
command_line->AppendSwitchASCII(
switches::kForceDeviceScaleFactor,
base::StringPrintf("%f", kDeviceScaleFactor));
}
};
// SitePerProcessIgnoreCertErrorsBrowserTest // SitePerProcessIgnoreCertErrorsBrowserTest
class SitePerProcessIgnoreCertErrorsBrowserTest class SitePerProcessIgnoreCertErrorsBrowserTest
...@@ -8833,4 +8853,59 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, ...@@ -8833,4 +8853,59 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
child->current_frame_host()->GetProcess()); child->current_frame_host()->GetProcess());
} }
// Test that MouseDown and MouseUp to the same coordinates do not result in
// different coordinates after routing. See bug https://crbug.com/670253.
#if defined(OS_ANDROID)
// Browser process hit testing is not implemented on Android.
// https://crbug.com/491334
#define MAYBE_MouseClickWithNonIntegerScaleFactor \
DISABLED_MouseClickWithNonIntegerScaleFactor
#else
#define MAYBE_MouseClickWithNonIntegerScaleFactor \
MouseClickWithNonIntegerScaleFactor
#endif
IN_PROC_BROWSER_TEST_F(SitePerProcessNonIntegerScaleFactorBrowserTest,
MAYBE_MouseClickWithNonIntegerScaleFactor) {
GURL initial_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
EXPECT_TRUE(NavigateToURL(shell(), initial_url));
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetFrameTree()
->root();
RenderWidgetHostViewBase* rwhv = static_cast<RenderWidgetHostViewBase*>(
root->current_frame_host()->GetRenderWidgetHost()->GetView());
RenderWidgetHostInputEventRouter* router =
static_cast<WebContentsImpl*>(shell()->web_contents())
->GetInputEventRouter();
// Create listener for input events.
RenderWidgetHostMouseEventMonitor event_monitor(
root->current_frame_host()->GetRenderWidgetHost());
blink::WebMouseEvent mouse_event;
mouse_event.type = blink::WebInputEvent::MouseDown;
mouse_event.button = blink::WebPointerProperties::Button::Left;
mouse_event.x = 75;
mouse_event.y = 75;
mouse_event.clickCount = 1;
event_monitor.ResetEventReceived();
router->RouteMouseEvent(rwhv, &mouse_event, ui::LatencyInfo());
EXPECT_TRUE(event_monitor.EventWasReceived());
gfx::Point mouse_down_coords =
gfx::Point(event_monitor.event().x, event_monitor.event().y);
event_monitor.ResetEventReceived();
mouse_event.type = blink::WebInputEvent::MouseUp;
mouse_event.x = 75;
mouse_event.y = 75;
router->RouteMouseEvent(rwhv, &mouse_event, ui::LatencyInfo());
EXPECT_TRUE(event_monitor.EventWasReceived());
EXPECT_EQ(mouse_down_coords,
gfx::Point(event_monitor.event().x, event_monitor.event().y));
}
} // namespace content } // namespace content
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