Commit 60726a37 authored by Ken Buchanan's avatar Ken Buchanan Committed by Commit Bot

Widen guard against accumulating input events under wrong frame root

This modifies a recent change that prevents accumulation of input
events under the wrong local frame root. The original CL only
guarded against recursion into frames that were under a different
root, whereas crashes can still occur from target elements that are
directly under the OOPIF.

Bug: 792687
Change-Id: Iabb5ab41ef5bf8dc429c591358d7b169f921f088
Reviewed-on: https://chromium-review.googlesource.com/820070
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523650}
parent 510fa119
...@@ -9221,6 +9221,34 @@ TEST_P(WebFrameSwapTest, EventsOnDisconnectedSubDocumentSkipped) { ...@@ -9221,6 +9221,34 @@ TEST_P(WebFrameSwapTest, EventsOnDisconnectedSubDocumentSkipped) {
main_document->View()->UpdateAllLifecyclePhases(); main_document->View()->UpdateAllLifecyclePhases();
} }
TEST_P(WebFrameSwapTest, EventsOnDisconnectedElementSkipped) {
WebRemoteFrame* remote_frame = FrameTestHelpers::CreateRemote();
WebFrame* target_frame = MainFrame()->FirstChild()->NextSibling();
EXPECT_TRUE(target_frame);
SwapAndVerifySubframeConsistency("local->remote", target_frame, remote_frame);
WebLocalFrameImpl* local_child =
FrameTestHelpers::CreateLocalChild(*remote_frame, "local-inside-remote");
Document* main_document =
WebView()->MainFrameImpl()->GetFrame()->GetDocument();
// Layout ensures that elements in the local_child frame get LayoutObjects
// attached, but doesn't paint, because the child frame needs to not have
// been composited for the purpose of this test.
local_child->GetFrameView()->UpdateLayout();
Document* child_document = local_child->GetFrame()->GetDocument();
EventHandlerRegistry& event_registry =
main_document->GetPage()->GetEventHandlerRegistry();
// Add the non-connected body element as having an event.
event_registry.DidAddEventHandler(
*child_document->body(),
EventHandlerRegistry::kTouchStartOrMoveEventBlocking);
// Passes if this does not crash or DCHECK.
main_document->View()->UpdateAllLifecyclePhases();
}
TEST_P(WebFrameSwapTest, SwapParentShouldDetachChildren) { TEST_P(WebFrameSwapTest, SwapParentShouldDetachChildren) {
WebRemoteFrame* remote_frame = FrameTestHelpers::CreateRemote(); WebRemoteFrame* remote_frame = FrameTestHelpers::CreateRemote();
WebFrame* target_frame = MainFrame()->FirstChild()->NextSibling(); WebFrame* target_frame = MainFrame()->FirstChild()->NextSibling();
......
...@@ -1098,17 +1098,16 @@ static void AccumulateDocumentTouchEventTargetRects( ...@@ -1098,17 +1098,16 @@ static void AccumulateDocumentTouchEventTargetRects(
node->GetDocument().View()->ShouldThrottleRendering()) node->GetDocument().View()->ShouldThrottleRendering())
continue; continue;
// Ignore events which apply to a different LocalFrameRoot, as they have
// their own lifecycle. Any events that apply to them will get processed
// accordingly.
if (node->GetDocument().GetFrame()->LocalFrameRoot() !=
document->GetFrame()->LocalFrameRoot())
continue;
if (node->IsDocumentNode() && node != document) { if (node->IsDocumentNode() && node != document) {
Document* child_document = ToDocument(node); AccumulateDocumentTouchEventTargetRects(
// Ignore events which apply to rects, event_class, ToDocument(node), supported_fast_actions);
// a different LocalFrameRoot, as they have their own lifecycle.
// Any events that apply to them will get processed accordingly.
if (child_document->GetFrame() &&
&child_document->GetFrame()->LocalFrameRoot() ==
&document->GetFrame()->LocalFrameRoot()) {
AccumulateDocumentTouchEventTargetRects(
rects, event_class, child_document, supported_fast_actions);
}
} else if (LayoutObject* layout_object = node->GetLayoutObject()) { } else if (LayoutObject* layout_object = node->GetLayoutObject()) {
// If the set also contains one of our ancestor nodes then processing // If the set also contains one of our ancestor nodes then processing
// this node would be redundant. // this node would be redundant.
......
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