convertEnumerationToJS() should not stash ASCIILiteral strings in NeverDestroyed...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 May 2017 20:34:47 +0000 (20:34 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 May 2017 20:34:47 +0000 (20:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172724
<rdar://problem/31193201>

Reviewed by Chris Dumez.

Use MAKE_STATIC_STRING_IMPL instead, which is guaranteed to be thread-safe and
satisfies the promise of immortality promised by NeverDestroyed (while ASCIILiteral
does not always satisfy this promise).

Also converted the ASSERT in convertEnumerationToJS() to a RELEASE_ASSERT as a
debugging aid to check if it is ever passed an invalid enumerationValue.

No new tests because this is a speculative fix for an issue observed in the wild
whose root cause is not known yet.  This patch also adds a release assert to
gather more info about the nature of the issue.

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateEnumerationImplementationContent):

* bindings/scripts/test/JS/JSTestCallbackInterface.cpp:
(WebCore::convertEnumerationToJS):
* bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::convertEnumerationToJS):
* bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp:
(WebCore::convertEnumerationToJS):
* bindings/scripts/test/JS/JSTestStandaloneEnumeration.cpp:
(WebCore::convertEnumerationToJS):
- re-baselined these test results.

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

Source/WebCore/ChangeLog
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackInterface.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneEnumeration.cpp

index 9ea9ea5..e510a99 100644 (file)
@@ -1,3 +1,35 @@
+2017-05-30  Mark Lam  <mark.lam@apple.com>
+
+        convertEnumerationToJS() should not stash ASCIILiteral strings in NeverDestroyed arrays.
+        https://bugs.webkit.org/show_bug.cgi?id=172724
+        <rdar://problem/31193201>
+
+        Reviewed by Chris Dumez.
+
+        Use MAKE_STATIC_STRING_IMPL instead, which is guaranteed to be thread-safe and
+        satisfies the promise of immortality promised by NeverDestroyed (while ASCIILiteral
+        does not always satisfy this promise).
+
+        Also converted the ASSERT in convertEnumerationToJS() to a RELEASE_ASSERT as a
+        debugging aid to check if it is ever passed an invalid enumerationValue.
+
+        No new tests because this is a speculative fix for an issue observed in the wild
+        whose root cause is not known yet.  This patch also adds a release assert to
+        gather more info about the nature of the issue.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateEnumerationImplementationContent):
+
+        * bindings/scripts/test/JS/JSTestCallbackInterface.cpp:
+        (WebCore::convertEnumerationToJS):
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        (WebCore::convertEnumerationToJS):
+        * bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp:
+        (WebCore::convertEnumerationToJS):
+        * bindings/scripts/test/JS/JSTestStandaloneEnumeration.cpp:
+        (WebCore::convertEnumerationToJS):
+        - re-baselined these test results.
+
 2017-05-30  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         REGRESSION(r216882): No image decoding is needed if the BitmapImage is created with a NativeImage
index 96c6572..2f18cdf 100644 (file)
@@ -1335,7 +1335,7 @@ sub GenerateEnumerationImplementationContent
         if ($value eq "") {
             $result .= "        emptyString(),\n";
         } else {
-            $result .= "        ASCIILiteral(\"$value\"),\n";
+            $result .= "        MAKE_STATIC_STRING_IMPL(\"$value\"),\n";
         }
     }
     $result .= "    };\n";
@@ -1345,7 +1345,12 @@ sub GenerateEnumerationImplementationContent
         $result .= "    static_assert(static_cast<size_t>(${className}::$enumerationValueName) == $index, \"${className}::$enumerationValueName is not $index as expected\");\n";
         $index++;
     }
-    $result .= "    ASSERT(static_cast<size_t>(enumerationValue) < WTF_ARRAY_LENGTH(values));\n";
+    # FIXME: This is a debugging aid for <rdar://problem/31193201>. Please revert when no longer needed.
+    if ($className eq "History::ScrollRestoration") {
+        $result .= "    RELEASE_ASSERT(static_cast<size_t>(enumerationValue) < WTF_ARRAY_LENGTH(values));\n";
+    } else {
+        $result .= "    ASSERT(static_cast<size_t>(enumerationValue) < WTF_ARRAY_LENGTH(values));\n";
+    }
     $result .= "    return jsStringWithCache(&state, values[static_cast<size_t>(enumerationValue)]);\n";
     $result .= "}\n\n";
 
