Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2759603002:
Check detached status before attempting to clear drag transfer state (reland.) (Closed)
DescriptionCheck detached status before attempting to clear drag transfer state (reland.)
Speculatively try to address failing access of dragState() when
clearing out state after 'dragend' has been dispatched.
R=dcheng
BUG=702516
Review-Url: https://codereview.chromium.org/2759603002
Cr-Original-Commit-Position: refs/heads/master@{#457993}
Committed: https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b34e4bc276171
Review-Url: https://codereview.chromium.org/2759603002
Cr-Commit-Position: refs/heads/master@{#458354}
Committed: https://chromium.googlesource.com/chromium/src/+/ead9f20d168c47838b2fe5f31ee0b22e6d213a64
Patch Set 1 #Patch Set 2 : add test #Patch Set 3 : rebased upto r458127 #
Messages
Total messages: 34 (21 generated)
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
sigbjornf@opera.com changed reviewers: + dcheng@chromium.org
please take a look. I tried writing a test where a window.dragend handler detaches from its frame, without being able to trigger what i speculate being the cause.
On 2017/03/17 13:23:29, sof wrote:
> please take a look.
>
> I tried writing a test where a window.dragend handler detaches from its frame,
> without being able to trigger what i speculate being the cause.
Something like this works for me:
<!DOCTYPE html>
<html>
<body>
<iframe srcdoc="<head><script>window.ondragend = function () {
parent.document.querySelector('iframe').remove(); }</script></head><body><a
href='#'>Drag Me</a></body>"></iframe>
</body>
</html>
as long as the drag is cancelled and doesn't trigger a navigation.
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/03/17 20:58:04, dcheng wrote:
> On 2017/03/17 13:23:29, sof wrote:
> > please take a look.
> >
> > I tried writing a test where a window.dragend handler detaches from its
frame,
> > without being able to trigger what i speculate being the cause.
>
> Something like this works for me:
>
> <!DOCTYPE html>
> <html>
> <body>
> <iframe srcdoc="<head><script>window.ondragend = function () {
> parent.document.querySelector('iframe').remove(); }</script></head><body><a
> href='#'>Drag Me</a></body>"></iframe>
> </body>
> </html>
>
> as long as the drag is cancelled and doesn't trigger a navigation.
Cheers, added a test (window.eventSender()'s coordinate handling confused me
initially when working the iframe. )
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Description was changed from ========== Check detached status before attempting to clear drag transfer state. Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R= BUG=702516 ========== to ========== Check detached status before attempting to clear drag transfer state. Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 ==========
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489903978178830,
"parent_rev": "16c516962e61ee86833d3a7b9369bb4d3182e684", "commit_rev":
"a995166eab58770b85268173bf9b34e4bc276171"}
Message was sent while issue was closed.
Description was changed from ========== Check detached status before attempting to clear drag transfer state. Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 ========== to ========== Check detached status before attempting to clear drag transfer state. Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 Review-Url: https://codereview.chromium.org/2759603002 Cr-Commit-Position: refs/heads/master@{#457993} Committed: https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b...
Message was sent while issue was closed.
In case anyone comes looking for the leak source for fast/events/drag-dragend-detaches.html, https://codereview.chromium.org/2762613002/ addresses.
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2759143002/ by vasilii@chromium.org. The reason for reverting is: The new test fails consistently on WebKit Linux Trusty Leak. 02:51:43.187 21593 worker/2 fast/events/drag-dragend-detaches.html leaked 02:51:43.188 21496 [37229/51558] fast/events/drag-dragend-detaches.html failed unexpectedly (leak detected: ({"numberOfLiveDocuments":[1,2],"numberOfLiveNodes":[4,15],"numberOfLiveSuspendableObjects":[2,3]})) See https://crbug.com/703057.
Message was sent while issue was closed.
Description was changed from ========== Check detached status before attempting to clear drag transfer state. Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 Review-Url: https://codereview.chromium.org/2759603002 Cr-Commit-Position: refs/heads/master@{#457993} Committed: https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b... ========== to ========== Check detached status before attempting to clear drag transfer state. Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 Review-Url: https://codereview.chromium.org/2759603002 Cr-Commit-Position: refs/heads/master@{#457993} Committed: https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b... ==========
Description was changed from ========== Check detached status before attempting to clear drag transfer state. Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 Review-Url: https://codereview.chromium.org/2759603002 Cr-Commit-Position: refs/heads/master@{#457993} Committed: https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b... ========== to ========== Check detached status before attempting to clear drag transfer state (reland.) Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 Review-Url: https://codereview.chromium.org/2759603002 Cr-Commit-Position: refs/heads/master@{#457993} Committed: https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b... ==========
With https://crrev.com/458116 in scope, relanding. No leaks reproducible locally.
The CQ bit was checked by sigbjornf@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2759603002/#ps40001 (title: "rebased upto r458127")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490082313580920,
"parent_rev": "26d6df1c82a043e0e094d3545443e2db7c7f7193", "commit_rev":
"ead9f20d168c47838b2fe5f31ee0b22e6d213a64"}
Message was sent while issue was closed.
Description was changed from ========== Check detached status before attempting to clear drag transfer state (reland.) Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 Review-Url: https://codereview.chromium.org/2759603002 Cr-Commit-Position: refs/heads/master@{#457993} Committed: https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b... ========== to ========== Check detached status before attempting to clear drag transfer state (reland.) Speculatively try to address failing access of dragState() when clearing out state after 'dragend' has been dispatched. R=dcheng BUG=702516 Review-Url: https://codereview.chromium.org/2759603002 Cr-Original-Commit-Position: refs/heads/master@{#457993} Committed: https://chromium.googlesource.com/chromium/src/+/a995166eab58770b85268173bf9b... Review-Url: https://codereview.chromium.org/2759603002 Cr-Commit-Position: refs/heads/master@{#458354} Committed: https://chromium.googlesource.com/chromium/src/+/ead9f20d168c47838b2fe5f31ee0... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/ead9f20d168c47838b2fe5f31ee0... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
