[CodeGeneratorJS] Standalone dictionaries have misplaced build guards
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Oct 2016 18:12:01 +0000 (18:12 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Oct 2016 18:12:01 +0000 (18:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163881

Patch by Zan Dobersek <zdobersek@igalia.com> on 2016-10-24
Reviewed by Chris Dumez.

Build guards that are generated from the Conditional attribute
on WebIDL dictionary declarations properly guard the relevant
convertDictionary() functions for WebIDL files that also specify
interfaces. But for standalone dictionaries these build guards
should guard the complete header and implementation files, much
like this is done for files that originate from interfaces or
callbacks.

Before this patch, guarding a standalone dictionary resulted in
malformed output because GenerateHeaderContentHeader() and
GenerateImplementationContentHeader() functions both generated
the #if macro that would guard the whole file, but
GenerateDictionary{Header,Implementation}() didn't generate the
closing #endif.

CodeGeneratorJS.pm now passes the conditional string, if any,
to GenerateDictionary{Header,Implementation}Content() functions
in case of a non-standalone dictionary. Otherwise, the
conditional string, if any, is used to guard the complete
header and implementation files.

Generator tests are updated to cover various build guard
combinations on dictionaries in TestObj.idl, and the standalone
dictionary WebIDL file now has a Conditional attribute to check
that the build guards cover complete generated header and
implementation files.

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateDictionaryHeaderContent):
(GenerateDictionariesHeaderContent):
(GenerateDictionaryImplementationContent):
(GenerateDictionariesImplementationContent):
(GenerateDictionaryHeader):
(GenerateDictionaryImplementation):
* bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::convertDictionary<TestObj::ConditionalDictionaryA>):
(WebCore::convertDictionary<TestObj::ConditionalDictionaryB>):
(WebCore::convertDictionary<TestObj::ConditionalDictionaryC>):
* bindings/scripts/test/JS/JSTestObj.h:
* bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp:
* bindings/scripts/test/JS/JSTestStandaloneDictionary.h:
* bindings/scripts/test/TestObj.idl:
* bindings/scripts/test/TestStandaloneDictionary.idl:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207766 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestObj.h
Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.h
Source/WebCore/bindings/scripts/test/TestObj.idl
Source/WebCore/bindings/scripts/test/TestStandaloneDictionary.idl

index 147c5dc..6230eae 100644 (file)
@@ -1,3 +1,54 @@
+2016-10-24  Zan Dobersek  <zdobersek@igalia.com>
+
+        [CodeGeneratorJS] Standalone dictionaries have misplaced build guards
+        https://bugs.webkit.org/show_bug.cgi?id=163881
+
+        Reviewed by Chris Dumez.
+
+        Build guards that are generated from the Conditional attribute
+        on WebIDL dictionary declarations properly guard the relevant
+        convertDictionary() functions for WebIDL files that also specify
+        interfaces. But for standalone dictionaries these build guards
+        should guard the complete header and implementation files, much
+        like this is done for files that originate from interfaces or
+        callbacks.
+
+        Before this patch, guarding a standalone dictionary resulted in
+        malformed output because GenerateHeaderContentHeader() and
+        GenerateImplementationContentHeader() functions both generated
+        the #if macro that would guard the whole file, but
+        GenerateDictionary{Header,Implementation}() didn't generate the
+        closing #endif.
+
+        CodeGeneratorJS.pm now passes the conditional string, if any,
+        to GenerateDictionary{Header,Implementation}Content() functions
+        in case of a non-standalone dictionary. Otherwise, the
+        conditional string, if any, is used to guard the complete
+        header and implementation files.
+
+        Generator tests are updated to cover various build guard
+        combinations on dictionaries in TestObj.idl, and the standalone
+        dictionary WebIDL file now has a Conditional attribute to check
+        that the build guards cover complete generated header and
+        implementation files.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateDictionaryHeaderContent):
+        (GenerateDictionariesHeaderContent):
+        (GenerateDictionaryImplementationContent):
+        (GenerateDictionariesImplementationContent):
+        (GenerateDictionaryHeader):
+        (GenerateDictionaryImplementation):
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        (WebCore::convertDictionary<TestObj::ConditionalDictionaryA>):
+        (WebCore::convertDictionary<TestObj::ConditionalDictionaryB>):
+        (WebCore::convertDictionary<TestObj::ConditionalDictionaryC>):
+        * bindings/scripts/test/JS/JSTestObj.h:
+        * bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp:
+        * bindings/scripts/test/JS/JSTestStandaloneDictionary.h:
+        * bindings/scripts/test/TestObj.idl:
+        * bindings/scripts/test/TestStandaloneDictionary.idl:
+
 2016-10-24  Eric Carlson  <eric.carlson@apple.com>
 
         [MediaStream] Separate media capture and audio playback muting
