[JSC] Add PrivateSymbolMode::{Include,Exclude} for PropertyNameArray
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Sep 2017 11:17:45 +0000 (11:17 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Sep 2017 11:17:45 +0000 (11:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176867

Reviewed by Sam Weinig.

JSTests:

* microbenchmarks/object-get-own-property-symbols.js: Added.
(test):

Source/JavaScriptCore:

We rarely require private symbols when enumerating property names.
This patch adds PrivateSymbolMode::{Include,Exclude}. If PrivateSymbolMode::Exclude
is specified, PropertyNameArray does not include private symbols.
This removes many ad-hoc `Identifier::isPrivateName()` in enumeration operations.

One additional good thing is that we do not need to filter private symbols out from PropertyNameArray.
It allows us to use Object.keys()'s fast path for Object.getOwnPropertySymbols.

object-get-own-property-symbols                48.6275+-1.0021     ^     38.1846+-1.7934        ^ definitely 1.2735x faster

* API/JSObjectRef.cpp:
(JSObjectCopyPropertyNames):
* bindings/ScriptValue.cpp:
(Inspector::jsToInspectorValue):
* bytecode/ObjectAllocationProfile.h:
(JSC::ObjectAllocationProfile::possibleDefaultPropertyCount):
* runtime/EnumerationMode.h:
* runtime/IntlObject.cpp:
(JSC::supportedLocales):
* runtime/JSONObject.cpp:
(JSC::Stringifier::Stringifier):
(JSC::Stringifier::Holder::appendNextProperty):
(JSC::Walker::walk):
* runtime/JSPropertyNameEnumerator.cpp:
(JSC::JSPropertyNameEnumerator::create):
* runtime/JSPropertyNameEnumerator.h:
(JSC::propertyNameEnumerator):
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorGetOwnPropertyDescriptors):
(JSC::objectConstructorAssign):
(JSC::objectConstructorValues):
(JSC::defineProperties):
(JSC::setIntegrityLevel):
(JSC::testIntegrityLevel):
(JSC::ownPropertyKeys):
* runtime/PropertyNameArray.h:
(JSC::PropertyNameArray::PropertyNameArray):
(JSC::PropertyNameArray::propertyNameMode const):
(JSC::PropertyNameArray::privateSymbolMode const):
(JSC::PropertyNameArray::addUncheckedInternal):
(JSC::PropertyNameArray::addUnchecked):
(JSC::PropertyNameArray::add):
(JSC::PropertyNameArray::isUidMatchedToTypeMode):
(JSC::PropertyNameArray::includeSymbolProperties const):
(JSC::PropertyNameArray::includeStringProperties const):
(JSC::PropertyNameArray::mode const): Deleted.
* runtime/ProxyObject.cpp:
(JSC::ProxyObject::performGetOwnPropertyNames):

Source/WebCore:

* bindings/js/JSDOMConvertRecord.h:
* bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneSerializer::serialize):
* bridge/NP_jsobject.cpp:
(_NPN_Enumerate):

Source/WebKit:

* WebProcess/Plugins/Netscape/NPJSObject.cpp:
(WebKit::NPJSObject::enumerate):

Source/WebKitLegacy/mac:

* Plugins/Hosted/NetscapePluginInstanceProxy.mm:
(WebKit::NetscapePluginInstanceProxy::enumerate):

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

22 files changed:
JSTests/ChangeLog
JSTests/microbenchmarks/object-get-own-property-symbols.js [new file with mode: 0644]
Source/JavaScriptCore/API/JSObjectRef.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bindings/ScriptValue.cpp
Source/JavaScriptCore/bytecode/ObjectAllocationProfile.h
Source/JavaScriptCore/runtime/EnumerationMode.h
Source/JavaScriptCore/runtime/IntlObject.cpp
Source/JavaScriptCore/runtime/JSONObject.cpp
Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.cpp
Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.h
Source/JavaScriptCore/runtime/ObjectConstructor.cpp
Source/JavaScriptCore/runtime/PropertyNameArray.h
Source/JavaScriptCore/runtime/ProxyObject.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMConvertRecord.h
Source/WebCore/bindings/js/SerializedScriptValue.cpp
Source/WebCore/bridge/NP_jsobject.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/Plugins/Netscape/NPJSObject.cpp
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm

index 0928ac3..6c30641 100644 (file)
@@ -1,3 +1,13 @@
+2017-09-14  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Add PrivateSymbolMode::{Include,Exclude} for PropertyNameArray
+        https://bugs.webkit.org/show_bug.cgi?id=176867
+
+        Reviewed by Sam Weinig.
+
+        * microbenchmarks/object-get-own-property-symbols.js: Added.
+        (test):
+
 2017-09-13  Mark Lam  <mark.lam@apple.com>
 
         Rolling out r221832: Regresses Speedometer by ~4% and Dromaeo CSS YUI by ~20%.
