Skip to content

GitLab

T
tangled
Commit 4c50e3e1 authored by Jaebaek Seo's avatar Jaebaek Seo Committed by Commit Bot
Browse files
Options

Adjust location and size of TapDisambiguator 

As shown in crbug.com/737777#c72, the location and size of Android tab
disambiguator is not correct when --use-zoom-for-dsf is enabled. It is because
RenderViewImpl::DidTapMultipleTargets() assumes |touch_rect| is in DIPs while it
is in physical pixels when --use-zoom-for-dsf is enabled. This CL solves this
problem.

TEST=Android Chrome with production code on NEXUS 5X,
content_browsertests RenderViewTapDisambiguationTest.CanvasSizeUseZoomForDSF
on NEXUS 5X, NEXUS 4, NEXUS 9 (w/ and w/o this change).

Bug: 737777
Change-Id: I64ca0b9b76fcfc3798036a80c9213973de8f3f77
Reviewed-on: https://chromium-review.googlesource.com/898824Reviewed-by: default avatarDmitry Gozman <dgozman@chromium.org>
Commit-Queue: Jaebaek Seo <jaebaek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536868}
parent 02349e18
Hide whitespace changes
Inline Side-by-side
Showing with 91 additions and 17 deletions
content/renderer/render_view_browsertest.cc
View file @ 4c50e3e1
...@@ -91,6 +91,13 @@ ...@@ -91,6 +91,13 @@
#include "ui/gfx/range/range.h" #include "ui/gfx/range/range.h"
#include "ui/native_theme/native_theme_features.h" #include "ui/native_theme/native_theme_features.h"
#if defined(OS_ANDROID)
#include "third_party/WebKit/public/platform/WebCoalescedInputEvent.h"
#include "third_party/WebKit/public/platform/WebGestureDevice.h"
#include "third_party/WebKit/public/platform/WebGestureEvent.h"
#include "third_party/WebKit/public/platform/WebInputEvent.h"
#endif
#if defined(OS_WIN) #if defined(OS_WIN)
#include "base/win/windows_version.h" #include "base/win/windows_version.h"
#endif #endif
...@@ -475,6 +482,82 @@ class RenderViewImplScaleFactorTest : public RenderViewImplBlinkSettingsTest { ...@@ -475,6 +482,82 @@ class RenderViewImplScaleFactorTest : public RenderViewImplBlinkSettingsTest {
} }
}; };
#if defined(OS_ANDROID)
class TapCallbackFilter : public IPC::Listener {
public:
explicit TapCallbackFilter(
base::RepeatingCallback<void(const gfx::Rect&, const gfx::Size&)>
callback)
: callback_(callback) {}
bool OnMessageReceived(const IPC::Message& msg) override {
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(TapCallbackFilter, msg)
IPC_MESSAGE_HANDLER(ViewHostMsg_ShowDisambiguationPopup,
OnShowDisambiguationPopup)
IPC_MESSAGE_UNHANDLED(handled = false)
IPC_END_MESSAGE_MAP()
return handled;
}
void OnShowDisambiguationPopup(const gfx::Rect& rect_pixels,
const gfx::Size& size,
base::SharedMemoryHandle handle) {
callback_.Run(rect_pixels, size);
}
private:
base::RepeatingCallback<void(const gfx::Rect&, const gfx::Size&)> callback_;
};
static blink::WebCoalescedInputEvent FatTap(int x,
int y,
int width,
int height) {
blink::WebGestureEvent event(blink::WebInputEvent::kGestureTap,
blink::WebInputEvent::kNoModifiers,
blink::WebInputEvent::kTimeStampForTesting);
event.source_device = blink::kWebGestureDeviceTouchscreen;
event.x = x;
event.y = y;
event.data.tap.width = width;
event.data.tap.height = height;
return blink::WebCoalescedInputEvent(event);
}
TEST_F(RenderViewImplScaleFactorTest, TapDisambiguatorSize) {
DoSetUp();
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kEnableUseZoomForDSF);
const float device_scale = 2.625f;
SetDeviceScaleFactor(device_scale);
EXPECT_EQ(device_scale, view()->GetDeviceScaleFactor());
LoadHTML(
"<style type=\"text/css\">"
" a {"
" display: block;"
" width: 40px;"
" height: 40px;"
" background-color:#ccccff;"
" }"
"</style>"
"<body style=\"margin: 0px\">"
"<a href=\"#\">link </a>"
"<a href=\"#\">link </a>"
"</body>");
std::unique_ptr<TapCallbackFilter> callback_filter(
new TapCallbackFilter(base::BindRepeating(
[](const gfx::Rect& rect_pixels, const gfx::Size& canvas_size) {
EXPECT_EQ(rect_pixels, gfx::Rect(28, 68, 24, 24));
EXPECT_EQ(canvas_size, gfx::Size(48, 48));
})));
render_thread_->sink().AddFilter(callback_filter.get());
view()->webview()->HandleInputEvent(FatTap(40, 80, 200, 200));
render_thread_->sink().RemoveFilter(callback_filter.get());
}
#endif
// Ensure that the main RenderFrame is deleted and cleared from the RenderView // Ensure that the main RenderFrame is deleted and cleared from the RenderView
// after closing it. // after closing it.
TEST_F(RenderViewImplTest, RenderFrameClearedAfterClose) { TEST_F(RenderViewImplTest, RenderFrameClearedAfterClose) {
......
content/renderer/render_view_impl.cc
View file @ 4c50e3e1
...@@ -1493,13 +1493,6 @@ void RenderViewImpl::UpdateTargetURL(const GURL& url, ...@@ -1493,13 +1493,6 @@ void RenderViewImpl::UpdateTargetURL(const GURL& url,
} }
} }
gfx::RectF RenderViewImpl::ClientRectToPhysicalWindowRect(
const gfx::RectF& rect) const {
gfx::RectF window_rect = rect;
window_rect.Scale(device_scale_factor_ * webview()->PageScaleFactor());
return window_rect;
}
void RenderViewImpl::StartNavStateSyncTimerIfNecessary(RenderFrameImpl* frame) { void RenderViewImpl::StartNavStateSyncTimerIfNecessary(RenderFrameImpl* frame) {
// Keep track of which frames have pending updates. // Keep track of which frames have pending updates.
frames_with_pending_state_.insert(frame->GetRoutingID()); frames_with_pending_state_.insert(frame->GetRoutingID());
...@@ -2300,12 +2293,13 @@ bool RenderViewImpl::DidTapMultipleTargets( ...@@ -2300,12 +2293,13 @@ bool RenderViewImpl::DidTapMultipleTargets(
// The touch_rect, target_rects and zoom_rect are in the outer viewport // The touch_rect, target_rects and zoom_rect are in the outer viewport
// reference frame. // reference frame.
float to_pix = IsUseZoomForDSFEnabled() ? 1 : device_scale_factor_;
gfx::Rect zoom_rect; gfx::Rect zoom_rect;
float new_total_scale = float new_total_scale =
DisambiguationPopupHelper::ComputeZoomAreaAndScaleFactor( DisambiguationPopupHelper::ComputeZoomAreaAndScaleFactor(
touch_rect, target_rects, GetSize(), touch_rect, target_rects, GetSize(),
gfx::Rect(webview()->MainFrame()->VisibleContentRect()).size(), gfx::Rect(webview()->MainFrame()->VisibleContentRect()).size(),
device_scale_factor_ * webview()->PageScaleFactor(), &zoom_rect); to_pix * webview()->PageScaleFactor(), &zoom_rect);
if (!new_total_scale || zoom_rect.IsEmpty()) if (!new_total_scale || zoom_rect.IsEmpty())
return false; return false;
...@@ -2342,10 +2336,8 @@ bool RenderViewImpl::DidTapMultipleTargets( ...@@ -2342,10 +2336,8 @@ bool RenderViewImpl::DidTapMultipleTargets(
// device scale will be applied in WebKit // device scale will be applied in WebKit
// --> zoom_rect doesn't include device scale, // --> zoom_rect doesn't include device scale,
// but WebKit will still draw on zoom_rect * device_scale_factor_ // but WebKit will still draw on zoom_rect * device_scale_factor_
canvas.scale(new_total_scale / device_scale_factor_, canvas.scale(new_total_scale / to_pix, new_total_scale / to_pix);
new_total_scale / device_scale_factor_); canvas.translate(-zoom_rect.x() * to_pix, -zoom_rect.y() * to_pix);
canvas.translate(-zoom_rect.x() * device_scale_factor_,
-zoom_rect.y() * device_scale_factor_);
DCHECK(webview_->IsAcceleratedCompositingActive()); DCHECK(webview_->IsAcceleratedCompositingActive());
webview_->UpdateAllLifecyclePhases(); webview_->UpdateAllLifecyclePhases();
...@@ -2357,7 +2349,8 @@ bool RenderViewImpl::DidTapMultipleTargets( ...@@ -2357,7 +2349,8 @@ bool RenderViewImpl::DidTapMultipleTargets(
inner_viewport_offset.height); inner_viewport_offset.height);
gfx::Rect physical_window_zoom_rect = gfx::ToEnclosingRect( gfx::Rect physical_window_zoom_rect = gfx::ToEnclosingRect(
ClientRectToPhysicalWindowRect(gfx::RectF(zoom_rect_in_screen))); gfx::ScaleRect(gfx::RectF(zoom_rect_in_screen),
to_pix * webview()->PageScaleFactor()));
// A SharedMemoryHandle is sent to the browser process, which is // A SharedMemoryHandle is sent to the browser process, which is
// responsible for freeing the shared memory when no longer needed. // responsible for freeing the shared memory when no longer needed.
......
content/renderer/render_view_impl.h
View file @ 4c50e3e1
...@@ -304,6 +304,8 @@ class CONTENT_EXPORT RenderViewImpl : public RenderWidget, ...@@ -304,6 +304,8 @@ class CONTENT_EXPORT RenderViewImpl : public RenderWidget,
const blink::WebNode& toNode) override; const blink::WebNode& toNode) override;
void DidUpdateLayout() override; void DidUpdateLayout() override;
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// |touch_rect| is in physical pixels if --use-zoom-for-dsf is enabled.
// Otherwise, it is in DIPs.
bool DidTapMultipleTargets( bool DidTapMultipleTargets(
const blink::WebSize& inner_viewport_offset, const blink::WebSize& inner_viewport_offset,
const blink::WebRect& touch_rect, const blink::WebRect& touch_rect,
...@@ -585,10 +587,6 @@ class CONTENT_EXPORT RenderViewImpl : public RenderWidget, ...@@ -585,10 +587,6 @@ class CONTENT_EXPORT RenderViewImpl : public RenderWidget,
// If |url| is empty, show |fallback_url|. // If |url| is empty, show |fallback_url|.
void UpdateTargetURL(const GURL& url, const GURL& fallback_url); void UpdateTargetURL(const GURL& url, const GURL& fallback_url);
// Coordinate conversion -----------------------------------------------------
gfx::RectF ClientRectToPhysicalWindowRect(const gfx::RectF& rect) const;
// RenderFrameImpl accessible state ------------------------------------------ // RenderFrameImpl accessible state ------------------------------------------
// The following section is the set of methods that RenderFrameImpl needs // The following section is the set of methods that RenderFrameImpl needs
// to access RenderViewImpl state. The set of state variables are page-level // to access RenderViewImpl state. The set of state variables are page-level
......
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