CustomEvent.detail should use ScriptValue::V8ValueFor |
||
Issue descriptionThe CustomEvent stores details attribute as TraceWrapperV8Reference but it's not good as per Yuki's comment[1]. We should store the value using ScriptValue and return it using ScriptValue::V8ValueFor() instead. The following link is very similar example. https://chromium-review.googlesource.com/c/chromium/src/+/1226774/4..8 [1] https://chromium-review.googlesource.com/c/chromium/src/+/1226774/4/third_party/blink/renderer/modules/payments/payment_method_change_event.h#45
,
Oct 3
Sure. Go ahead!
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/162bfa65f8211ebcd391f62d326c54d7fe9cd2f3 commit 162bfa65f8211ebcd391f62d326c54d7fe9cd2f3 Author: Haeun Kim <ggrace.kim93@gmail.com> Date: Thu Oct 04 19:13:13 2018 Events: CustomEvent.detail should use ScriptValue::V8ValueFor Storing details as TraceWrapperV8Reference is not good. We should use ScriptValue and return it using V8ValueFor(). Bug: 891598 Change-Id: I7ff310ed6fda629ed316e99e8ac13fd10b5aed53 Reviewed-on: https://chromium-review.googlesource.com/c/1257472 Reviewed-by: Jinho Bang <jinho.bang@samsung.com> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Commit-Queue: Jinho Bang <jinho.bang@samsung.com> Cr-Commit-Position: refs/heads/master@{#596795} [modify] https://crrev.com/162bfa65f8211ebcd391f62d326c54d7fe9cd2f3/third_party/blink/renderer/core/dom/events/custom_event.cc [modify] https://crrev.com/162bfa65f8211ebcd391f62d326c54d7fe9cd2f3/third_party/blink/renderer/core/dom/events/custom_event.h
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e54c83602137a9dd83c06bd65b97dce90d045645 commit e54c83602137a9dd83c06bd65b97dce90d045645 Author: Marijn Kruisselbrink <mek@chromium.org> Date: Thu Oct 04 21:30:08 2018 Revert "Events: CustomEvent.detail should use ScriptValue::V8ValueFor" This reverts commit 162bfa65f8211ebcd391f62d326c54d7fe9cd2f3. Reason for revert: Seems to be causing leaks in https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/24959 Original change's description: > Events: CustomEvent.detail should use ScriptValue::V8ValueFor > > Storing details as TraceWrapperV8Reference is not good. > We should use ScriptValue and return it using V8ValueFor(). > > Bug: 891598 > Change-Id: I7ff310ed6fda629ed316e99e8ac13fd10b5aed53 > Reviewed-on: https://chromium-review.googlesource.com/c/1257472 > Reviewed-by: Jinho Bang <jinho.bang@samsung.com> > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Reviewed-by: Yuki Shiino <yukishiino@chromium.org> > Commit-Queue: Jinho Bang <jinho.bang@samsung.com> > Cr-Commit-Position: refs/heads/master@{#596795} TBR=yukishiino@chromium.org,jinho.bang@samsung.com,haraken@chromium.org,ggrace.kim93@gmail.com Change-Id: I354de04091d5a8a40e99a6a5ea072557b84cc790 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 891598 Reviewed-on: https://chromium-review.googlesource.com/c/1262968 Reviewed-by: Marijn Kruisselbrink <mek@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/master@{#596861} [modify] https://crrev.com/e54c83602137a9dd83c06bd65b97dce90d045645/third_party/blink/renderer/core/dom/events/custom_event.cc [modify] https://crrev.com/e54c83602137a9dd83c06bd65b97dce90d045645/third_party/blink/renderer/core/dom/events/custom_event.h |
||
►
Sign in to add a comment |
||
Comment 1 by haeun...@gmail.com
, Oct 3