index ce369d4..12c3d65 100644 (file)
@@ -42,8 +42,8 @@ namespace WebCore {
 template<> JSString* convertEnumerationToJS(ExecState& state, TestCallbackInterface::Enum enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("value1"),
-        ASCIILiteral("value2"),
+        MAKE_STATIC_STRING_IMPL("value1"),
+        MAKE_STATIC_STRING_IMPL("value2"),
     };
     static_assert(static_cast<size_t>(TestCallbackInterface::Enum::Value1) == 0, "TestCallbackInterface::Enum::Value1 is not 0 as expected");
     static_assert(static_cast<size_t>(TestCallbackInterface::Enum::Value2) == 1, "TestCallbackInterface::Enum::Value2 is not 1 as expected");
index e5cab6f..bf30889 100644 (file)
@@ -99,9 +99,9 @@ template<> JSString* convertEnumerationToJS(ExecState& state, TestObj::EnumType
 {
     static NeverDestroyed<const String> values[] = {
         emptyString(),
-        ASCIILiteral("enumValue1"),
-        ASCIILiteral("EnumValue2"),
-        ASCIILiteral("EnumValue3"),
+        MAKE_STATIC_STRING_IMPL("enumValue1"),
+        MAKE_STATIC_STRING_IMPL("EnumValue2"),
+        MAKE_STATIC_STRING_IMPL("EnumValue3"),
     };
     static_assert(static_cast<size_t>(TestObj::EnumType::EmptyString) == 0, "TestObj::EnumType::EmptyString is not 0 as expected");
     static_assert(static_cast<size_t>(TestObj::EnumType::EnumValue1) == 1, "TestObj::EnumType::EnumValue1 is not 1 as expected");
@@ -134,9 +134,9 @@ template<> JSString* convertEnumerationToJS(ExecState& state, TestObj::Optional
 {
     static NeverDestroyed<const String> values[] = {
         emptyString(),
-        ASCIILiteral("OptionalValue1"),
-        ASCIILiteral("OptionalValue2"),
-        ASCIILiteral("OptionalValue3"),
+        MAKE_STATIC_STRING_IMPL("OptionalValue1"),
+        MAKE_STATIC_STRING_IMPL("OptionalValue2"),
+        MAKE_STATIC_STRING_IMPL("OptionalValue3"),
     };
     static_assert(static_cast<size_t>(TestObj::Optional::EmptyString) == 0, "TestObj::Optional::EmptyString is not 0 as expected");
     static_assert(static_cast<size_t>(TestObj::Optional::OptionalValue1) == 1, "TestObj::Optional::OptionalValue1 is not 1 as expected");
@@ -168,8 +168,8 @@ template<> const char* expectedEnumerationValues<TestObj::Optional>()
 template<> JSString* convertEnumerationToJS(ExecState& state, AlternateEnumName enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("enumValue1"),
-        ASCIILiteral("EnumValue2"),
+        MAKE_STATIC_STRING_IMPL("enumValue1"),
+        MAKE_STATIC_STRING_IMPL("EnumValue2"),
     };
     static_assert(static_cast<size_t>(AlternateEnumName::EnumValue1) == 0, "AlternateEnumName::EnumValue1 is not 0 as expected");
     static_assert(static_cast<size_t>(AlternateEnumName::EnumValue2) == 1, "AlternateEnumName::EnumValue2 is not 1 as expected");
@@ -197,7 +197,7 @@ template<> const char* expectedEnumerationValues<AlternateEnumName>()
 template<> JSString* convertEnumerationToJS(ExecState& state, TestObj::EnumA enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("A"),
+        MAKE_STATIC_STRING_IMPL("A"),
     };
     static_assert(static_cast<size_t>(TestObj::EnumA::A) == 0, "TestObj::EnumA::A is not 0 as expected");
     ASSERT(static_cast<size_t>(enumerationValue) < WTF_ARRAY_LENGTH(values));
@@ -224,7 +224,7 @@ template<> const char* expectedEnumerationValues<TestObj::EnumA>()
 template<> JSString* convertEnumerationToJS(ExecState& state, TestObj::EnumB enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("B"),
+        MAKE_STATIC_STRING_IMPL("B"),
     };
     static_assert(static_cast<size_t>(TestObj::EnumB::B) == 0, "TestObj::EnumB::B is not 0 as expected");
     ASSERT(static_cast<size_t>(enumerationValue) < WTF_ARRAY_LENGTH(values));
@@ -251,7 +251,7 @@ template<> const char* expectedEnumerationValues<TestObj::EnumB>()
 template<> JSString* convertEnumerationToJS(ExecState& state, TestObj::EnumC enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("C"),
+        MAKE_STATIC_STRING_IMPL("C"),
     };
     static_assert(static_cast<size_t>(TestObj::EnumC::C) == 0, "TestObj::EnumC::C is not 0 as expected");
     ASSERT(static_cast<size_t>(enumerationValue) < WTF_ARRAY_LENGTH(values));
@@ -276,8 +276,8 @@ template<> const char* expectedEnumerationValues<TestObj::EnumC>()
 template<> JSString* convertEnumerationToJS(ExecState& state, TestObj::Kind enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("quick"),
-        ASCIILiteral("dead"),
+        MAKE_STATIC_STRING_IMPL("quick"),
+        MAKE_STATIC_STRING_IMPL("dead"),
     };
     static_assert(static_cast<size_t>(TestObj::Kind::Quick) == 0, "TestObj::Kind::Quick is not 0 as expected");
     static_assert(static_cast<size_t>(TestObj::Kind::Dead) == 1, "TestObj::Kind::Dead is not 1 as expected");
@@ -303,8 +303,8 @@ template<> const char* expectedEnumerationValues<TestObj::Kind>()
 template<> JSString* convertEnumerationToJS(ExecState& state, TestObj::Size enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("small"),
-        ASCIILiteral("much-much-larger"),
+        MAKE_STATIC_STRING_IMPL("small"),
+        MAKE_STATIC_STRING_IMPL("much-much-larger"),
     };
     static_assert(static_cast<size_t>(TestObj::Size::Small) == 0, "TestObj::Size::Small is not 0 as expected");
     static_assert(static_cast<size_t>(TestObj::Size::MuchMuchLarger) == 1, "TestObj::Size::MuchMuchLarger is not 1 as expected");
@@ -330,8 +330,8 @@ template<> const char* expectedEnumerationValues<TestObj::Size>()
 template<> JSString* convertEnumerationToJS(ExecState& state, TestObj::Confidence enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("high"),
-        ASCIILiteral("kinda-low"),
+        MAKE_STATIC_STRING_IMPL("high"),
+        MAKE_STATIC_STRING_IMPL("kinda-low"),
     };
     static_assert(static_cast<size_t>(TestObj::Confidence::High) == 0, "TestObj::Confidence::High is not 0 as expected");
     static_assert(static_cast<size_t>(TestObj::Confidence::KindaLow) == 1, "TestObj::Confidence::KindaLow is not 1 as expected");
index d7c4f02..dcc22a6 100644 (file)
@@ -67,8 +67,8 @@ template<> DictionaryImplName convertDictionary<DictionaryImplName>(ExecState& s
 template<> JSString* convertEnumerationToJS(ExecState& state, TestStandaloneDictionary::EnumInStandaloneDictionaryFile enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("enumValue1"),
-        ASCIILiteral("enumValue2"),
+        MAKE_STATIC_STRING_IMPL("enumValue1"),
+        MAKE_STATIC_STRING_IMPL("enumValue2"),
     };
     static_assert(static_cast<size_t>(TestStandaloneDictionary::EnumInStandaloneDictionaryFile::EnumValue1) == 0, "TestStandaloneDictionary::EnumInStandaloneDictionaryFile::EnumValue1 is not 0 as expected");
     static_assert(static_cast<size_t>(TestStandaloneDictionary::EnumInStandaloneDictionaryFile::EnumValue2) == 1, "TestStandaloneDictionary::EnumInStandaloneDictionaryFile::EnumValue2 is not 1 as expected");
index 6df7ec5..899ba63 100644 (file)
@@ -34,8 +34,8 @@ namespace WebCore {
 template<> JSString* convertEnumerationToJS(ExecState& state, TestStandaloneEnumeration enumerationValue)
 {
     static NeverDestroyed<const String> values[] = {
-        ASCIILiteral("enumValue1"),
-        ASCIILiteral("enumValue2"),
+        MAKE_STATIC_STRING_IMPL("enumValue1"),
+        MAKE_STATIC_STRING_IMPL("enumValue2"),
     };
     static_assert(static_cast<size_t>(TestStandaloneEnumeration::EnumValue1) == 0, "TestStandaloneEnumeration::EnumValue1 is not 0 as expected");
     static_assert(static_cast<size_t>(TestStandaloneEnumeration::EnumValue2) == 1, "TestStandaloneEnumeration::EnumValue2 is not 1 as expected");