Remove v8Call macros |
||||||
Issue descriptionWe have decided to deprecate v8Call macros and use To/ToLocal/ToChecked/ToLocalChecked instead. https://groups.google.com/a/chromium.org/d/topic/platform-architecture-dev/vGLGCaMqhCU/discussion
,
Oct 11 2017
,
Oct 11 2017
Looks like the remaining work to remove V8CallBoolean(). jbroman@ do you still have your clang plugin sitting around somewhere?
I'm assuming we want to end up with option b) below, is that right?
// Original
if (!V8CallBoolean(instance->HasOwnProperty(state->GetContext(), v8_name)))
return;
// Rewrite option a)
bool hasOwnProperty;
if (!instance->HasOwnProperty(state->GetContext(), v8_name).To(&hasOwnProperty) ||
!hasOwnProperty) {
return;
}
// Rewrite option b)
if (!instance->HasOwnProperty(state->GetContext(), v8_name).FromMaybe(false))
return;
,
Oct 11 2017
Our goal is a). Maybe/MaybeLocal are confusing, so we want to unify the API to To/ToLocal/ToChecked/ToLocalChecked.
,
Oct 11 2017
Hm, that's a shame. It's noisier and harder to automate because we have to introduce a local variable. Is the problem with b) a matter of naming? The semantics of FromMaybe() seem simple and useful here.
,
Oct 11 2017
It's for consistency. Other APIs that return Maybe/MaybeLocal handles are following the pattern a). The pattern b) works only for the boolean version.
,
Oct 12 2017
Ok, I think I'll leave this for now. It's not obvious to me how to do an automated refactor because we have to introduce local variables.
,
Oct 12 2017
I don't think automated refactoring is possible. (Also another side goal is to fix wrong usage of V8 APIs through the refactoring.)
,
Oct 12
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/707a2ce3a810dd3a8f139af75e7ccaf6e2055728 commit 707a2ce3a810dd3a8f139af75e7ccaf6e2055728 Author: Jeremy Roman <jbroman@chromium.org> Date: Tue Oct 16 20:24:38 2018 Move RTCStatsReport to use V8ObjectBuilder. Bug: 670615 Change-Id: Ia42b62d84082a431e615721cb0065d5d6b43fdd3 Reviewed-on: https://chromium-review.googlesource.com/c/1280856 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Henrik Boström <hbos@chromium.org> Commit-Queue: Jeremy Roman <jbroman@chromium.org> Cr-Commit-Position: refs/heads/master@{#600104} [modify] https://crrev.com/707a2ce3a810dd3a8f139af75e7ccaf6e2055728/third_party/blink/renderer/modules/peerconnection/rtc_stats_report.cc
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec6d5dcb9278eb812d4c0f1f61b6ed5acc374cf0 commit ec6d5dcb9278eb812d4c0f1f61b6ed5acc374cf0 Author: Jeremy Roman <jbroman@chromium.org> Date: Tue Oct 16 21:36:35 2018 Use V8ObjectBuilder for V8IteratorResultValue. Bug: 670615 Change-Id: I2a7592581cc251b9f1a21171416ea3492932e218 Reviewed-on: https://chromium-review.googlesource.com/c/1281047 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Commit-Queue: Jeremy Roman <jbroman@chromium.org> Cr-Commit-Position: refs/heads/master@{#600130} [modify] https://crrev.com/ec6d5dcb9278eb812d4c0f1f61b6ed5acc374cf0/third_party/blink/renderer/bindings/core/v8/v8_iterator_result_value.cc [modify] https://crrev.com/ec6d5dcb9278eb812d4c0f1f61b6ed5acc374cf0/third_party/blink/renderer/bindings/core/v8/v8_iterator_result_value.h
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/09060f57fa21f20f5a82676769c02dd8d4c74e87 commit 09060f57fa21f20f5a82676769c02dd8d4c74e87 Author: Jeremy Roman <jbroman@chromium.org> Date: Wed Oct 17 13:07:11 2018 bindings: Drop use of V8CallBoolean in generated dictionary code in favor of a lambda that uses v8::MaybeLocal::To. Bug: 670615 Change-Id: Ib4f1b5674cec44a328b484510b4b197b2afec21d Reviewed-on: https://chromium-review.googlesource.com/c/1281087 Commit-Queue: Jeremy Roman <jbroman@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#600369} [modify] https://crrev.com/09060f57fa21f20f5a82676769c02dd8d4c74e87/third_party/blink/renderer/bindings/templates/dictionary_v8.cc.tmpl [modify] https://crrev.com/09060f57fa21f20f5a82676769c02dd8d4c74e87/third_party/blink/renderer/bindings/tests/results/core/v8_test_dictionary.cc [modify] https://crrev.com/09060f57fa21f20f5a82676769c02dd8d4c74e87/third_party/blink/renderer/bindings/tests/results/core/v8_test_dictionary_derived.cc [modify] https://crrev.com/09060f57fa21f20f5a82676769c02dd8d4c74e87/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_event_init.cc [modify] https://crrev.com/09060f57fa21f20f5a82676769c02dd8d4c74e87/third_party/blink/renderer/bindings/tests/results/core/v8_test_permissive_dictionary.cc
,
Oct 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/62ae386e6e42e6999b9c393008181c6948a976c7 commit 62ae386e6e42e6999b9c393008181c6948a976c7 Author: Jeremy Roman <jbroman@chromium.org> Date: Thu Oct 18 14:01:12 2018 ScriptPromise: Use ToV8 to convert Vector<ScriptValue> to a V8 array. This eliminates the use of V8CallBoolean. Bug: 670615 Change-Id: Ifaaa803e47b08018546d30ad633f861f38a8d645 Reviewed-on: https://chromium-review.googlesource.com/c/1288673 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Jeremy Roman <jbroman@chromium.org> Cr-Commit-Position: refs/heads/master@{#600750} [modify] https://crrev.com/62ae386e6e42e6999b9c393008181c6948a976c7/third_party/blink/renderer/bindings/core/v8/script_promise.cc
,
Oct 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e64e8ebb00c28d5600533bba5c67d66e6256cd85 commit e64e8ebb00c28d5600533bba5c67d66e6256cd85 Author: Jeremy Roman <jbroman@chromium.org> Date: Wed Oct 24 15:43:13 2018 bindings: Don't use V8CallBoolean for [Replaceable] attributes. Bug: 670615 Change-Id: Ic386bad40225688c259613fde7f8d81de00044b5 Reviewed-on: https://chromium-review.googlesource.com/c/1288678 Commit-Queue: Jeremy Roman <jbroman@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#602348} [modify] https://crrev.com/e64e8ebb00c28d5600533bba5c67d66e6256cd85/third_party/blink/renderer/bindings/scripts/v8_attributes.py [modify] https://crrev.com/e64e8ebb00c28d5600533bba5c67d66e6256cd85/third_party/blink/renderer/bindings/templates/attributes.cc.tmpl [modify] https://crrev.com/e64e8ebb00c28d5600533bba5c67d66e6256cd85/third_party/blink/renderer/bindings/tests/results/core/v8_test_object.cc
,
Oct 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bbad11d3fd48fdeafc9b3984e7f6268108a6b236 commit bbad11d3fd48fdeafc9b3984e7f6268108a6b236 Author: Jeremy Roman <jbroman@chromium.org> Date: Wed Oct 24 20:00:14 2018 Remove use of V8CallBoolean in ImageData. This is deprecated in favor of explicitly checking with v8::Maybe<bool>::To. Bug: 670615 Change-Id: I71e8f3c1f643a7eabfaf25359581c15565513302 Reviewed-on: https://chromium-review.googlesource.com/c/1298049 Reviewed-by: Fernando Serboncini <fserb@chromium.org> Commit-Queue: Jeremy Roman <jbroman@chromium.org> Cr-Commit-Position: refs/heads/master@{#602408} [modify] https://crrev.com/bbad11d3fd48fdeafc9b3984e7f6268108a6b236/third_party/blink/renderer/core/html/canvas/image_data.cc
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e8370ac7fcbb48b3ad03255d313768bbedadffdc commit e8370ac7fcbb48b3ad03255d313768bbedadffdc Author: Jeremy Roman <jbroman@chromium.org> Date: Thu Oct 25 13:59:58 2018 Use V8ObjectBuilder in IDBKey conversion unit tests. It's more concise, easier to read, and allows removal of V8CallBoolean uses. Bug: 670615 Change-Id: I795bbab22257011441f5db9e9abc468d6dfd3f58 Reviewed-on: https://chromium-review.googlesource.com/c/1298251 Commit-Queue: Jeremy Roman <jbroman@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#602689} [modify] https://crrev.com/e8370ac7fcbb48b3ad03255d313768bbedadffdc/third_party/blink/renderer/bindings/modules/v8/v8_binding_for_modules_test.cc
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed7da3786ff30ca60f0125bfe1d4d370aa0ef395 commit ed7da3786ff30ca60f0125bfe1d4d370aa0ef395 Author: Jeremy Roman <jbroman@chromium.org> Date: Thu Oct 25 14:05:20 2018 Remove use of V8CallBoolean in IndexedPropertyEnumerator. Bug: 670615 Change-Id: I4c36b020c9528ff8670fba6c80517bef8c231fb4 Reviewed-on: https://chromium-review.googlesource.com/c/1298052 Commit-Queue: Jeremy Roman <jbroman@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#602692} [modify] https://crrev.com/ed7da3786ff30ca60f0125bfe1d4d370aa0ef395/third_party/blink/renderer/platform/bindings/v8_binding.h
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38a0005a196fc249225ffe5b6eebd6b590f9f215 commit 38a0005a196fc249225ffe5b6eebd6b590f9f215 Author: Jeremy Roman <jbroman@chromium.org> Date: Thu Oct 25 14:10:37 2018 Remove use of V8CallBoolean in blink::Dictionary. It's deprecated in favor of explicitly checking for an exception. At the same time, swallow exceptions that can emerge from v8::Object::Has (e.g. if the object is a Proxy). Bug: 670615 Change-Id: I2b560d44066e97f79820fcc478a395f2926dbb5b Reviewed-on: https://chromium-review.googlesource.com/c/1297745 Commit-Queue: Jeremy Roman <jbroman@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#602694} [modify] https://crrev.com/38a0005a196fc249225ffe5b6eebd6b590f9f215/third_party/blink/renderer/bindings/core/v8/dictionary.cc
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69547ba16cb007ffd0496e1c9b0cc7023edbed68 commit 69547ba16cb007ffd0496e1c9b0cc7023edbed68 Author: Jeremy Roman <jbroman@chromium.org> Date: Thu Oct 25 16:02:08 2018 Remove use of V8CallBoolean in IDB value conversion code. While passing by, fixing cases where some exceptions were rethrown but ones emerging from the V8CallBoolean use were not. Bug: 670615 Change-Id: I5442269f4c6a8dee7137ba129f18d31a9f36e888 Reviewed-on: https://chromium-review.googlesource.com/c/1297468 Commit-Queue: Jeremy Roman <jbroman@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#602726} [modify] https://crrev.com/69547ba16cb007ffd0496e1c9b0cc7023edbed68/third_party/blink/renderer/bindings/modules/v8/v8_binding_for_modules.cc
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e62ee2ddfb5bd82d707e8605abb0fe9e9d556359 commit e62ee2ddfb5bd82d707e8605abb0fe9e9d556359 Author: Jeremy Roman <jbroman@chromium.org> Date: Fri Oct 26 14:46:34 2018 Remove V8CallBoolean by using script evaluation to construct NativeValueTraitsImplTest values. This removes deprecated uses of V8CallBoolean, and takes advantage of the fact that some of these complicated objects can be more concisely constructed from script. Bug: 670615 Change-Id: Ie4a3cc6404559240a7cb0ea07736e7db0648f4d8 Reviewed-on: https://chromium-review.googlesource.com/c/1299339 Commit-Queue: Jeremy Roman <jbroman@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#603087} [modify] https://crrev.com/e62ee2ddfb5bd82d707e8605abb0fe9e9d556359/third_party/blink/renderer/bindings/core/v8/native_value_traits_impl_test.cc
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f62c7ec13bdfd72dac54ce2fd03ab9812ec9b2fc commit f62c7ec13bdfd72dac54ce2fd03ab9812ec9b2fc Author: Jeremy Roman <jbroman@chromium.org> Date: Fri Oct 26 19:35:13 2018 Remove remaining uses of V8CallBoolean. Bug: 670615 Change-Id: Ifd560c15fe8440d6501935753afb5f6eff6cd176 Reviewed-on: https://chromium-review.googlesource.com/c/1299735 Commit-Queue: Jeremy Roman <jbroman@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#603173} [modify] https://crrev.com/f62c7ec13bdfd72dac54ce2fd03ab9812ec9b2fc/third_party/blink/renderer/bindings/core/v8/custom/v8_html_plugin_element_custom.cc [modify] https://crrev.com/f62c7ec13bdfd72dac54ce2fd03ab9812ec9b2fc/third_party/blink/renderer/bindings/core/v8/v0_custom_element_constructor_builder.cc [modify] https://crrev.com/f62c7ec13bdfd72dac54ce2fd03ab9812ec9b2fc/third_party/blink/renderer/bindings/core/v8/v8_v0_custom_element_lifecycle_callbacks.cc [modify] https://crrev.com/f62c7ec13bdfd72dac54ce2fd03ab9812ec9b2fc/third_party/blink/renderer/platform/bindings/to_v8.h [modify] https://crrev.com/f62c7ec13bdfd72dac54ce2fd03ab9812ec9b2fc/third_party/blink/renderer/platform/bindings/v8_binding_macros.h
,
Oct 28
,
Oct 28
Excellent! \o/ |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Apr 25 2017