Skip to content

GitLab

T
tangled
Commit 5af8e7eb authored by Adithya Srinivasan's avatar Adithya Srinivasan Committed by Commit Bot
Browse files
Options

Revert "Reland "Check page exist after dispatch dragstart"" 

This reverts commit 67834de4.

Reason for revert: Causing failures on WebKit Linux Trusty Leak (https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/19550)

Original change's description:
> Reland "Check page exist after dispatch dragstart"
> 
> This is a reland of 2c03b1bd
> 
> The CL was reverted because of failures on the leak bot.
> However the leak is not cause by the test or the patch.
> Add the exception for the test for further investigation.
> 
> Original change's description:
> > Check page exist after dispatch dragstart
> >
> > frame_->GetPage() may be null. need to check before use GetDragState
> > This CL reorder the some checks in MouseEventManager::TryStartDrag
> > to make sure GetPage is valid before start drag.
> >
> > Bug: 843502
> > Change-Id: Ifdf5b20d7132ca4b089c9a5b7652ebbd41370c33
> > Reviewed-on: https://chromium-review.googlesource.com/1064878
> > Commit-Queue: Ella Ge <eirage@chromium.org>
> > Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
> > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#561262}
> 
> Bug: 843502
> Change-Id: I093da085a84eecf2937d522b762b2feeeb1d7947
> Reviewed-on: https://chromium-review.googlesource.com/1079247
> Commit-Queue: Ella Ge <eirage@chromium.org>
> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#562953}

TBR=dcheng@chromium.org,nzolghadr@chromium.org,eirage@chromium.org

