[CleanUp] Clean-up duplicate codes with ScriptValue::V8ValueFor |
||||
Issue descriptionPopStateEvent: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/events/PopStateEvent.cpp?rcl=802974c24ad833fe7b4d4856342cdf7ed103374c&l=56 PerformanceMark: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/timing/PerformanceMark.cpp?rcl=802974c24ad833fe7b4d4856342cdf7ed103374c&l=23 CustomEvent: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/events/CustomEvent.cpp?rcl=802974c24ad833fe7b4d4856342cdf7ed103374c&l=59 These classes share the same blocks of code that can be cleaned up with ScriptValue::V8ValueFor.
,
Jan 19 2018
We have a plan to make ScriptState be GarbagedCollected, and in this course, we do not want ScriptValue to have a ScriptState (or, ScriptValue should be GarbagedCollected, too), and I'm currently wondering if we really need ScriptValue or not (i.e. can we retire ScriptValue?). Cloning v8::Value across worlds is quite important, and the bindings layer should provide an utility function at least. However, we maybe do not need ScriptValue itself (I'm not convinced yet, though). Peria@ will be working on this point once he finish the context snapshot project.
,
Jan 19 2018
From what I remember, the greater idea was to retire ScriptValue if possible. IIUC, it currently holds a strong persistent into V8 which can easily cause leaks. That said, I don't know whether it is possible to get around without it.
,
Feb 15 2018
Yuki suggested another great idea to make ScriptValue inherit from GCed<>, then we can depend on the infrastructure of ToV8().
,
Feb 15 2018
s/GCed<>/ScriptWrappable/
,
May 30 2018
Ping - peria@ any update on this bug?
,
May 30 2018
No updates. I have no active working plan for this issue now.
,
Jul 4
,
Oct 15
peria@, any update on when you might get to this bug?
,
Oct 17
Jinho and I are now looking at an issue related to this. peria@, feel free to assign this issue to me if you're no longer interested. I'm also happy to help you design a new class to support "wrapper-tracing + world-safe" representation.
,
Oct 17
Thank you. I reassign this to yukishiino.
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/db73b059187ab4f4dbdb1fec6ac0e753ad565140 commit db73b059187ab4f4dbdb1fec6ac0e753ad565140 Author: Yuki Shiino <yukishiino@chromium.org> Date: Wed Dec 12 10:22:19 2018 v8binding: Introduce WorldSafeV8Reference class WorldSafeV8Reference class provides safe access across worlds plus wrapper-tracing. The usage is demonstrated at CustomEvent.detail. Change-Id: I6c640d916063ac87604bd63f25524f8a4f8dd817 Bug: 501866, 755520, 803478 Reviewed-on: https://chromium-review.googlesource.com/c/1358611 Commit-Queue: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Cr-Commit-Position: refs/heads/master@{#615844} [modify] https://crrev.com/db73b059187ab4f4dbdb1fec6ac0e753ad565140/third_party/blink/renderer/bindings/bindings.gni [add] https://crrev.com/db73b059187ab4f4dbdb1fec6ac0e753ad565140/third_party/blink/renderer/bindings/core/v8/world_safe_v8_reference.cc [add] https://crrev.com/db73b059187ab4f4dbdb1fec6ac0e753ad565140/third_party/blink/renderer/bindings/core/v8/world_safe_v8_reference.h [modify] https://crrev.com/db73b059187ab4f4dbdb1fec6ac0e753ad565140/third_party/blink/renderer/core/dom/events/custom_event.cc [modify] https://crrev.com/db73b059187ab4f4dbdb1fec6ac0e753ad565140/third_party/blink/renderer/core/dom/events/custom_event.h
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d579fc15d93d5fd9677bbfda8c5144569d27240 commit 8d579fc15d93d5fd9677bbfda8c5144569d27240 Author: Yuki Shiino <yukishiino@chromium.org> Date: Thu Dec 13 06:06:01 2018 v8binding: Unify ScriptValue::ToWorldSafeScriptValue to WorldSafeV8Reference Remove ScriptValue::ToWorldSafeScriptValue and replace its usage with WorldSafeV8Reference. Bug: 501866, 755520, 803478 Change-Id: I75f38dd4d4dd8c3730bec1e9d002fadaa236395b Reviewed-on: https://chromium-review.googlesource.com/c/1373894 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Commit-Queue: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#616228} [modify] https://crrev.com/8d579fc15d93d5fd9677bbfda8c5144569d27240/third_party/blink/renderer/bindings/core/v8/script_value.cc [modify] https://crrev.com/8d579fc15d93d5fd9677bbfda8c5144569d27240/third_party/blink/renderer/bindings/core/v8/script_value.h [modify] https://crrev.com/8d579fc15d93d5fd9677bbfda8c5144569d27240/third_party/blink/renderer/bindings/core/v8/world_safe_v8_reference.h [modify] https://crrev.com/8d579fc15d93d5fd9677bbfda8c5144569d27240/third_party/blink/renderer/modules/payments/payment_method_change_event.cc [modify] https://crrev.com/8d579fc15d93d5fd9677bbfda8c5144569d27240/third_party/blink/renderer/modules/payments/payment_method_change_event.h [modify] https://crrev.com/8d579fc15d93d5fd9677bbfda8c5144569d27240/third_party/blink/renderer/modules/payments/payment_response.cc [modify] https://crrev.com/8d579fc15d93d5fd9677bbfda8c5144569d27240/third_party/blink/renderer/modules/payments/payment_response.h |
||||
►
Sign in to add a comment |
||||
Comment 1 by jbroman@chromium.org
, Jan 18 2018