Commit 205abd5f authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

Revert "Reland of: Handle iframe touch ack time out"

This reverts commit 38642faa.

Reason for revert: <INSERT REASONING HERE>
Crashes on Win canary

Original change's description:
> Reland of: Handle iframe touch ack time out
> 
> The previous CL:
> https://chromium-review.googlesource.com/c/chromium/src/+/1102729
> was reverted. The reason is that in the test we have two gesture scroll,
> and the second gesture starts before the first TapDown finishes.
> 
> This CL solve the problem. PS#1 is exactly the same as the previous CL
> which was reverted, so that it is easier for review.
> 
> TBR=creis@chromium.org
> 
> Bug: 851644
> Change-Id: Ife9bf8f9a37892380e6587b80bcc03366cc0e80d
> Reviewed-on: https://chromium-review.googlesource.com/1151094
> Commit-Queue: Xida Chen <xidachen@chromium.org>
> Reviewed-by: Xida Chen <xidachen@chromium.org>
> Reviewed-by: Timothy Dresser <tdresser@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#579074}

TBR=creis@chromium.org,tdresser@chromium.org,xidachen@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 851644
Change-Id: I8d1c4753963f22e55f1b338bf2b264376b683660
Reviewed-on: https://chromium-review.googlesource.com/1157036Reviewed-by: default avatarXida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579540}
parent 687b3af4
...@@ -52,7 +52,9 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent( ...@@ -52,7 +52,9 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
DCHECK(!suppress_manipulation_events_); DCHECK(!suppress_manipulation_events_);
DCHECK(!touchscreen_scroll_in_progress_); DCHECK(!touchscreen_scroll_in_progress_);
touchscreen_scroll_in_progress_ = true; touchscreen_scroll_in_progress_ = true;
DCHECK(scrolling_touch_action_.has_value()); // TODO(https://crbug.com/851644): Make sure the value is properly set.
if (!scrolling_touch_action_.has_value())
SetTouchAction(cc::kTouchActionAuto);
suppress_manipulation_events_ = suppress_manipulation_events_ =
ShouldSuppressManipulation(*gesture_event); ShouldSuppressManipulation(*gesture_event);
return suppress_manipulation_events_ return suppress_manipulation_events_
...@@ -116,6 +118,9 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent( ...@@ -116,6 +118,9 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
// If double tap is disabled, there's no reason for the tap delay. // If double tap is disabled, there's no reason for the tap delay.
case WebInputEvent::kGestureTapUnconfirmed: { case WebInputEvent::kGestureTapUnconfirmed: {
DCHECK_EQ(1, gesture_event->data.tap.tap_count); DCHECK_EQ(1, gesture_event->data.tap.tap_count);
// TODO(https://crbug.com/851644): Make sure the value is properly set.
if (!scrolling_touch_action_.has_value())
SetTouchAction(cc::kTouchActionAuto);
allow_current_double_tap_event_ = (scrolling_touch_action_.value() & allow_current_double_tap_event_ = (scrolling_touch_action_.value() &
cc::kTouchActionDoubleTapZoom) != 0; cc::kTouchActionDoubleTapZoom) != 0;
if (!allow_current_double_tap_event_) { if (!allow_current_double_tap_event_) {
...@@ -139,6 +144,11 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent( ...@@ -139,6 +144,11 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
if (gesture_event->is_source_touch_event_set_non_blocking) if (gesture_event->is_source_touch_event_set_non_blocking)
SetTouchAction(cc::kTouchActionAuto); SetTouchAction(cc::kTouchActionAuto);
scrolling_touch_action_ = allowed_touch_action_; scrolling_touch_action_ = allowed_touch_action_;
// TODO(https://crbug.com/851644): The value may not set in the case when
// the gesture event is flushed due to touch ack time out after the finger
// is lifted up. Make sure the value is properly set.
if (!scrolling_touch_action_.has_value())
SetTouchAction(cc::kTouchActionAuto);
DCHECK(!drop_current_tap_ending_event_); DCHECK(!drop_current_tap_ending_event_);
break; break;
......
...@@ -888,8 +888,8 @@ TEST_F(TouchActionFilterTest, TouchActionNotResetWithinGestureSequence) { ...@@ -888,8 +888,8 @@ TEST_F(TouchActionFilterTest, TouchActionNotResetWithinGestureSequence) {
EXPECT_EQ(filter_.FilterGestureEvent(&tap_down), EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
FilterGestureEventResult::kFilterGestureEventAllowed); FilterGestureEventResult::kFilterGestureEventAllowed);
EXPECT_FALSE(filter_.allowed_touch_action().has_value()); EXPECT_TRUE(filter_.allowed_touch_action().has_value());
EXPECT_FALSE(ScrollingTouchAction().has_value()); EXPECT_TRUE(ScrollingTouchAction().has_value());
} }
TEST_F(TouchActionFilterTest, TouchpadScroll) { TEST_F(TouchActionFilterTest, TouchpadScroll) {
......
...@@ -1197,11 +1197,6 @@ void RenderWidgetHostInputEventRouter::DispatchTouchscreenGestureEvent( ...@@ -1197,11 +1197,6 @@ void RenderWidgetHostInputEventRouter::DispatchTouchscreenGestureEvent(
map_size_key, map_size_key,
base::StringPrintf("%u", static_cast<int>(owner_map_.size()))); base::StringPrintf("%u", static_cast<int>(owner_map_.size())));
if (events_being_flushed_) {
touchscreen_gesture_target_.target->host()
->input_router()
->ForceSetTouchActionAuto();
}
touchscreen_gesture_target_.target->ProcessGestureEvent(event, latency); touchscreen_gesture_target_.target->ProcessGestureEvent(event, latency);
if (gesture_event.GetType() == blink::WebInputEvent::kGestureFlingStart) if (gesture_event.GetType() == blink::WebInputEvent::kGestureFlingStart)
...@@ -1373,11 +1368,6 @@ RenderWidgetHostInputEventRouter::FindTargetSynchronously( ...@@ -1373,11 +1368,6 @@ RenderWidgetHostInputEventRouter::FindTargetSynchronously(
return RenderWidgetTargetResult(); return RenderWidgetTargetResult();
} }
void RenderWidgetHostInputEventRouter::SetEventsBeingFlushed(
bool events_being_flushed) {
events_being_flushed_ = events_being_flushed;
}
void RenderWidgetHostInputEventRouter::DispatchEventToTarget( void RenderWidgetHostInputEventRouter::DispatchEventToTarget(
RenderWidgetHostViewBase* root_view, RenderWidgetHostViewBase* root_view,
RenderWidgetHostViewBase* target, RenderWidgetHostViewBase* target,
......
...@@ -280,9 +280,6 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter ...@@ -280,9 +280,6 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
const blink::WebInputEvent& event, const blink::WebInputEvent& event,
const ui::LatencyInfo& latency, const ui::LatencyInfo& latency,
const base::Optional<gfx::PointF>& target_location) override; const base::Optional<gfx::PointF>& target_location) override;
// Notify whether the events in the queue are being flushed due to touch ack
// timeout, or the flushing has completed.
void SetEventsBeingFlushed(bool events_being_flushed) override;
FrameSinkIdOwnerMap owner_map_; FrameSinkIdOwnerMap owner_map_;
TargetMap touchscreen_gesture_target_map_; TargetMap touchscreen_gesture_target_map_;
...@@ -322,7 +319,6 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter ...@@ -322,7 +319,6 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
std::unique_ptr<RenderWidgetTargeter> event_targeter_; std::unique_ptr<RenderWidgetTargeter> event_targeter_;
bool use_viz_hit_test_ = false; bool use_viz_hit_test_ = false;
bool events_being_flushed_ = false;
std::unique_ptr<TouchEmulator> touch_emulator_; std::unique_ptr<TouchEmulator> touch_emulator_;
......
...@@ -215,7 +215,6 @@ void RenderWidgetTargeter::QueryClient( ...@@ -215,7 +215,6 @@ void RenderWidgetTargeter::QueryClient(
} }
void RenderWidgetTargeter::FlushEventQueue() { void RenderWidgetTargeter::FlushEventQueue() {
bool events_being_flushed = false;
while (!request_in_flight_ && !requests_.empty()) { while (!request_in_flight_ && !requests_.empty()) {
auto request = std::move(requests_.front()); auto request = std::move(requests_.front());
requests_.pop(); requests_.pop();
...@@ -225,16 +224,9 @@ void RenderWidgetTargeter::FlushEventQueue() { ...@@ -225,16 +224,9 @@ void RenderWidgetTargeter::FlushEventQueue() {
continue; continue;
} }
request.tracker->Stop(); request.tracker->Stop();
// Only notify the delegate once that the current event queue is being
// flushed. Once all the events are flushed, notify the delegate again.
if (!events_being_flushed) {
delegate_->SetEventsBeingFlushed(true);
events_being_flushed = true;
}
FindTargetAndDispatch(request.root_view.get(), *request.event, FindTargetAndDispatch(request.root_view.get(), *request.event,
request.latency); request.latency);
} }
delegate_->SetEventsBeingFlushed(false);
} }
void RenderWidgetTargeter::FoundFrameSinkId( void RenderWidgetTargeter::FoundFrameSinkId(
......
...@@ -70,8 +70,6 @@ class RenderWidgetTargeter { ...@@ -70,8 +70,6 @@ class RenderWidgetTargeter {
const ui::LatencyInfo& latency, const ui::LatencyInfo& latency,
const base::Optional<gfx::PointF>& target_location) = 0; const base::Optional<gfx::PointF>& target_location) = 0;
virtual void SetEventsBeingFlushed(bool events_being_flushed) = 0;
virtual RenderWidgetHostViewBase* FindViewFromFrameSinkId( virtual RenderWidgetHostViewBase* FindViewFromFrameSinkId(
const viz::FrameSinkId& frame_sink_id) const = 0; const viz::FrameSinkId& frame_sink_id) const = 0;
}; };
......
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
#include "components/viz/common/features.h" #include "components/viz/common/features.h"
#include "content/browser/compositor/surface_utils.h" #include "content/browser/compositor/surface_utils.h"
#include "content/browser/renderer_host/cursor_manager.h" #include "content/browser/renderer_host/cursor_manager.h"
#include "content/browser/renderer_host/input/synthetic_smooth_scroll_gesture.h"
#include "content/browser/renderer_host/input/synthetic_tap_gesture.h" #include "content/browser/renderer_host/input/synthetic_tap_gesture.h"
#include "content/browser/renderer_host/input/touch_emulator.h" #include "content/browser/renderer_host/input/touch_emulator.h"
#include "content/browser/renderer_host/render_widget_host_input_event_router.h" #include "content/browser/renderer_host/render_widget_host_input_event_router.h"
...@@ -1315,99 +1314,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessEmulatedTouchBrowserTest, ...@@ -1315,99 +1314,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessEmulatedTouchBrowserTest,
RunTest(TouchActionBubbling); RunTest(TouchActionBubbling);
} }
#if defined(OS_ANDROID) || defined(USE_AURA)
namespace {
// This function is used in TouchActionAckTimeout and
// SubframeGestureEventRouting, which is defined either under Android or Aura.
void OnSyntheticGestureCompleted(scoped_refptr<MessageLoopRunner> runner,
SyntheticGesture::Result result) {
EXPECT_EQ(SyntheticGesture::GESTURE_FINISHED, result);
runner->Quit();
}
} // namespace
#endif // defined(OS_ANDROID) || defined(USE_AURA)
// Regression test for https://crbug.com/851644. The test passes as long as it
// doesn't crash.
// Touch action ack timeout is enabled on Android only.
#if defined(OS_ANDROID)
IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
TouchActionAckTimeout) {
GURL main_url(
embedded_test_server()->GetURL("/frame_tree/page_with_janky_frame.html"));
ASSERT_TRUE(NavigateToURL(shell(), main_url));
FrameTreeNode* root = web_contents()->GetFrameTree()->root();
ASSERT_EQ(1U, root->child_count());
GURL frame_url(embedded_test_server()->GetURL(
"baz.com", "/page_with_touch_start_janking_main_thread.html"));
auto* child_frame_host = root->child_at(0)->current_frame_host();
RenderWidgetHostViewBase* rwhv_root = static_cast<RenderWidgetHostViewBase*>(
root->current_frame_host()->GetRenderWidgetHost()->GetView());
RenderWidgetHostViewChildFrame* rwhv_child =
static_cast<RenderWidgetHostViewChildFrame*>(
child_frame_host->GetRenderWidgetHost()->GetView());
WaitForHitTestDataOrChildSurfaceReady(child_frame_host);
// Compute the point so that the gesture event can target the child frame.
const gfx::Rect root_bounds = rwhv_root->GetViewBounds();
const gfx::Rect child_bounds = rwhv_child->GetViewBounds();
RenderFrameSubmissionObserver render_frame_submission_observer(
shell()->web_contents());
const float page_scale_factor =
render_frame_submission_observer.LastRenderFrameMetadata()
.page_scale_factor;
const gfx::PointF point_in_child(
(child_bounds.x() - root_bounds.x() + 25) * page_scale_factor,
(child_bounds.y() - root_bounds.y() + 25) * page_scale_factor);
SyntheticSmoothScrollGestureParams params;
params.gesture_source_type = SyntheticGestureParams::TOUCH_INPUT;
params.anchor = gfx::PointF(point_in_child.x(), point_in_child.y());
params.distances.push_back(gfx::Vector2dF(0, -10));
// Make this scroll slow so that the second scroll will be queued even before
// this one ends.
params.speed_in_pixels_s = 1000;
std::unique_ptr<SyntheticSmoothScrollGesture> gesture(
new SyntheticSmoothScrollGesture(params));
scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner();
RenderWidgetHostImpl* render_widget_host =
root->current_frame_host()->GetRenderWidgetHost();
render_widget_host->QueueSyntheticGesture(
std::move(gesture), base::BindOnce(OnSyntheticGestureCompleted, runner));
// The first gesture takes 100ms, so wait for 120ms to ensure that it has
// finished.
runner->Run();
std::unique_ptr<RenderFrameSubmissionObserver> frame_observer =
std::make_unique<RenderFrameSubmissionObserver>(
child_frame_host->GetRenderWidgetHost()->render_frame_metadata_provider());
frame_observer->WaitForAnyFrameSubmission();
SyntheticSmoothScrollGestureParams params2;
params2.gesture_source_type = SyntheticGestureParams::TOUCH_INPUT;
params2.anchor = gfx::PointF(point_in_child.x(), point_in_child.y());
params2.distances.push_back(gfx::Vector2dF(0, -10));
params2.speed_in_pixels_s = 100000;
std::unique_ptr<SyntheticSmoothScrollGesture> gesture2(
new SyntheticSmoothScrollGesture(params2));
render_widget_host->QueueSyntheticGesture(
std::move(gesture2), base::BindOnce(OnSyntheticGestureCompleted, runner));
runner->Run();
runner = nullptr;
// Give enough time to make sure all gesture are flushed and handled.
base::RunLoop run_loop;
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, run_loop.QuitClosure(),
base::TimeDelta::FromMilliseconds(2500));
run_loop.Run();
}
#endif // defined(OS_ANDROID)
#if defined(USE_AURA) || defined(OS_ANDROID) #if defined(USE_AURA) || defined(OS_ANDROID)
// When unconsumed scrolls in a child bubble to the root and start an // When unconsumed scrolls in a child bubble to the root and start an
...@@ -3142,6 +3048,17 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest, ...@@ -3142,6 +3048,17 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
render_widget_host->input_router()->AllowedTouchAction()); render_widget_host->input_router()->AllowedTouchAction());
} }
namespace {
// Declared here to be close to the SubframeGestureEventRouting test.
void OnSyntheticGestureCompleted(scoped_refptr<MessageLoopRunner> runner,
SyntheticGesture::Result result) {
EXPECT_EQ(SyntheticGesture::GESTURE_FINISHED, result);
runner->Quit();
}
} // anonymous namespace
// https://crbug.com/592320 // https://crbug.com/592320
IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest, IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
DISABLED_SubframeGestureEventRouting) { DISABLED_SubframeGestureEventRouting) {
......
<!DOCTYPE html>
<style>
.container {
touch-action: pan-y;
}
iframe {
position:absolute;
top: 50px;
left: 50px;
width: 100px;
height: 100px;
}
</style>
<html>
<body>
<div class="container">
<iframe src="/cross-site/baz.com/page_with_touch_start_janking_main_thread.html"></iframe>
</div>
This page contains a positioned cross-origin iframe.
</body>
</html>
<html>
<style>
#janktest{
width: 1000px;
height: 10000px;
}
</style>
<body onload='setup()'>
<div id="janktest"></div>
</body>
<script>
function setup() {
janktest.ontouchstart = function() {
var end = performance.now() + 1200;
while (performance.now() < end) ;
};
}
</script>
</html>
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