index 8e819b7..5f31454 100644 (file)
@@ -1053,10 +1053,9 @@ sub GenerateConversionRuleWithLeadingComma
 
 sub GenerateDictionaryHeaderContent
 {
-    my ($dictionary, $className) = @_;
+    my ($dictionary, $className, $conditionalString) = @_;
 
     my $result = "";
-    my $conditionalString = $codeGenerator->GenerateConditionalString($dictionary);
     $result .= "#if ${conditionalString}\n\n" if $conditionalString;
     $result .= "template<> $className convertDictionary<$className>(JSC::ExecState&, JSC::JSValue);\n\n";
     $result .= "#endif\n\n" if $conditionalString;
@@ -1075,20 +1074,20 @@ sub GenerateDictionariesHeaderContent
     foreach my $dictionary (@$allDictionaries) {
         $headerIncludes{$interface->name . ".h"} = 1;
         my $className = GetDictionaryClassName($dictionary->name, $interface);
-        $result .= GenerateDictionaryHeaderContent($dictionary, $className);
+        my $conditionalString = $codeGenerator->GenerateConditionalString($dictionary);
+        $result .= GenerateDictionaryHeaderContent($dictionary, $className, $conditionalString);
     }
     return $result;
 }
 
 sub GenerateDictionaryImplementationContent
 {
-    my ($dictionary, $className, $interface) = @_;
+    my ($dictionary, $className, $interface, $conditionalString) = @_;
 
     my $result = "";
 
     my $name = $dictionary->name;
 
-    my $conditionalString = $codeGenerator->GenerateConditionalString($dictionary);
     $result .= "#if ${conditionalString}\n\n" if $conditionalString;
 
     # FIXME: A little ugly to have this be a side effect instead of a return value.
@@ -1186,7 +1185,8 @@ sub GenerateDictionariesImplementationContent
     my $result = "";
     foreach my $dictionary (@$allDictionaries) {
         my $className = GetDictionaryClassName($dictionary->name, $interface);
-        $result .= GenerateDictionaryImplementationContent($dictionary, $className, $interface);
+        my $conditionalString = $codeGenerator->GenerateConditionalString($dictionary);
+        $result .= GenerateDictionaryImplementationContent($dictionary, $className, $interface, $conditionalString);
     }
     return $result;
 }
@@ -4467,6 +4467,9 @@ sub GenerateDictionaryHeader
     push(@headerContent, "\nnamespace WebCore {\n\n");
     push(@headerContent, GenerateDictionaryHeaderContent($dictionary, $className));
     push(@headerContent, "} // namespace WebCore\n");
+
+    my $conditionalString = $codeGenerator->GenerateConditionalString($dictionary);
+    push(@headerContent, "\n#endif // ${conditionalString}\n") if $conditionalString;
     
     # - Generate dependencies.
     if ($writeDependencies) {
@@ -4494,6 +4497,9 @@ sub GenerateDictionaryImplementation
     push(@implContent, "namespace WebCore {\n\n");
     push(@implContent, GenerateDictionaryImplementationContent($dictionary, $className));
     push(@implContent, "} // namespace WebCore\n");
+
+    my $conditionalString = $codeGenerator->GenerateConditionalString($dictionary);
+    push(@implContent, "\n#endif // ${conditionalString}\n") if $conditionalString;
 }
 
 sub GenerateCallbackHeader
index 289084c..bc61b23 100644 (file)
@@ -857,6 +857,87 @@ template<> TestObj::ChildDictionary convertDictionary<TestObj::ChildDictionary>(
     return result;
 }
 