diff --git a/JSTests/microbenchmarks/object-get-own-property-symbols.js b/JSTests/microbenchmarks/object-get-own-property-symbols.js
new file mode 100644 (file)
index 0000000..8c56acb
--- /dev/null
@@ -0,0 +1,13 @@
+var object = {};
+for (var i = 0; i < 1e3; ++i) {
+    object[Symbol(i + 'prop')] = i;
+}
+
+function test(object)
+{
+    return Object.getOwnPropertySymbols(object);
+}
+noInline(test);
+
+for (var i = 0; i < 1e3; ++i)
+    test(object);
index 702c823..249b554 100644 (file)
@@ -647,7 +647,7 @@ JSPropertyNameArrayRef JSObjectCopyPropertyNames(JSContextRef ctx, JSObjectRef o
 
     JSObject* jsObject = toJS(object);
     JSPropertyNameArrayRef propertyNames = new OpaqueJSPropertyNameArray(vm);
-    PropertyNameArray array(vm, PropertyNameMode::Strings);
+    PropertyNameArray array(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
     jsObject->methodTable(*vm)->getPropertyNames(jsObject, exec, array, EnumerationMode());
 
     size_t size = array.size();
index 0d41c6b..f72f3ab 100644 (file)
@@ -1,3 +1,59 @@
+2017-09-14  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Add PrivateSymbolMode::{Include,Exclude} for PropertyNameArray
+        https://bugs.webkit.org/show_bug.cgi?id=176867
+
+        Reviewed by Sam Weinig.
+
+        We rarely require private symbols when enumerating property names.
+        This patch adds PrivateSymbolMode::{Include,Exclude}. If PrivateSymbolMode::Exclude
+        is specified, PropertyNameArray does not include private symbols.
+        This removes many ad-hoc `Identifier::isPrivateName()` in enumeration operations.
+
+        One additional good thing is that we do not need to filter private symbols out from PropertyNameArray.
+        It allows us to use Object.keys()'s fast path for Object.getOwnPropertySymbols.
+
+        object-get-own-property-symbols                48.6275+-1.0021     ^     38.1846+-1.7934        ^ definitely 1.2735x faster
+
+        * API/JSObjectRef.cpp:
+        (JSObjectCopyPropertyNames):
+        * bindings/ScriptValue.cpp:
+        (Inspector::jsToInspectorValue):
+        * bytecode/ObjectAllocationProfile.h:
+        (JSC::ObjectAllocationProfile::possibleDefaultPropertyCount):
+        * runtime/EnumerationMode.h:
+        * runtime/IntlObject.cpp:
+        (JSC::supportedLocales):
+        * runtime/JSONObject.cpp:
+        (JSC::Stringifier::Stringifier):
+        (JSC::Stringifier::Holder::appendNextProperty):
+        (JSC::Walker::walk):
+        * runtime/JSPropertyNameEnumerator.cpp:
+        (JSC::JSPropertyNameEnumerator::create):
+        * runtime/JSPropertyNameEnumerator.h:
+        (JSC::propertyNameEnumerator):
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorGetOwnPropertyDescriptors):
+        (JSC::objectConstructorAssign):
+        (JSC::objectConstructorValues):
+        (JSC::defineProperties):
+        (JSC::setIntegrityLevel):
+        (JSC::testIntegrityLevel):
+        (JSC::ownPropertyKeys):
+        * runtime/PropertyNameArray.h:
+        (JSC::PropertyNameArray::PropertyNameArray):
+        (JSC::PropertyNameArray::propertyNameMode const):
+        (JSC::PropertyNameArray::privateSymbolMode const):
+        (JSC::PropertyNameArray::addUncheckedInternal):
+        (JSC::PropertyNameArray::addUnchecked):
+        (JSC::PropertyNameArray::add):
+        (JSC::PropertyNameArray::isUidMatchedToTypeMode):
+        (JSC::PropertyNameArray::includeSymbolProperties const):
+        (JSC::PropertyNameArray::includeStringProperties const):
+        (JSC::PropertyNameArray::mode const): Deleted.
+        * runtime/ProxyObject.cpp:
+        (JSC::ProxyObject::performGetOwnPropertyNames):
+
 2017-09-13  Mark Lam  <mark.lam@apple.com>
 
         Rolling out r221832: Regresses Speedometer by ~4% and Dromaeo CSS YUI by ~20%.
index 0f15ae7..d0070ec 100644 (file)
@@ -79,7 +79,7 @@ static RefPtr<InspectorValue> jsToInspectorValue(ExecState& scriptState, JSValue
         }
         auto inspectorObject = InspectorObject::create();
         auto& object = *value.getObject();
-        PropertyNameArray propertyNames(&scriptState, PropertyNameMode::Strings);
+        PropertyNameArray propertyNames(&scriptState.vm(), PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
         object.methodTable()->getOwnPropertyNames(&object, &scriptState, propertyNames, EnumerationMode());
         for (auto& name : propertyNames) {
             auto inspectorValue = jsToInspectorValue(scriptState, object.get(&scriptState, name), maxDepth);
index 301a358..d9865f5 100644 (file)
@@ -132,7 +132,7 @@ private:
             return 0;
 
         size_t count = 0;
-        PropertyNameArray propertyNameArray(&vm, PropertyNameMode::StringsAndSymbols);
+        PropertyNameArray propertyNameArray(&vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Include);
         prototype->structure()->getPropertyNamesFromStructure(vm, propertyNameArray, EnumerationMode());
         PropertyNameArrayData::PropertyNameVector& propertyNameVector = propertyNameArray.data()->propertyNameVector();
         for (size_t i = 0; i < propertyNameVector.size(); ++i) {
index 7abbed2..31798f9 100644 (file)
@@ -33,6 +33,11 @@ enum class PropertyNameMode {
     StringsAndSymbols = Symbols | Strings,
 };
 
+enum class PrivateSymbolMode {
+    Include,
+    Exclude
+};
+
 enum class DontEnumPropertiesMode {
     Include,
     Exclude
index 25d21f2..730ee88 100644 (file)
@@ -830,7 +830,7 @@ JSValue supportedLocales(ExecState& state, const HashSet<String>& availableLocal
         : lookupSupportedLocales(state, availableLocales, requestedLocales);
     RETURN_IF_EXCEPTION(scope, JSValue());
 
-    PropertyNameArray keys(&state, PropertyNameMode::Strings);
+    PropertyNameArray keys(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
     supportedLocales->getOwnPropertyNames(supportedLocales, &state, keys, EnumerationMode());
     RETURN_IF_EXCEPTION(scope, JSValue());
 
index 0c7f3d1..ea68507 100644 (file)
@@ -224,7 +224,7 @@ Stringifier::Stringifier(ExecState* exec, const Local<Unknown>& replacer, const
     : m_exec(exec)
     , m_replacer(replacer)
     , m_usingArrayReplacer(false)
-    , m_arrayReplacerPropertyNames(exec, PropertyNameMode::Strings)
+    , m_arrayReplacerPropertyNames(&exec->vm(), PropertyNameMode::Strings, PrivateSymbolMode::Exclude)
     , m_replacerCallType(CallType::None)
 {
     VM& vm = exec->vm();
@@ -484,7 +484,7 @@ bool Stringifier::Holder::appendNextProperty(Stringifier& stringifier, StringBui
             if (stringifier.m_usingArrayReplacer)
                 m_propertyNames = stringifier.m_arrayReplacerPropertyNames.data();
             else {
-                PropertyNameArray objectPropertyNames(exec, PropertyNameMode::Strings);
+                PropertyNameArray objectPropertyNames(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
                 m_object->methodTable(vm)->getOwnPropertyNames(m_object.get(), exec, objectPropertyNames, EnumerationMode());
                 RETURN_IF_EXCEPTION(scope, false);
                 m_propertyNames = objectPropertyNames.releaseData();
@@ -705,7 +705,7 @@ NEVER_INLINE JSValue Walker::walk(JSValue unfiltered)
                 JSObject* object = asObject(inValue);
                 objectStack.push(object);
                 indexStack.append(0);
-                propertyStack.append(PropertyNameArray(m_exec, PropertyNameMode::Strings));
+                propertyStack.append(PropertyNameArray(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude));
                 object->methodTable(vm)->getOwnPropertyNames(object, m_exec, propertyStack.last(), EnumerationMode());
                 RETURN_IF_EXCEPTION(scope, { });
             }
index 5f8b0b3..82bb0dc 100644 (file)
@@ -37,7 +37,7 @@ const ClassInfo JSPropertyNameEnumerator::s_info = { "JSPropertyNameEnumerator",
 JSPropertyNameEnumerator* JSPropertyNameEnumerator::create(VM& vm)
 {
     if (!vm.emptyPropertyNameEnumerator.get()) {
-        PropertyNameArray propertyNames(&vm, PropertyNameMode::Strings);
+        PropertyNameArray propertyNames(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
         vm.emptyPropertyNameEnumerator = Strong<JSCell>(vm, create(vm, 0, 0, 0, WTFMove(propertyNames)));
     }
     return jsCast<JSPropertyNameEnumerator*>(vm.emptyPropertyNameEnumerator.get());
index 145557a..d403238 100644 (file)
@@ -113,7 +113,7 @@ inline JSPropertyNameEnumerator* propertyNameEnumerator(ExecState* exec, JSObjec
 
     uint32_t numberStructureProperties = 0;
 
-    PropertyNameArray propertyNames(exec, PropertyNameMode::Strings);
+    PropertyNameArray propertyNames(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
 
     if (structure->canAccessPropertiesQuicklyForEnumeration() && indexedLength == base->getArrayLength()) {
         base->methodTable(vm)->getStructurePropertyNames(base, exec, propertyNames, EnumerationMode());
index 7b0d70d..4ec64d3 100644 (file)
@@ -214,7 +214,7 @@ JSValue objectConstructorGetOwnPropertyDescriptors(ExecState* exec, JSObject* ob
 {
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
-    PropertyNameArray properties(exec, PropertyNameMode::StringsAndSymbols);
+    PropertyNameArray properties(&vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
     object->methodTable(vm)->getOwnPropertyNames(object, exec, properties, EnumerationMode(DontEnumPropertiesMode::Include));
     RETURN_IF_EXCEPTION(scope, { });
 
@@ -316,7 +316,7 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
         JSObject* source = sourceValue.toObject(exec);
         RETURN_IF_EXCEPTION(scope, { });
 
-        PropertyNameArray properties(exec, PropertyNameMode::StringsAndSymbols);
+        PropertyNameArray properties(&vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
         source->methodTable(vm)->getOwnPropertyNames(source, exec, properties, EnumerationMode(DontEnumPropertiesMode::Include));
         RETURN_IF_EXCEPTION(scope, { });
 
@@ -358,7 +358,8 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
         if (foundSymbol) {
             for (unsigned j = 0; j < numProperties; j++) {
                 const auto& propertyName = properties[j];
-                if (propertyName.isSymbol() && !propertyName.isPrivateName()) {
+                if (propertyName.isSymbol()) {
+                    ASSERT(!propertyName.isPrivateName());
                     assign(propertyName);
                     RETURN_IF_EXCEPTION(scope, { });
                 }
@@ -382,7 +383,7 @@ EncodedJSValue JSC_HOST_CALL objectConstructorValues(ExecState* exec)
     JSArray* values = constructEmptyArray(exec, nullptr);
     RETURN_IF_EXCEPTION(scope, { });
 
-    PropertyNameArray properties(exec, PropertyNameMode::Strings);
+    PropertyNameArray properties(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
     target->methodTable(vm)->getOwnPropertyNames(target, exec, properties, EnumerationMode(DontEnumPropertiesMode::Include));
     RETURN_IF_EXCEPTION(scope, { });
 
@@ -543,7 +544,7 @@ static JSValue defineProperties(ExecState* exec, JSObject* object, JSObject* pro
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    PropertyNameArray propertyNames(exec, PropertyNameMode::StringsAndSymbols);
+    PropertyNameArray propertyNames(&vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
     asObject(properties)->methodTable(vm)->getOwnPropertyNames(asObject(properties), exec, propertyNames, EnumerationMode(DontEnumPropertiesMode::Exclude));
     RETURN_IF_EXCEPTION(scope, { });
     size_t numProperties = propertyNames.size();
@@ -569,9 +570,9 @@ static JSValue defineProperties(ExecState* exec, JSObject* object, JSObject* pro
         }
     }
     for (size_t i = 0; i < numProperties; i++) {
-        Identifier propertyName = propertyNames[i];
-        if (propertyName.isPrivateName())
-            continue;
+        auto& propertyName = propertyNames[i];
+        ASSERT(!propertyName.isPrivateName());
+
         object->methodTable(vm)->defineOwnProperty(object, exec, propertyName, descriptors[i], true);
         RETURN_IF_EXCEPTION(scope, { });
     }
@@ -629,15 +630,14 @@ bool setIntegrityLevel(ExecState* exec, VM& vm, JSObject* object)
     if (UNLIKELY(!success))
         return false;
 
-    PropertyNameArray properties(exec, PropertyNameMode::StringsAndSymbols);
+    PropertyNameArray properties(&vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
     object->methodTable(vm)->getOwnPropertyNames(object, exec, properties, EnumerationMode(DontEnumPropertiesMode::Include));
     RETURN_IF_EXCEPTION(scope, false);
 
     PropertyNameArray::const_iterator end = properties.end();
     for (PropertyNameArray::const_iterator iter = properties.begin(); iter != end; ++iter) {
-        Identifier propertyName = *iter;
-        if (propertyName.isPrivateName())
-            continue;
+        auto& propertyName = *iter;
+        ASSERT(!propertyName.isPrivateName());
 
         PropertyDescriptor desc;
         if (level == IntegrityLevel::Sealed)
@@ -677,16 +677,15 @@ bool testIntegrityLevel(ExecState* exec, VM& vm, JSObject* object)
         return false;
 
     // 6. Let keys be ? O.[[OwnPropertyKeys]]().
-    PropertyNameArray keys(exec, PropertyNameMode::StringsAndSymbols);
+    PropertyNameArray keys(&vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
     object->methodTable(vm)->getOwnPropertyNames(object, exec, keys, EnumerationMode(DontEnumPropertiesMode::Include));
     RETURN_IF_EXCEPTION(scope, { });
 
     // 7. For each element k of keys, do
     PropertyNameArray::const_iterator end = keys.end();
     for (PropertyNameArray::const_iterator iter = keys.begin(); iter != end; ++iter) {
-        Identifier propertyName = *iter;
-        if (propertyName.isPrivateName())
-            continue;
+        auto& propertyName = *iter;
+        ASSERT(!propertyName.isPrivateName());
 
         // a. Let currentDesc be ? O.[[GetOwnProperty]](k)
         PropertyDescriptor desc;
@@ -836,7 +835,7 @@ JSArray* ownPropertyKeys(ExecState* exec, JSObject* object, PropertyNameMode pro
 {
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
-    PropertyNameArray properties(exec, propertyNameMode);
+    PropertyNameArray properties(&vm, propertyNameMode, PrivateSymbolMode::Exclude);
     object->methodTable(vm)->getOwnPropertyNames(object, exec, properties, EnumerationMode(dontEnumPropertiesMode));
     RETURN_IF_EXCEPTION(scope, nullptr);
 
@@ -854,18 +853,26 @@ JSArray* ownPropertyKeys(ExecState* exec, JSObject* object, PropertyNameMode pro
 
     // If !mustFilterProperty and PropertyNameMode::Strings mode, we do not need to filter out any entries in PropertyNameArray.
     // We can use fast allocation and initialization.
-    if (!mustFilterProperty && propertyNameMode == PropertyNameMode::Strings && properties.size() < MIN_SPARSE_ARRAY_INDEX) {
-        auto* globalObject = exec->lexicalGlobalObject();
-        if (LIKELY(!globalObject->isHavingABadTime())) {
-            size_t numProperties = properties.size();
-            JSArray* keys = JSArray::create(vm, globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous), numProperties);
-            WriteBarrier<Unknown>* buffer = keys->butterfly()->contiguous().data();
-            for (size_t i = 0; i < numProperties; i++) {
-                const auto& identifier = properties[i];
-                ASSERT(!identifier.isSymbol());
-                buffer[i].set(vm, keys, jsOwnedString(&vm, identifier.string()));
+    if (propertyNameMode != PropertyNameMode::StringsAndSymbols) {
+        ASSERT(propertyNameMode == PropertyNameMode::Strings || propertyNameMode == PropertyNameMode::Symbols);
+        if (!mustFilterProperty && properties.size() < MIN_SPARSE_ARRAY_INDEX) {
+            auto* globalObject = exec->lexicalGlobalObject();
+            if (LIKELY(!globalObject->isHavingABadTime())) {
+                size_t numProperties = properties.size();
+                JSArray* keys = JSArray::create(vm, globalObject->originalArrayStructureForIndexingType(ArrayWithContiguous), numProperties);
+                WriteBarrier<Unknown>* buffer = keys->butterfly()->contiguous().data();
+                for (size_t i = 0; i < numProperties; i++) {
+                    const auto& identifier = properties[i];
+                    if (propertyNameMode == PropertyNameMode::Strings) {
+                        ASSERT(!identifier.isSymbol());
+                        buffer[i].set(vm, keys, jsOwnedString(&vm, identifier.string()));
+                    } else {
+                        ASSERT(identifier.isSymbol());
+                        buffer[i].set(vm, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
+                    }
+                }
+                return keys;
             }
-            return keys;
         }
     }
 
@@ -897,13 +904,12 @@ JSArray* ownPropertyKeys(ExecState* exec, JSObject* object, PropertyNameMode pro
         for (size_t i = 0; i < numProperties; i++) {
             const auto& identifier = properties[i];
             ASSERT(identifier.isSymbol());
-            if (!identifier.isPrivateName()) {
-                bool hasProperty = filterPropertyIfNeeded(identifier);
-                EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
-                if (hasProperty)
-                    pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
-                RETURN_IF_EXCEPTION(scope, nullptr);
-            }
+            ASSERT(!identifier.isPrivateName());
+            bool hasProperty = filterPropertyIfNeeded(identifier);
+            EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
+            if (hasProperty)
+                pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
+            RETURN_IF_EXCEPTION(scope, nullptr);
         }
         break;
     }
@@ -913,15 +919,17 @@ JSArray* ownPropertyKeys(ExecState* exec, JSObject* object, PropertyNameMode pro
         size_t numProperties = properties.size();
         for (size_t i = 0; i < numProperties; i++) {
             const auto& identifier = properties[i];
-            if (identifier.isSymbol() && !identifier.isPrivateName())
+            if (identifier.isSymbol()) {
+                ASSERT(!identifier.isPrivateName());
                 propertySymbols.append(identifier);
-            else {
-                bool hasProperty = filterPropertyIfNeeded(identifier);
-                EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
-                if (hasProperty)
-                    pushDirect(exec, keys, jsOwnedString(exec, identifier.string()));
-                RETURN_IF_EXCEPTION(scope, nullptr);
+                continue;
             }
+
+            bool hasProperty = filterPropertyIfNeeded(identifier);
+            EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
+            if (hasProperty)
+                pushDirect(exec, keys, jsOwnedString(exec, identifier.string()));
+            RETURN_IF_EXCEPTION(scope, nullptr);
         }
 
         // To ensure the order defined in the spec (9.1.12), we append symbols at the last elements of keys.
index a7afe78..5e93b79 100644 (file)
@@ -47,15 +47,11 @@ private:
 // FIXME: Rename to PropertyNameArrayBuilder.
 class PropertyNameArray {
 public:
-    PropertyNameArray(VM* vm, PropertyNameMode mode)
+    PropertyNameArray(VM* vm, PropertyNameMode propertyNameMode, PrivateSymbolMode privateSymbolMode)
         : m_data(PropertyNameArrayData::create())
         , m_vm(vm)
-        , m_mode(mode)
-    {
-    }
-
-    PropertyNameArray(ExecState* exec, PropertyNameMode mode)
-        : PropertyNameArray(&exec->vm(), mode)
+        , m_propertyNameMode(propertyNameMode)
+        , m_privateSymbolMode(privateSymbolMode)
     {
     }
 
@@ -83,17 +79,21 @@ public:
     const_iterator begin() const { return m_data->propertyNameVector().begin(); }
     const_iterator end() const { return m_data->propertyNameVector().end(); }
 
-    PropertyNameMode mode() const { return m_mode; }
     bool includeSymbolProperties() const;
     bool includeStringProperties() const;
 
+    PropertyNameMode propertyNameMode() const { return m_propertyNameMode; }
+    PrivateSymbolMode privateSymbolMode() const { return m_privateSymbolMode; }
+
 private:
+    void addUncheckedInternal(UniquedStringImpl*);
     bool isUidMatchedToTypeMode(UniquedStringImpl* identifier);
 
     RefPtr<PropertyNameArrayData> m_data;
     HashSet<UniquedStringImpl*> m_set;
     VM* m_vm;
-    PropertyNameMode m_mode;
+    PropertyNameMode m_propertyNameMode;
+    PrivateSymbolMode m_privateSymbolMode;
 };
 
 ALWAYS_INLINE void PropertyNameArray::add(const Identifier& identifier)
@@ -101,11 +101,16 @@ ALWAYS_INLINE void PropertyNameArray::add(const Identifier& identifier)
     add(identifier.impl());
 }
 
+ALWAYS_INLINE void PropertyNameArray::addUncheckedInternal(UniquedStringImpl* identifier)
+{
+    m_data->propertyNameVector().append(Identifier::fromUid(m_vm, identifier));
+}
+
 ALWAYS_INLINE void PropertyNameArray::addUnchecked(UniquedStringImpl* identifier)
 {
     if (!isUidMatchedToTypeMode(identifier))
         return;
-    m_data->propertyNameVector().append(Identifier::fromUid(m_vm, identifier));
+    addUncheckedInternal(identifier);
 }
 
 ALWAYS_INLINE void PropertyNameArray::add(UniquedStringImpl* identifier)
@@ -129,24 +134,29 @@ ALWAYS_INLINE void PropertyNameArray::add(UniquedStringImpl* identifier)
             return;
     }
 
-    addUnchecked(identifier);
+    addUncheckedInternal(identifier);
 }
 
 ALWAYS_INLINE bool PropertyNameArray::isUidMatchedToTypeMode(UniquedStringImpl* identifier)
 {
-    if (identifier->isSymbol())
-        return includeSymbolProperties();
+    if (identifier->isSymbol()) {
+        if (!includeSymbolProperties())
+            return false;
+        if (UNLIKELY(m_privateSymbolMode == PrivateSymbolMode::Include))
+            return true;
+        return !static_cast<SymbolImpl*>(identifier)->isPrivate();
+    }
     return includeStringProperties();
 }
 
 ALWAYS_INLINE bool PropertyNameArray::includeSymbolProperties() const
 {
-    return static_cast<std::underlying_type<PropertyNameMode>::type>(m_mode) & static_cast<std::underlying_type<PropertyNameMode>::type>(PropertyNameMode::Symbols);
+    return static_cast<std::underlying_type<PropertyNameMode>::type>(m_propertyNameMode) & static_cast<std::underlying_type<PropertyNameMode>::type>(PropertyNameMode::Symbols);
 }
 
 ALWAYS_INLINE bool PropertyNameArray::includeStringProperties() const
 {
-    return static_cast<std::underlying_type<PropertyNameMode>::type>(m_mode) & static_cast<std::underlying_type<PropertyNameMode>::type>(PropertyNameMode::Strings);
+    return static_cast<std::underlying_type<PropertyNameMode>::type>(m_propertyNameMode) & static_cast<std::underlying_type<PropertyNameMode>::type>(PropertyNameMode::Strings);
 }
 
 } // namespace JSC
index 9231b0e..07bec09 100644 (file)
@@ -911,7 +911,7 @@ void ProxyObject::performGetOwnPropertyNames(ExecState* exec, PropertyNameArray&
     JSValue arrayLikeObject = call(exec, ownKeysMethod, callType, callData, handler, arguments);
     RETURN_IF_EXCEPTION(scope, void());
 
-    PropertyNameMode propertyNameMode = trapResult.mode();
+    PropertyNameMode propertyNameMode = trapResult.propertyNameMode();
     RuntimeTypeMask resultFilter = 0;
     switch (propertyNameMode) {
     case PropertyNameMode::Symbols:
@@ -949,7 +949,7 @@ void ProxyObject::performGetOwnPropertyNames(ExecState* exec, PropertyNameArray&
     bool targetIsExensible = target->isExtensible(exec);
     RETURN_IF_EXCEPTION(scope, void());
 
-    PropertyNameArray targetKeys(&vm, propertyNameMode);
+    PropertyNameArray targetKeys(&vm, propertyNameMode, trapResult.privateSymbolMode());
     target->methodTable(vm)->getOwnPropertyNames(target, exec, targetKeys, enumerationMode);
     RETURN_IF_EXCEPTION(scope, void());
     Vector<UniquedStringImpl*> targetConfigurableKeys;
index 873fd00..d7fef62 100644 (file)
@@ -1,3 +1,16 @@
+2017-09-14  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Add PrivateSymbolMode::{Include,Exclude} for PropertyNameArray
+        https://bugs.webkit.org/show_bug.cgi?id=176867
+
+        Reviewed by Sam Weinig.
+
+        * bindings/js/JSDOMConvertRecord.h:
+        * bindings/js/SerializedScriptValue.cpp:
+        (WebCore::CloneSerializer::serialize):
+        * bridge/NP_jsobject.cpp:
+        (_NPN_Enumerate):
+
 2017-09-14  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Timeline should show when events preventDefault() was called on an event or not
index e1b068a..eba9876 100644 (file)
@@ -85,7 +85,7 @@ template<typename K, typename V> struct Converter<IDLRecord<K, V>> : DefaultConv
         ReturnType result;
     
         // 4. Let keys be ? O.[[OwnPropertyKeys]]().
-        JSC::PropertyNameArray keys(&vm, JSC::PropertyNameMode::Strings);
+        JSC::PropertyNameArray keys(&vm, JSC::PropertyNameMode::Strings, JSC::PrivateSymbolMode::Exclude);
         object->methodTable(vm)->getOwnPropertyNames(object, &state, keys, JSC::EnumerationMode());
 
         RETURN_IF_EXCEPTION(scope, { });
index c3e0631..3c18a51 100644 (file)
@@ -1490,7 +1490,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
                     indexStack.removeLast();
                     lengthStack.removeLast();
 
-                    propertyStack.append(PropertyNameArray(m_exec, PropertyNameMode::Strings));
+                    propertyStack.append(PropertyNameArray(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude));
                     array->methodTable(vm)->getOwnNonIndexPropertyNames(array, m_exec, propertyStack.last(), EnumerationMode());
                     if (propertyStack.last().size()) {
                         write(NonIndexPropertiesTag);
@@ -1540,7 +1540,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
                     return SerializationReturnCode::DataCloneError;
                 inputObjectStack.append(inObject);
                 indexStack.append(0);
-                propertyStack.append(PropertyNameArray(m_exec, PropertyNameMode::Strings));
+                propertyStack.append(PropertyNameArray(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude));
                 inObject->methodTable(vm)->getOwnPropertyNames(inObject, m_exec, propertyStack.last(), EnumerationMode());
             }
             objectStartVisitMember:
@@ -1608,7 +1608,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
                     mapIteratorStack.removeLast();
                     JSObject* object = inputObjectStack.last();
                     ASSERT(jsDynamicDowncast<JSMap*>(vm, object));
-                    propertyStack.append(PropertyNameArray(m_exec, PropertyNameMode::Strings));
+                    propertyStack.append(PropertyNameArray(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude));
                     object->methodTable(vm)->getOwnPropertyNames(object, m_exec, propertyStack.last(), EnumerationMode());
                     write(NonMapPropertiesTag);
                     indexStack.append(0);
@@ -1652,7 +1652,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
                     setIteratorStack.removeLast();
                     JSObject* object = inputObjectStack.last();
                     ASSERT(jsDynamicDowncast<JSSet*>(vm, object));
-                    propertyStack.append(PropertyNameArray(m_exec, PropertyNameMode::Strings));
+                    propertyStack.append(PropertyNameArray(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude));
                     object->methodTable(vm)->getOwnPropertyNames(object, m_exec, propertyStack.last(), EnumerationMode());
                     write(NonSetPropertiesTag);
                     indexStack.append(0);
index ef8a42c..12bb9e5 100644 (file)
@@ -484,7 +484,7 @@ bool _NPN_Enumerate(NPP, NPObject* o, NPIdentifier** identifier, uint32_t* count
         auto scope = DECLARE_CATCH_SCOPE(vm);
 
         ExecState* exec = globalObject->globalExec();
-        PropertyNameArray propertyNames(exec, PropertyNameMode::Strings);
+        PropertyNameArray propertyNames(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
 
         obj->imp->methodTable(vm)->getPropertyNames(obj->imp, exec, propertyNames, EnumerationMode());
         unsigned size = static_cast<unsigned>(propertyNames.size());
index 3d7dc95..813a516 100644 (file)
@@ -1,3 +1,13 @@
+2017-09-14  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Add PrivateSymbolMode::{Include,Exclude} for PropertyNameArray
+        https://bugs.webkit.org/show_bug.cgi?id=176867
+
+        Reviewed by Sam Weinig.
+
+        * WebProcess/Plugins/Netscape/NPJSObject.cpp:
+        (WebKit::NPJSObject::enumerate):
+
 2017-09-14  Maureen Daum  <mdaum@apple.com>
 
         Introduce the option to mark an HTML element as having AutoFill available.
index f3f79f4..8d0a145 100644 (file)
@@ -253,7 +253,7 @@ bool NPJSObject::enumerate(NPIdentifier** identifiers, uint32_t* identifierCount
     VM& vm = exec->vm();
     JSLockHolder lock(vm);
 
-    PropertyNameArray propertyNames(exec, PropertyNameMode::Strings);
+    PropertyNameArray propertyNames(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
     m_jsObject->methodTable(vm)->getPropertyNames(m_jsObject.get(), exec, propertyNames, EnumerationMode());
 
     NPIdentifier* nameIdentifiers = npnMemNewArray<NPIdentifier>(propertyNames.size());
index 2b7d11f..687184e 100644 (file)
@@ -1,3 +1,13 @@
+2017-09-14  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Add PrivateSymbolMode::{Include,Exclude} for PropertyNameArray
+        https://bugs.webkit.org/show_bug.cgi?id=176867
+
+        Reviewed by Sam Weinig.
+
+        * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
+        (WebKit::NetscapePluginInstanceProxy::enumerate):
+
 2017-09-13  Andy Estes  <aestes@apple.com>
 
         [CF] Upstream CFNetwork-related WebKitSystemInterface functions
index 2f60f38..9984151 100644 (file)
@@ -1276,7 +1276,7 @@ bool NetscapePluginInstanceProxy::enumerate(uint32_t objectID, data_t& resultDat
 
     ExecState* exec = frame->script().globalObject(pluginWorld())->globalExec();
  
-    PropertyNameArray propertyNames(exec, PropertyNameMode::Strings);
+    PropertyNameArray propertyNames(&vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
     object->methodTable(vm)->getPropertyNames(object, exec, propertyNames, EnumerationMode());
 
     RetainPtr<NSMutableArray*> array = adoptNS([[NSMutableArray alloc] init]);