[CodeGeneratorJS] Standalone dictionaries have misplaced build guards
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: http://svn.webkit.org/repository/webkit/trunk@207766 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 147c5dc..6230eae 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -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
diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
index 8e819b7..5f314545 100644
--- a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
+++ b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
@@ -1053,10 +1053,9 @@
 
 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 @@
     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 @@
     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 @@
     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 @@
     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
diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
index 289084c..bc61b23 100644
--- a/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
+++ b/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
@@ -857,6 +857,87 @@
     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)
diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestObj.h b/Source/WebCore/bindings/scripts/test/JS/JSTestObj.h
index 0b4ea8d..6418775 100644
--- a/Source/WebCore/bindings/scripts/test/JS/JSTestObj.h
+++ b/Source/WebCore/bindings/scripts/test/JS/JSTestObj.h
@@ -184,5 +184,23 @@
 
 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
diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp
index 3d7f0f4..a044ccf 100644
--- a/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp
+++ b/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp
@@ -19,6 +19,9 @@
 */
 
 #include "config.h"
+
+#if ENABLE(Condition1)
+
 #include "JSTestStandaloneDictionary.h"
 
 
@@ -55,3 +58,5 @@
 }
 
 } // namespace WebCore
+
+#endif // ENABLE(Condition1)
diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.h b/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.h
index 76a2ab1..e3b199b 100644
--- a/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.h
+++ b/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.h
@@ -20,6 +20,8 @@
 
 #pragma once
 
+#if ENABLE(Condition1)
+
 #include "DictionaryImplName.h"
 #include "JSDOMConvert.h"
 
@@ -28,3 +30,5 @@
 template<> DictionaryImplName convertDictionary<DictionaryImplName>(JSC::ExecState&, JSC::JSValue);
 
 } // namespace WebCore
+
+#endif // ENABLE(Condition1)
diff --git a/Source/WebCore/bindings/scripts/test/TestObj.idl b/Source/WebCore/bindings/scripts/test/TestObj.idl
index b63a03a..162dfdd 100644
--- a/Source/WebCore/bindings/scripts/test/TestObj.idl
+++ b/Source/WebCore/bindings/scripts/test/TestObj.idl
@@ -512,3 +512,20 @@
     boolean childMember1;
 };
 
+[
+    Conditional=Condition1
+] dictionary TestConditionalDictionaryA {
+    DOMString stringWithoutDefault;
+};
+
+[
+    Conditional=Condition1&Condition2
+] dictionary TestConditionalDictionaryB {
+    DOMString stringWithoutDefault;
+};
+
+[
+    Conditional=Condition1|Condition2
+] dictionary TestConditionalDictionaryC {
+    DOMString stringWithoutDefault;
+};
diff --git a/Source/WebCore/bindings/scripts/test/TestStandaloneDictionary.idl b/Source/WebCore/bindings/scripts/test/TestStandaloneDictionary.idl
index 95897ea..fef5974 100644
--- a/Source/WebCore/bindings/scripts/test/TestStandaloneDictionary.idl
+++ b/Source/WebCore/bindings/scripts/test/TestStandaloneDictionary.idl
@@ -28,6 +28,7 @@
 
 [
     ImplementedAs=DictionaryImplName,
+    Conditional=Condition1,
 ] dictionary TestStandaloneDictionary {
     boolean boolMember;
     DOMString stringMember;