Skip to content

Commit

Permalink
Revert of IDL: Support optional argument default value syntax (https:…
Browse files Browse the repository at this point in the history
…//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

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

git-svn-id: svn://svn.chromium.org/blink/trunk@176220 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
[email protected] committed Jun 16, 2014
1 parent 7e33121 commit 4a1fd1c
Show file tree
Hide file tree
Showing 34 changed files with 74 additions and 293 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=Undefined
Default=Null|NullString|Undefined
DependentLifetime
DeprecateAs=*
DoNotCheckConstants
Expand Down
56 changes: 0 additions & 56 deletions Source/bindings/scripts/idl_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,59 +393,6 @@ 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 @@ -528,7 +475,6 @@ 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 @@ -542,8 +488,6 @@ 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: 3 additions & 7 deletions Source/bindings/scripts/v8_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,8 @@ 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,
# FIXME: remove once [Default] removed and just use argument.default_value
'has_default': 'Default' in extended_attributes or argument.default_value,
'has_default': 'Default' in extended_attributes,
'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 @@ -301,10 +298,9 @@ 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)
# FIXME: This special way of handling string arguments with null defaults
# can go away once we fully support default values.
# [Default=NullString]
if (argument.is_optional and idl_type.name in ('String', 'ByteString') and
argument.default_value and argument.default_value.is_null):
extended_attributes.get('Default') == 'NullString'):
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,
optional DOMString defaultNullStringOptionalStringArg = null,
[Default=NullString] optional DOMString defaultNullStringOptionalStringArg,
[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,
optional DOMString defaultNullStringOptionalstringArg = null,
[Default=NullString] optional DOMString defaultNullStringOptionalstringArg,
optional DOMString optionalStringArg),
RaisesException=Constructor,
] interface TestInterfaceNamedConstructor {
Expand Down
13 changes: 2 additions & 11 deletions Source/bindings/tests/idls/TestObject.idl
Original file line number Diff line number Diff line change
Expand Up @@ -379,16 +379,6 @@ 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 @@ -411,7 +401,7 @@ interface TestObject {
void overloadedMethodF(optional DOMString stringArg);
void overloadedMethodF(double doubleArg);
void overloadedMethodG(long longArg);
void overloadedMethodG(optional TestInterfaceEmpty? testInterfaceEmptyOrNullArg = null);
void overloadedMethodG([Default=Null] TestInterfaceEmpty? testInterfaceEmptyOrNullArg);
void overloadedMethodH(TestInterface testInterfaceArg);
void overloadedMethodH(TestInterfaceEmpty testInterfaceEmptyArg);
void overloadedMethodI(DOMString stringArg);
Expand All @@ -432,6 +422,7 @@ 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 4a1fd1c

Please sign in to comment.