+#if ENABLE(Condition1)
+
+template<> TestObj::ConditionalDictionaryA convertDictionary<TestObj::ConditionalDictionaryA>(ExecState& state, JSValue value)
+{
+    VM& vm = state.vm();
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    bool isNullOrUndefined = value.isUndefinedOrNull();
+    auto* object = isNullOrUndefined ? nullptr : value.getObject();
+    if (UNLIKELY(!isNullOrUndefined && !object)) {
+        throwTypeError(&state, throwScope);
+        return { };
+    }
+    if (UNLIKELY(object && object->type() == RegExpObjectType)) {
+        throwTypeError(&state, throwScope);
+        return { };
+    }
+    TestObj::ConditionalDictionaryA result;
+    JSValue stringWithoutDefaultValue = isNullOrUndefined ? jsUndefined() : object->get(&state, Identifier::fromString(&state, "stringWithoutDefault"));
+    if (!stringWithoutDefaultValue.isUndefined()) {
+        result.stringWithoutDefault = convert<IDLDOMString>(state, stringWithoutDefaultValue);
+        RETURN_IF_EXCEPTION(throwScope, { });
+    }
+    return result;
+}
+
+#endif
+
+#if ENABLE(Condition1) && ENABLE(Condition2)
+
+template<> TestObj::ConditionalDictionaryB convertDictionary<TestObj::ConditionalDictionaryB>(ExecState& state, JSValue value)
+{
+    VM& vm = state.vm();
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    bool isNullOrUndefined = value.isUndefinedOrNull();
+    auto* object = isNullOrUndefined ? nullptr : value.getObject();
+    if (UNLIKELY(!isNullOrUndefined && !object)) {
+        throwTypeError(&state, throwScope);
+        return { };
+    }
+    if (UNLIKELY(object && object->type() == RegExpObjectType)) {
+        throwTypeError(&state, throwScope);
+        return { };
+    }
+    TestObj::ConditionalDictionaryB result;
+    JSValue stringWithoutDefaultValue = isNullOrUndefined ? jsUndefined() : object->get(&state, Identifier::fromString(&state, "stringWithoutDefault"));
+    if (!stringWithoutDefaultValue.isUndefined()) {
+        result.stringWithoutDefault = convert<IDLDOMString>(state, stringWithoutDefaultValue);
+        RETURN_IF_EXCEPTION(throwScope, { });
+    }
+    return result;
+}
+
+#endif
+
+#if ENABLE(Condition1) || ENABLE(Condition2)
+
+template<> TestObj::ConditionalDictionaryC convertDictionary<TestObj::ConditionalDictionaryC>(ExecState& state, JSValue value)
+{
+    VM& vm = state.vm();
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    bool isNullOrUndefined = value.isUndefinedOrNull();
+    auto* object = isNullOrUndefined ? nullptr : value.getObject();
+    if (UNLIKELY(!isNullOrUndefined && !object)) {
+        throwTypeError(&state, throwScope);
+        return { };
+    }
+    if (UNLIKELY(object && object->type() == RegExpObjectType)) {
+        throwTypeError(&state, throwScope);
+        return { };
+    }
+    TestObj::ConditionalDictionaryC result;
+    JSValue stringWithoutDefaultValue = isNullOrUndefined ? jsUndefined() : object->get(&state, Identifier::fromString(&state, "stringWithoutDefault"));
+    if (!stringWithoutDefaultValue.isUndefined()) {
+        result.stringWithoutDefault = convert<IDLDOMString>(state, stringWithoutDefaultValue);
+        RETURN_IF_EXCEPTION(throwScope, { });
+    }
+    return result;
+}
+
+#endif
+
 // Functions
 
 #if ENABLE(TEST_FEATURE)
index 0b4ea8d..6418775 100644 (file)
@@ -184,5 +184,23 @@ template<> TestObj::ParentDictionary convertDictionary<TestObj::ParentDictionary
 
 template<> TestObj::ChildDictionary convertDictionary<TestObj::ChildDictionary>(JSC::ExecState&, JSC::JSValue);
 
+#if ENABLE(Condition1)
+
+template<> TestObj::ConditionalDictionaryA convertDictionary<TestObj::ConditionalDictionaryA>(JSC::ExecState&, JSC::JSValue);
+
+#endif
+
+#if ENABLE(Condition1) && ENABLE(Condition2)
+
+template<> TestObj::ConditionalDictionaryB convertDictionary<TestObj::ConditionalDictionaryB>(JSC::ExecState&, JSC::JSValue);
+
+#endif
+
+#if ENABLE(Condition1) || ENABLE(Condition2)
+
+template<> TestObj::ConditionalDictionaryC convertDictionary<TestObj::ConditionalDictionaryC>(JSC::ExecState&, JSC::JSValue);
+
+#endif
+
 
 } // namespace WebCore
index 3d7f0f4..a044ccf 100644 (file)
@@ -19,6 +19,9 @@
 */
 
 #include "config.h"
+
+#if ENABLE(Condition1)
+
 #include "JSTestStandaloneDictionary.h"
 
 
@@ -55,3 +58,5 @@ template<> DictionaryImplName convertDictionary<DictionaryImplName>(ExecState& s
 }
 
 } // namespace WebCore
+
+#endif // ENABLE(Condition1)
index 76a2ab1..e3b199b 100644 (file)
@@ -20,6 +20,8 @@
 
 #pragma once
 
+#if ENABLE(Condition1)
+
 #include "DictionaryImplName.h"
 #include "JSDOMConvert.h"
 
@@ -28,3 +30,5 @@ namespace WebCore {
 template<> DictionaryImplName convertDictionary<DictionaryImplName>(JSC::ExecState&, JSC::JSValue);
 
 } // namespace WebCore
+
+#endif // ENABLE(Condition1)
index b63a03a..162dfdd 100644 (file)
@@ -512,3 +512,20 @@ dictionary ChildDictionary : ParentDictionary {
     boolean childMember1;
 };
 
+[
+    Conditional=Condition1
+] dictionary TestConditionalDictionaryA {
+    DOMString stringWithoutDefault;
+};
+
+[
+    Conditional=Condition1&Condition2
+] dictionary TestConditionalDictionaryB {
+    DOMString stringWithoutDefault;
+};
+
+[
+    Conditional=Condition1|Condition2
+] dictionary TestConditionalDictionaryC {
+    DOMString stringWithoutDefault;
+};
index 95897ea..fef5974 100644 (file)
@@ -28,6 +28,7 @@
 
 [
     ImplementedAs=DictionaryImplName,
+    Conditional=Condition1,
 ] dictionary TestStandaloneDictionary {
     boolean boolMember;
     DOMString stringMember;