fast/events/drag-remove-iframe-crash.html failed on leak detection |
||||
Issue description
Layout test fast/events/drag-remove-iframe-crash.html failed when leak detection enabled.
leak log like
({"numberOfLiveDocuments":[1,2],"numberOfLiveFrames":[1,2],"numberOfLiveNodes":[4,9],"numberOfLivePausableObjects":[2,3],"numberOfLiveResourceFetchers":[1,2]})
The leak is not because of the test or the patch that added this test, but is an existing issue in drag and drop.
Changing the test to remove iframe in dragend event will also leak.
This might because ClearDragImage wasn't called from MouseEventManager::ClearDragDataTransfer() since the frame_ is detached. and somehow DataTransfer::drag_image_element_ causes the removed iframe not release correctly.
,
May 30 2018
fast/events/drag-remove-iframe-crash.html is landed in crrev.com/c/1079247
,
May 30 2018
dcheng@, Could you help take a look at this leak? You seem familiar with drag image in DataTransfer. Or could you help find a better owner? I can't figure out how to fix this one myself :)
,
May 30 2018
Yeah it seems likely that this leak is coming from some of the fields stored in DragState but I don't think there's an easy way to fix this atm. Ideally, I think that DragState would probably become a global that's independent from Page, but this would be a fairly involved change. (It's not a permanent leak since subsequent drags will force this to be cleaned up... but it's not great) Unassigning myself since I probably won't have cycles to work on this in the near feature.
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b4cbe3d9008a0a74f70f24b96314759de0afa329 commit b4cbe3d9008a0a74f70f24b96314759de0afa329 Author: Ella Ge <eirage@chromium.org> Date: Mon Jun 04 19:59:05 2018 Fix layout test leak fast/events/drag-remove-iframe-crash.html Layout test fast/events/drag-remove-iframe-crash.html was failed on leak bot because an existing issue with drag and drop data transfer. This CL fixes the leak by reset data transfer on MEM:Clear Bug: 847868 Change-Id: I5bf931c7086f971af43aa0a4bd40a34fe4f2ac71 Reviewed-on: https://chromium-review.googlesource.com/1082603 Commit-Queue: Ella Ge <eirage@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#564200} [modify] https://crrev.com/b4cbe3d9008a0a74f70f24b96314759de0afa329/third_party/WebKit/LayoutTests/LeakExpectations [modify] https://crrev.com/b4cbe3d9008a0a74f70f24b96314759de0afa329/third_party/blink/renderer/core/input/mouse_event_manager.cc
,
Jun 4 2018
The CL fixed the test leak. Although datatransfer still not reset immediately after page detached, but it should be fine since subsequent drags will cleaned it up as #4 pointed out. Closing this for now since the test is no leaking :) |
||||
►
Sign in to add a comment |
||||
Comment 1 by nzolghadr@chromium.org
, May 30 2018