Change-Id: I5546c2b3469ecbf1ad40cc47ab8437075073abd5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 843502
Reviewed-on: https://chromium-review.googlesource.com/1079968Reviewed-by: default avatarAdithya Srinivasan <adithyas@chromium.org>
Commit-Queue: Adithya Srinivasan <adithyas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562991}
parent 18c9afdf
Hide whitespace changes
Inline Side-by-side
Showing with 24 additions and 56 deletions
third_party/WebKit/LayoutTests/LeakExpectations
View file @ 5af8e7eb
...@@ -75,8 +75,6 @@ crbug.com/835802 [ Linux ] fast/loader/scroll-position-restored-on-back-at-load- ...@@ -75,8 +75,6 @@ crbug.com/835802 [ Linux ] fast/loader/scroll-position-restored-on-back-at-load-
crbug.com/836278 [ Linux ] external/wpt/offscreen-canvas/convert-to-blob/offscreencanvas.convert.to.blob.html [ Pass Leak ] crbug.com/836278 [ Linux ] external/wpt/offscreen-canvas/convert-to-blob/offscreencanvas.convert.to.blob.html [ Pass Leak ]
crbug.com/836278 [ Linux ] virtual/threaded/external/wpt/offscreen-canvas/convert-to-blob/offscreencanvas.convert.to.blob.html [ Pass Leak ] crbug.com/836278 [ Linux ] virtual/threaded/external/wpt/offscreen-canvas/convert-to-blob/offscreencanvas.convert.to.blob.html [ Pass Leak ]
crbug.com/847868 [ Linux ] fast/events/drag-remove-iframe-crash.html [ Pass Leak ]
########################################################################### ###########################################################################
# WARNING: Memory leaks must be fixed asap. Sheriff is expected to revert # # WARNING: Memory leaks must be fixed asap. Sheriff is expected to revert #
# culprit CLs instead of suppressing the leaks. If you have any question, # # culprit CLs instead of suppressing the leaks. If you have any question, #
......
third_party/WebKit/LayoutTests/fast/events/drag-remove-iframe-crash.html deleted 100644 → 0
View file @ 18c9afdf
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<body onload="run_test()"> </body>
<script>
var t = async_test("Remove iframe when start dragging should not crash")
function run_test() {
var fr = document.createElement('iframe');
document.body.appendChild(fr);
fr.contentDocument.body.innerHTML = '<div draggable="true" id="target" style="margin: 0px; width: 100px; height: 100px;background-color:red"></div>';
fr.contentDocument.getElementById("target").ondragstart = function(event) {
document.body.removeChild(fr);
t.done();
};
if (window.eventSender) {
eventSender.mouseMoveTo(50, 50);
eventSender.mouseDown();
eventSender.mouseMoveTo(80, 80);
eventSender.mouseUp();
}
}
</script>
\ No newline at end of file
third_party/blink/renderer/core/input/mouse_event_manager.cc
View file @ 5af8e7eb
...@@ -970,44 +970,40 @@ bool MouseEventManager::TryStartDrag( ...@@ -970,44 +970,40 @@ bool MouseEventManager::TryStartDrag(
mouse_down_pos_)) mouse_down_pos_))
return false; return false;
if (DispatchDragSrcEvent(EventTypeNames::dragstart, mouse_down_) !=
WebInputEventResult::kNotHandled)
return false;
// Dispatching the event could cause |frame_| to be detached.
if (!frame_->GetPage())
return false;
// If dispatching dragstart brings about another mouse down -- one way // If dispatching dragstart brings about another mouse down -- one way
// this will happen is if a DevTools user breaks within a dragstart // this will happen is if a DevTools user breaks within a dragstart
// handler and then clicks on the suspended page -- the drag state is // handler and then clicks on the suspended page -- the drag state is
// reset. Hence, need to check if this particular drag operation can // reset. Hence, need to check if this particular drag operation can
// continue even if dispatchEvent() indicates no (direct) cancellation. // continue even if dispatchEvent() indicates no (direct) cancellation.
// Do that by checking if m_dragSrc is still set. // Do that by checking if m_dragSrc is still set.
if (!GetDragState().drag_src_) mouse_down_may_start_drag_ = false;
return false; if (DispatchDragSrcEvent(EventTypeNames::dragstart, mouse_down_) ==
WebInputEventResult::kNotHandled &&
// Do not start dragging in password field. GetDragState().drag_src_) {
// TODO(editing-dev): The use of // TODO(editing-dev): The use of
// updateStyleAndLayoutIgnorePendingStylesheets needs to be audited. See // updateStyleAndLayoutIgnorePendingStylesheets needs to be audited. See
// http://crbug.com/590369 for more details. // http://crbug.com/590369 for more details.
frame_->GetDocument()->UpdateStyleAndLayoutIgnorePendingStylesheets(); frame_->GetDocument()->UpdateStyleAndLayoutIgnorePendingStylesheets();
if (IsInPasswordField( mouse_down_may_start_drag_ = !IsInPasswordField(
frame_->Selection().ComputeVisibleSelectionInDOMTree().Start())) frame_->Selection().ComputeVisibleSelectionInDOMTree().Start());
return false; }
// Invalidate clipboard here against anymore pasteboard writing for // Invalidate clipboard here against anymore pasteboard writing for security.
// security. The drag image can still be changed as we drag, but not // The drag image can still be changed as we drag, but not the pasteboard
// the pasteboard data. // data.
GetDragState().drag_data_transfer_->SetAccessPolicy( GetDragState().drag_data_transfer_->SetAccessPolicy(
DataTransferAccessPolicy::kImageWritable); DataTransferAccessPolicy::kImageWritable);
if (drag_controller.StartDrag(frame_, GetDragState(), event.Event(), if (mouse_down_may_start_drag_) {
mouse_down_pos_)) // Dispatching the event could cause Page to go away. Make sure it's still
return true; // valid before trying to use DragController.
if (frame_->GetPage() &&
// Drag was canned at the last minute - we owe m_dragSrc a DRAGEND event drag_controller.StartDrag(frame_, GetDragState(), event.Event(),
DispatchDragSrcEvent(EventTypeNames::dragend, event.Event()); mouse_down_pos_))
return true;
// Drag was canned at the last minute - we owe m_dragSrc a DRAGEND event
DispatchDragSrcEvent(EventTypeNames::dragend, event.Event());
}
return false; return false;
} }
......
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