[webkit-changes] [217568] trunk/Source/WebCore

Tue, 30 May 2017 13:35:23 -0700

Title: [217568] trunk/Source/WebCore
Revision
217568
Author
[email protected]
Date
2017-05-30 13:34:47 -0700 (Tue, 30 May 2017)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (217567 => 217568)


--- trunk/Source/WebCore/ChangeLog	2017-05-30 20:02:09 UTC (rev 217567)
+++ trunk/Source/WebCore/ChangeLog	2017-05-30 20:34:47 UTC (rev 217568)
@@ -1,3 +1,35 @@
+2017-05-30  Mark Lam  <[email protected]>
+
+        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  <[email protected]>
 
         REGRESSION(r216882): No image decoding is needed if the BitmapImage is created with a NativeImage

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (217567 => 217568)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2017-05-30 20:02:09 UTC (rev 217567)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2017-05-30 20:34:47 UTC (rev 217568)
@@ -1335,7 +1335,7 @@
         if ($value eq "") {
             $result .= "        emptyString(),\n";
         } else {
-            $result .= "        ASCIILiteral(\"$value\"),\n";
+            $result .= "        MAKE_STATIC_STRING_IMPL(\"$value\"),\n";
         }
     }
     $result .= "    };\n";
@@ -1345,7 +1345,12 @@
         $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";

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackInterface.cpp (217567 => 217568)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackInterface.cpp	2017-05-30 20:02:09 UTC (rev 217567)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackInterface.cpp	2017-05-30 20:34:47 UTC (rev 217568)
@@ -42,8 +42,8 @@
 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");

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp (217567 => 217568)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2017-05-30 20:02:09 UTC (rev 217567)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2017-05-30 20:34:47 UTC (rev 217568)
@@ -99,9 +99,9 @@
 {
     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 @@
 {
     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<> 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<> 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<> 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<> 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<> 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<> 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<> 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");

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp (217567 => 217568)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp	2017-05-30 20:02:09 UTC (rev 217567)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneDictionary.cpp	2017-05-30 20:34:47 UTC (rev 217568)
@@ -67,8 +67,8 @@
 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");

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneEnumeration.cpp (217567 => 217568)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneEnumeration.cpp	2017-05-30 20:02:09 UTC (rev 217567)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestStandaloneEnumeration.cpp	2017-05-30 20:34:47 UTC (rev 217568)
@@ -34,8 +34,8 @@
 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");

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to