Skip to content

Commit

Permalink
Revert of Revert of IDL: Support optional argument default value synt…
Browse files Browse the repository at this point in the history
…ax (https://codereview.chromium.org/339683002/)

Reason for revert:
Revert the revert because the bot went green even without this revert, and other bots had no trouble.

Original issue's description:
> Revert of IDL: Support optional argument default value syntax (https://codereview.chromium.org/312683005/)
> 
> Reason for revert:
> This patch is the most likely candidate for browser test failures: http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%29/builds/2905
> 
> BrowserTestBase signal handler received SIGTERM. Backtrace:
> #0 0x7f2c2a201a1d base::debug::StackTrace::StackTrace()
> #1 0x00000460caf2 content::(anonymous namespace)::DumpStackTraceSignalHandler()
> #2 0x7f2c232cd4a0 \u003Cunknown>
> #3 0x7f2c2337f313 __poll
> #4 0x7f2c240b6036 \u003Cunknown>
> #5 0x7f2c240b6164 g_main_context_iteration
> #6 0x7f2c2a1c9b45 base::MessagePumpGlib::Run()
> #7 0x7f2c2a29eed7 base::MessageLoop::RunHandler()
> #8 0x7f2c2a2f69d8 base::RunLoop::Run()
> #9 0x00000466bd59 content::RunThisRunLoop()
> #10 0x00000466c1b8 content::MessageLoopRunner::Run()
> #11 0x000004610946 content::TitleWatcher::WaitAndGetTitle()
> #12 0x0000011fdf00 MediaBrowserTest::RunTest()
> #13 0x0000011fdc00 MediaBrowserTest::RunMediaTestPage()
> #14 0x0000011ec98b EncryptedMediaTestBase::RunEncryptedMediaTestPage()
> #15 0x0000011ee5fa EncryptedMediaTest::TestConfigChange()
> #16 0x0000011eb73a EncryptedMediaTest_ConfigChangeVideo_Test::RunTestOnMainThread()
> ...
> 
> Please look into it. An alterate possible candidate is  https://codereview.chromium.org/327553002 but I think that's less likely.
> 
> Original issue's description:
> > IDL: Support optional argument default value syntax
> > 
> > Adds support for parsing default values of different types, but
> > only handles null default values when generating code.
> > 
> > Replaces existing
> > 
> >   [Default=Null] optional SomeInterface arg
> >   [Default=NullString] optional DOMString arg
> > 
> > with the now equivalent
> > 
> >   optional SomeInterface arg = null
> >   optional DOMString arg = null
> > 
> > in IDL files, and drops support for those [Default] attributes.
> > 
> > No changes to generated code.
> > 
> > BUG=258153
> > 
> > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176200
> 
> [email protected],[email protected],[email protected],[email protected],[email protected]
> NOTREECHECKS=true
> NOTRY=true
> BUG=258153
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176220

[email protected],[email protected],[email protected],[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true
BUG=258153

Review URL: https://codereview.chromium.org/339723002

git-svn-id: svn://svn.chromium.org/blink/trunk@176231 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
[email protected] committed Jun 16, 2014
1 parent 8aeee2b commit 356a3bd
Show file tree
Hide file tree
Showing 34 changed files with 293 additions and 74 deletions.
2 changes: 1 addition & 1 deletion Source/bindings/IDLExtendedAttributes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ ConstructorCallWith=ExecutionContext|Document
Custom=|Getter|Setter|LegacyCallAsFunction|ToV8|VisitDOMWrapper|Wrap|PropertyGetter|PropertyEnumerator|PropertyQuery
CustomConstructor
CustomElementCallbacks
Default=Null|NullString|Undefined
Default=Undefined
DependentLifetime
DeprecateAs=*
DoNotCheckConstants
Expand Down
56 changes: 56 additions & 0 deletions Source/bindings/scripts/idl_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,59 @@ def __init__(self, node):
self.extended_attributes = {}


################################################################################
# Literals
################################################################################

class IdlLiteral(object):
def __init__(self, idl_type, value):
self.idl_type = idl_type
self.value = value
self.is_null = False

def __str__(self):
if self.idl_type == 'DOMString':
return 'String("%s")' % self.value
if self.idl_type == 'integer':
return '%d' % self.value
if self.idl_type == 'float':
return '%g' % self.value
if self.idl_type == 'boolean':
return 'true' if self.value else 'false'
raise ValueError('Unsupported literal type: %s' % self.idl_type)


class IdlLiteralNull(IdlLiteral):
def __init__(self):
self.idl_type = 'NULL'
self.value = None
self.is_null = True

def __str__(self):
return 'nullptr'


def default_node_to_idl_literal(node):
# FIXME: This code is unnecessarily complicated due to the rather
# inconsistent way the upstream IDL parser outputs default values.
# http://crbug.com/374178
idl_type = node.GetProperty('TYPE')
if idl_type == 'DOMString':
value = node.GetProperty('NAME')
if '"' in value or '\\' in value:
raise ValueError('Unsupported string value: %r' % value)
return IdlLiteral(idl_type, value)
if idl_type == 'integer':
return IdlLiteral(idl_type, int(node.GetProperty('NAME')))
if idl_type == 'float':
return IdlLiteral(idl_type, float(node.GetProperty('VALUE')))
if idl_type == 'boolean':
return IdlLiteral(idl_type, node.GetProperty('VALUE'))
if idl_type == 'NULL':
return IdlLiteralNull()
raise ValueError('Unrecognized default value type: %s' % idl_type)


################################################################################
# Operations
################################################################################
Expand Down Expand Up @@ -475,6 +528,7 @@ def __init__(self, node):
self.is_optional = node.GetProperty('OPTIONAL') # syntax: (optional T)
self.is_variadic = False # syntax: (T...)
self.name = node.GetName()
self.default_value = None

children = node.GetChildren()
for child in children:
Expand All @@ -488,6 +542,8 @@ def __init__(self, node):
if child_name != '...':
raise ValueError('Unrecognized Argument node; expected "...", got "%s"' % child_name)
self.is_variadic = child.GetProperty('ELLIPSIS') or False
elif child_class == 'Default':
self.default_value = default_node_to_idl_literal(child)
else:
raise ValueError('Unrecognized node class: %s' % child_class)

Expand Down
10 changes: 7 additions & 3 deletions Source/bindings/scripts/v8_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,11 @@ def generate_argument(interface, method, argument, index):
used_as_argument=True,
used_as_variadic_argument=argument.is_variadic),
'cpp_value': this_cpp_value,
# FIXME: check that the default value's type is compatible with the argument's
'default_value': str(argument.default_value) if argument.default_value else None,
'enum_validation_expression': idl_type.enum_validation_expression,
'has_default': 'Default' in extended_attributes,
# FIXME: remove once [Default] removed and just use argument.default_value
'has_default': 'Default' in extended_attributes or argument.default_value,
'has_event_listener_argument': any(
argument_so_far for argument_so_far in method.arguments[:index]
if argument_so_far.idl_type.name == 'EventListener'),
Expand Down Expand Up @@ -298,9 +301,10 @@ def v8_value_to_local_cpp_value(argument, index):
name = argument.name
if argument.is_variadic:
return v8_value_to_local_cpp_variadic_value(argument, index)
# [Default=NullString]
# FIXME: This special way of handling string arguments with null defaults
# can go away once we fully support default values.
if (argument.is_optional and idl_type.name in ('String', 'ByteString') and
extended_attributes.get('Default') == 'NullString'):
argument.default_value and argument.default_value.is_null):
v8_value = 'argumentOrNull(info, %s)' % index
else:
v8_value = 'info[%s]' % index
Expand Down
2 changes: 1 addition & 1 deletion Source/bindings/tests/idls/TestInterfaceConstructor2.idl
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
TestInterfaceEmpty testInterfaceEmptyArg,
long longArg,
[Default=Undefined] optional DOMString defaultUndefinedOptionalStringArg,
[Default=NullString] optional DOMString defaultNullStringOptionalStringArg,
optional DOMString defaultNullStringOptionalStringArg = null,
[Default=Undefined] optional Dictionary defaultUndefinedOptionalDictionaryArg,
optional DOMString optionalStringArg),
] interface TestInterfaceConstructor2 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
[Default=Undefined] optional boolean defaultUndefinedOptionalBooleanArg,
[Default=Undefined] optional long defaultUndefinedOptionalLongArg,
[Default=Undefined] optional DOMString defaultUndefinedOptionalStringArg,
[Default=NullString] optional DOMString defaultNullStringOptionalstringArg,
optional DOMString defaultNullStringOptionalstringArg = null,
optional DOMString optionalStringArg),
RaisesException=Constructor,
] interface TestInterfaceNamedConstructor {
Expand Down
13 changes: 11 additions & 2 deletions Source/bindings/tests/idls/TestObject.idl
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,16 @@ interface TestObject {
// Optional arguments: exceptional case
void voidMethodOptionalDictionaryArg(optional Dictionary optionalDictionaryArg);

// Optional arguments with defaults
void voidMethodDefaultStringArg(optional DOMString defaultStringArg = "foo");
void voidMethodDefaultNullStringArg(optional DOMString defaultStringArg = null);
void voidMethodDefaultLongArg(optional long defaultLongArg = 10);
void voidMethodDefaultDoubleArg(optional double defaultDoubleArg = 0.5);
void voidMethodDefaultTrueBooleanArg(optional boolean defaultBooleanArg = true);
void voidMethodDefaultFalseBooleanArg(optional boolean defaultBooleanArg = false);
void voidMethodDefaultNullableStringArg(optional DOMString? defaultStringArg = null);
void voidMethodDefaultNullableTestInterfaceArg(optional TestInterface? defaultTestInterfaceArg = null);

// Variadic operations
void voidMethodVariadicStringArg(DOMString... variadicStringArgs);
void voidMethodStringArgVariadicStringArg(DOMString stringArg, DOMString... variadicStringArgs);
Expand All @@ -401,7 +411,7 @@ interface TestObject {
void overloadedMethodF(optional DOMString stringArg);
void overloadedMethodF(double doubleArg);
void overloadedMethodG(long longArg);
void overloadedMethodG([Default=Null] TestInterfaceEmpty? testInterfaceEmptyOrNullArg);
void overloadedMethodG(optional TestInterfaceEmpty? testInterfaceEmptyOrNullArg = null);
void overloadedMethodH(TestInterface testInterfaceArg);
void overloadedMethodH(TestInterfaceEmpty testInterfaceEmptyArg);
void overloadedMethodI(DOMString stringArg);
Expand All @@ -422,7 +432,6 @@ interface TestObject {
void voidMethodDefaultUndefinedTestInterfaceEmptyArg([Default=Undefined] optional TestInterfaceEmpty defaultUndefinedTestInterfaceEmptyArg);
void voidMethodDefaultUndefinedLongArg([Default=Undefined] optional long defaultUndefinedLongArg);
void voidMethodDefaultUndefinedStringArg([Default=Undefined] optional DOMString defaultUndefinedStringArg);
void voidMethodDefaultNullStringStringArg([Default=NullString] optional DOMString defaultNullStringStringArg);
// [EnforceRange]
void voidMethodEnforceRangeLongArg([EnforceRange] long enforceRangeLongArg);
// [TreatNullAs], [TreatUndefinedAs]
Expand Down
Loading

0 comments on commit 356a3bd

Please sign in to comment.