New issue
Advanced search Search tips

Issue 871208 link

Starred by 0 users
Status: Fixed
Owner:
yukishiino@chromium.org
Closed: Aug 14
Cc:
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

Support IDL callback function Function defined in Web IDL

Project Member Reported by yukishiino@chromium.org, Aug 6 
Type 'Function' in Web IDL:
https://heycam.github.io/webidl/#Function

The current implementation treats Function type as a sort of built-in types, and the generated bindings code just passes it as ScriptValue to Blink implementation.  This is not good because it doesn't handle the incumbent realm correctly.

Let's support Function type based on blink::CalbackFunctionBase in the same way of other callback functions.

This task requires support of variadic arguments on callback functions.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 8 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239

commit 2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Wed Aug 08 08:50:01 2018

v8binding: Supports IDL callback function with variadic arguments.

It turned out that we need type Function defined in Web IDL in
order to correctly implement callback-constructor-related
(custom elements, etc.) and also timer APIs (setTimeout,
setInterval).

This patch supports variadic arguments on callback functions
in order to support Function type.
https://heycam.github.io/webidl/#Function

Change-Id: I435053cd770a36e3e993c5bc789f77dc51829cb8
Bug:  chromium:871208 
Reviewed-on: https://chromium-review.googlesource.com/1163559
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581504}
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/scripts/v8_callback_function.py
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/templates/callback_function.cpp.tmpl
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/idls/core/test_callback_functions.idl
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_any_callback_function_optional_any_arg.cc
[add] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_any_callback_function_variadic_any_args.cc
[add] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_any_callback_function_variadic_any_args.h
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_long_callback_function.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_string_sequence_callback_function_long_sequence_arg.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_void_callback_function.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_void_callback_function_dictionary_arg.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_void_callback_function_enum_arg.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_void_callback_function_interface_arg.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_void_callback_function_test_interface_sequence_arg.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/core/v8_void_callback_function_typedef.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/bindings/tests/results/modules/v8_void_callback_function_modules.cc
[modify] https://crrev.com/2ddc03b8433f9d56a5ed6b49a5a8dcf42b70b239/third_party/blink/renderer/core/dom/common_definitions.idl
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd5bbc4a36ed1eb3458c5da983d0068187c873cb

commit dd5bbc4a36ed1eb3458c5da983d0068187c873cb
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Fri Aug 10 00:41:28 2018

v8binding: Rename Function type in *.idl to CallbackFunctionTreatedAsScriptValue.

Renames existing old Function to CallbackFunctionTreatedAsScriptValue
in *.idl so that we can introduce new Function type that is type of
|any ()|.

Function type is now defined in core/dom/common_definitions.idl,
and v8_function.{h,cc} are now code-generated (not yet used
though).

Bug:  chromium:871208 
Change-Id: I395c49e5d664cec14ad3b29653b10c4bb000f4fe
Reviewed-on: https://chromium-review.googlesource.com/1166931
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@{#581985}
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/bindings/core/v8/BUILD.gn
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/bindings/scripts/idl_types.py
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/bindings/scripts/v8_interface.py
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/bindings/tests/idls/core/test_object.idl
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/core/dom/common_definitions.idl
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/core/frame/window_timers.idl
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/core/html/custom/custom_element_registry.idl
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.idl
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope.idl
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.idl
[modify] https://crrev.com/dd5bbc4a36ed1eb3458c5da983d0068187c873cb/third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope.idl
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f2c0c03c15b12e36df3b9939bcfca3467d2e6c9

commit 3f2c0c03c15b12e36df3b9939bcfca3467d2e6c9
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Tue Aug 14 05:53:13 2018

v8binding: Supports InvokeAndReportException for type Function.

Web IDL type Function (= any (any...)) is a sort of "arbitrary
function", and used very widely in the web standards.  Its
return value is often discarded, as if its return type is void.

So, this patch supports InvokeAndReportException for type
Function.

Bug:  871208 
Change-Id: Icbe56560585c402271363e7c8294d3b0c96b1562
Reviewed-on: https://chromium-review.googlesource.com/1173931
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582843}
[modify] https://crrev.com/3f2c0c03c15b12e36df3b9939bcfca3467d2e6c9/third_party/blink/renderer/bindings/templates/callback_function.cpp.tmpl
[modify] https://crrev.com/3f2c0c03c15b12e36df3b9939bcfca3467d2e6c9/third_party/blink/renderer/bindings/templates/callback_function.h.tmpl

Comment 4 by yukishiino@chromium.org, Aug 14

Status: Fixed (was: Started)
►

Sign in to add a comment