Changeset 182205 in webkit for trunk/Source/JavaScriptCore/runtime/PropertyNameArray.cpp

Timestamp:

Mar 31, 2015, 2:25:14 PM (10 years ago)

Author:

Yusuke Suzuki

Message:

Clean up Identifier factories to clarify the meaning of StringImpl*
​https://bugs.webkit.org/show_bug.cgi?id=143146

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

In the a lot of places, Identifier(VM*/ExecState*, StringImpl*) constructor is used.
However, it's ambiguous because StringImpl* has 2 different meanings.
1) normal string, it is replacable with WTFString and
2) uid, which holds isSymbol information to represent Symbols.
So we dropped Identifier constructors for strings and instead, introduced 2 factory functions.
+ Identifier::fromString(VM*/ExecState*, const String&).
Just construct Identifier from strings. The symbol-ness of StringImpl* is not kept.
+ Identifier::fromUid(VM*/ExecState*, StringImpl*).
This function is used for 2) uid. So symbol-ness of StringImpl* is kept.

And to clean up StringImpl which is used as uid,
we introduce StringKind into StringImpl. There's 3 kinds

1. StringNormal (non-atomic, non-symbol)
2. StringAtomic (atomic, non-symbol)
3. StringSymbol (non-atomic, symbol)

They are mutually exclusive. And (atomic, symbol) case should not exist.

• API/JSCallbackObjectFunctions.h:

(JSC::JSCallbackObject<Parent>::getOwnNonIndexPropertyNames):

• API/JSObjectRef.cpp:

(JSObjectMakeFunction):

• API/OpaqueJSString.cpp:

(OpaqueJSString::identifier):

• bindings/ScriptFunctionCall.cpp:

(Deprecated::ScriptFunctionCall::call):

• builtins/BuiltinExecutables.cpp:

(JSC::BuiltinExecutables::createExecutableInternal):

• builtins/BuiltinNames.h:

(JSC::BuiltinNames::BuiltinNames):

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::emitThrowReferenceError):
(JSC::BytecodeGenerator::emitThrowTypeError):
(JSC::BytecodeGenerator::emitReadOnlyExceptionIfNeeded):
(JSC::BytecodeGenerator::emitEnumeration):

• dfg/DFGDesiredIdentifiers.cpp:

(JSC::DFG::DesiredIdentifiers::reallyAdd):

• inspector/JSInjectedScriptHost.cpp:

(Inspector::JSInjectedScriptHost::functionDetails):
(Inspector::constructInternalProperty):
(Inspector::JSInjectedScriptHost::weakMapEntries):
(Inspector::JSInjectedScriptHost::iteratorEntries):

• inspector/JSInjectedScriptHostPrototype.cpp:

(Inspector::JSInjectedScriptHostPrototype::finishCreation):

• inspector/JSJavaScriptCallFramePrototype.cpp:
• inspector/ScriptCallStackFactory.cpp:

(Inspector::extractSourceInformationFromException):

• jit/JITOperations.cpp:
• jsc.cpp:

(GlobalObject::finishCreation):
(GlobalObject::addFunction):
(GlobalObject::addConstructableFunction):
(functionRun):
(runWithScripts):

• llint/LLIntData.cpp:

(JSC::LLInt::Data::performAssertions):

• llint/LowLevelInterpreter.asm:
• parser/ASTBuilder.h:

(JSC::ASTBuilder::addVar):

• parser/Parser.cpp:

(JSC::Parser<LexerType>::parseInner):
(JSC::Parser<LexerType>::createBindingPattern):

• parser/ParserArena.h:

(JSC::IdentifierArena::makeIdentifier):
(JSC::IdentifierArena::makeIdentifierLCharFromUChar):
(JSC::IdentifierArena::makeNumericIdentifier):

• runtime/ArgumentsIteratorPrototype.cpp:

(JSC::ArgumentsIteratorPrototype::finishCreation):

• runtime/ArrayIteratorPrototype.cpp:

(JSC::ArrayIteratorPrototype::finishCreation):

• runtime/ArrayPrototype.cpp:

(JSC::ArrayPrototype::finishCreation):
(JSC::arrayProtoFuncPush):

• runtime/ClonedArguments.cpp:

(JSC::ClonedArguments::getOwnPropertySlot):

• runtime/CommonIdentifiers.cpp:

(JSC::CommonIdentifiers::CommonIdentifiers):

• runtime/CommonIdentifiers.h:
• runtime/Error.cpp:

(JSC::addErrorInfo):
(JSC::hasErrorInfo):

• runtime/ExceptionHelpers.cpp:

(JSC::createUndefinedVariableError):

• runtime/GenericArgumentsInlines.h:

(JSC::GenericArguments<Type>::getOwnPropertySlot):

• runtime/Identifier.h:

(JSC::Identifier::isSymbol):
(JSC::Identifier::Identifier):
(JSC::Identifier::from): Deleted.

• runtime/IdentifierInlines.h:

(JSC::Identifier::Identifier):
(JSC::Identifier::fromUid):
(JSC::Identifier::fromString):

• runtime/JSCJSValue.cpp:

(JSC::JSValue::dumpInContextAssumingStructure):

• runtime/JSCJSValueInlines.h:

(JSC::JSValue::toPropertyKey):

• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

• runtime/JSLexicalEnvironment.cpp:

(JSC::JSLexicalEnvironment::getOwnNonIndexPropertyNames):

• runtime/JSObject.cpp:

(JSC::getClassPropertyNames):
(JSC::JSObject::reifyStaticFunctionsForDelete):

• runtime/JSObject.h:

(JSC::makeIdentifier):

• runtime/JSPromiseConstructor.cpp:

(JSC::JSPromiseConstructorFuncRace):
(JSC::JSPromiseConstructorFuncAll):

• runtime/JSString.h:

(JSC::JSString::toIdentifier):

• runtime/JSSymbolTableObject.cpp:

(JSC::JSSymbolTableObject::getOwnNonIndexPropertyNames):

• runtime/LiteralParser.cpp:

(JSC::LiteralParser<CharType>::tryJSONPParse):
(JSC::LiteralParser<CharType>::makeIdentifier):

• runtime/Lookup.h:

(JSC::reifyStaticProperties):

• runtime/MapConstructor.cpp:

(JSC::constructMap):

• runtime/MapIteratorPrototype.cpp:

(JSC::MapIteratorPrototype::finishCreation):

• runtime/MapPrototype.cpp:

(JSC::MapPrototype::finishCreation):

• runtime/MathObject.cpp:

(JSC::MathObject::finishCreation):

• runtime/NumberConstructor.cpp:

(JSC::NumberConstructor::finishCreation):

• runtime/ObjectConstructor.cpp:

(JSC::ObjectConstructor::finishCreation):

• runtime/PrivateName.h:

(JSC::PrivateName::PrivateName):

• runtime/PropertyMapHashTable.h:

(JSC::PropertyTable::find):
(JSC::PropertyTable::get):

• runtime/PropertyName.h:

(JSC::PropertyName::PropertyName):
(JSC::PropertyName::publicName):
(JSC::PropertyName::asIndex):

• runtime/PropertyNameArray.cpp:

(JSC::PropertyNameArray::add):

• runtime/PropertyNameArray.h:

(JSC::PropertyNameArray::addKnownUnique):

• runtime/RegExpConstructor.cpp:

(JSC::RegExpConstructor::finishCreation):

• runtime/SetConstructor.cpp:

(JSC::constructSet):

• runtime/SetIteratorPrototype.cpp:

(JSC::SetIteratorPrototype::finishCreation):

• runtime/SetPrototype.cpp:

(JSC::SetPrototype::finishCreation):

• runtime/StringIteratorPrototype.cpp:

(JSC::StringIteratorPrototype::finishCreation):

• runtime/StringPrototype.cpp:

(JSC::StringPrototype::finishCreation):

• runtime/Structure.cpp:

(JSC::Structure::getPropertyNamesFromStructure):

• runtime/SymbolConstructor.cpp:
• runtime/VM.cpp:

(JSC::VM::throwException):

• runtime/WeakMapConstructor.cpp:

(JSC::constructWeakMap):

Source/WebCore:

Just change Identifier creations.

1. If the code creates Identifier from StringImpl*

which is treated as symbol or string(unique id), use Identifier::fromUid.

2. If the code creates Identifier from string, use Identifier::fromString.

• Modules/plugins/QuickTimePluginReplacement.mm:

(WebCore::QuickTimePluginReplacement::ensureReplacementScriptInjected):
(WebCore::QuickTimePluginReplacement::installReplacement):

• bindings/js/IDBBindingUtilities.cpp:

(WebCore::get):
(WebCore::set):

• bindings/js/JSCSSStyleDeclarationCustom.cpp:

(WebCore::JSCSSStyleDeclaration::getOwnPropertyNames):

• bindings/js/JSCallbackData.cpp:

(WebCore::JSCallbackData::invokeCallback):

• bindings/js/JSCommandLineAPIHostCustom.cpp:

(WebCore::getJSListenerFunctions):
(WebCore::JSCommandLineAPIHost::getEventListeners):

• bindings/js/JSCryptoAlgorithmBuilder.cpp:

(WebCore::JSCryptoAlgorithmBuilder::add):

• bindings/js/JSCryptoAlgorithmDictionary.cpp:

(WebCore::getProperty):
(WebCore::getHashAlgorithm):

• bindings/js/JSCryptoKeySerializationJWK.cpp:

(WebCore::getJSArrayFromJSON):
(WebCore::getStringFromJSON):
(WebCore::getBooleanFromJSON):
(WebCore::addToJSON):
(WebCore::buildJSONForRSAComponents):
(WebCore::addBoolToJSON):
(WebCore::addUsagesToJSON):

• bindings/js/JSCustomXPathNSResolver.cpp:

(WebCore::JSCustomXPathNSResolver::lookupNamespaceURI):

• bindings/js/JSDOMStringMapCustom.cpp:

(WebCore::JSDOMStringMap::getOwnPropertyNames):

• bindings/js/JSDOMWindowCustom.cpp:

(WebCore::JSDOMWindow::defineOwnProperty):
(WebCore::JSDOMWindow::setLocation):
(WebCore::DialogHandler::dialogCreated):
(WebCore::DialogHandler::returnValue):

• bindings/js/JSDeviceMotionEventCustom.cpp:

(WebCore::readAccelerationArgument):
(WebCore::readRotationRateArgument):
(WebCore::createAccelerationObject):
(WebCore::createRotationRateObject):

• bindings/js/JSDictionary.cpp:

(WebCore::JSDictionary::tryGetProperty):

• bindings/js/JSEventListener.cpp:

(WebCore::JSEventListener::handleEvent):

• bindings/js/JSHTMLAllCollectionCustom.cpp:

(WebCore::callHTMLAllCollection):
(WebCore::JSHTMLAllCollection::item):
(WebCore::JSHTMLAllCollection::namedItem):

• bindings/js/JSHTMLDocumentCustom.cpp:

(WebCore::JSHTMLDocument::all):
(WebCore::JSHTMLDocument::setAll):
(WebCore::JSHTMLDocument::open):

• bindings/js/JSHTMLFormControlsCollectionCustom.cpp:

(WebCore::JSHTMLFormControlsCollection::namedItem):

• bindings/js/JSIDBDatabaseCustom.cpp:

(WebCore::JSIDBDatabase::createObjectStore):

• bindings/js/JSIDBObjectStoreCustom.cpp:

(WebCore::JSIDBObjectStore::createIndex):

• bindings/js/JSImageDataCustom.cpp:

(WebCore::toJS):

• bindings/js/JSInspectorFrontendHostCustom.cpp:

(WebCore::populateContextMenuItems):

• bindings/js/JSLazyEventListener.cpp:

(WebCore::JSLazyEventListener::initializeJSFunction):

• bindings/js/JSNodeFilterCondition.cpp:

(WebCore::JSNodeFilterCondition::acceptNode):

• bindings/js/JSSQLResultSetRowListCustom.cpp:

(WebCore::JSSQLResultSetRowList::item):

• bindings/js/JSStorageCustom.cpp:

(WebCore::JSStorage::getOwnPropertyNames):

• bindings/js/ReadableStreamJSSource.cpp:

(WebCore::setInternalSlotToObject):
(WebCore::getInternalSlotFromObject):

• bindings/js/ScriptGlobalObject.cpp:

(WebCore::ScriptGlobalObject::set):
(WebCore::ScriptGlobalObject::get):
(WebCore::ScriptGlobalObject::remove):

• bindings/js/SerializedScriptValue.cpp:

(WebCore::CloneSerializer::CloneSerializer):
(WebCore::CloneSerializer::write):
(WebCore::CloneDeserializer::deserialize):

• bindings/objc/WebScriptObject.mm:

(-[WebScriptObject callWebScriptMethod:withArguments:]):
(-[WebScriptObject setValue:forKey:]):
(-[WebScriptObject valueForKey:]):
(-[WebScriptObject removeWebScriptKey:]):
(-[WebScriptObject hasWebScriptKey:]):

• bindings/scripts/CodeGeneratorJS.pm:

(GenerateImplementation):

• bindings/scripts/test/JS/JSFloat64Array.cpp:

(WebCore::JSFloat64Array::getOwnPropertyNames):

• bindings/scripts/test/JS/JSTestEventTarget.cpp:

(WebCore::JSTestEventTarget::getOwnPropertyNames):

• bindings/scripts/test/JS/JSTestObj.cpp:

(WebCore::setJSTestObjTestSubObjEnabledBySettingConstructor):
(WebCore::setJSTestObjConditionalAttr4Constructor):
(WebCore::setJSTestObjConditionalAttr5Constructor):
(WebCore::setJSTestObjConditionalAttr6Constructor):
(WebCore::setJSTestObjReplaceableAttribute):

• bridge/c/c_utility.cpp:

(JSC::Bindings::identifierFromNPIdentifier):

• bridge/objc/objc_runtime.mm:

(JSC::Bindings::ObjcFallbackObjectImp::defaultValue):

• bridge/testbindings.cpp:

(main):

• bridge/testbindings.mm:

(main):

• contentextensions/ContentExtensionParser.cpp:

(WebCore::ContentExtensions::loadTrigger):
(WebCore::ContentExtensions::loadAction):

• html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::parseAttribute):

• html/HTMLPlugInImageElement.cpp:

(WebCore::HTMLPlugInImageElement::didAddUserAgentShadowRoot):

• testing/js/WebCoreTestSupport.cpp:

(WebCoreTestSupport::injectInternalsObject):

Source/WebKit/mac:

Just change to Identifier::fromString.

• Plugins/Hosted/NetscapePluginHostProxy.mm:

(identifierFromIdentifierRep):

• Plugins/Hosted/ProxyInstance.mm:

(WebKit::ProxyInstance::getPropertyNames):

Source/WebKit2:

Just change to Identifier::fromString.

• WebProcess/Plugins/Netscape/JSNPObject.cpp:

(WebKit::JSNPObject::getOwnPropertyNames):

• WebProcess/Plugins/Netscape/NPJSObject.cpp:

(WebKit::identifierFromIdentifierRep):

Source/WTF:

Introduce StringKind into StringImpl. There's 3 kinds

1. StringNormal (non-atomic, non-symbol)
2. StringAtomic (atomic, non-symbol)
3. StringSymbol (non-atomic, symbol)

They are mutually exclusive.

• wtf/text/AtomicString.cpp:

(WTF::AtomicString::addSlowCase):
(WTF::AtomicString::findSlowCase):
When registering a string into AtomicStringTable,
it should not be a symbol string
because symbol and atomic types are mutually exclusive.
When a symbol string comes, we extract an owner string
from a symbol string by using StringImpl::extractFoldedStringInSymbol().
It always succeeds because a symbol (non empty) string
is always BufferSubstring and has an owner string.
Empty symbol string doesn't have an owner string.
This case is filtered by !string.length() guard.

• wtf/text/AtomicString.h:

(WTF::AtomicString::add):
(WTF::AtomicString::addWithStringTableProvider):

• wtf/text/StringImpl.cpp:

(WTF::StringImpl::~StringImpl):
(WTF::StringImpl::createSymbol):
(WTF::StringImpl::createUnique): Deleted.

• wtf/text/StringImpl.h:

(WTF::StringImpl::StringImpl):
(WTF::StringImpl::createSymbolEmpty):
(WTF::StringImpl::flagIsAtomic):
(WTF::StringImpl::flagIsSymbol):
(WTF::StringImpl::maskStringKind):
(WTF::StringImpl::stringKind):
(WTF::StringImpl::isSymbol):
(WTF::StringImpl::isAtomic):
(WTF::StringImpl::setIsAtomic):
(WTF::StringImpl::extractFoldedStringInSymbol):
(WTF::StringImpl::createUniqueEmpty): Deleted.
(WTF::StringImpl::flagIsUnique): Deleted.
(WTF::StringImpl::isUnique): Deleted.

• wtf/text/StringStatics.cpp:

(WTF::StringImpl::hashAndFlagsForSymbol):
(WTF::StringImpl::hashAndFlagsForUnique): Deleted.

Tools:

Simple API tests for StringImpl are added.

• TestWebKitAPI/Tests/WTF/StringImpl.cpp:

(TestWebKitAPI::TEST):

File:

• trunk/Source/JavaScriptCore/runtime/PropertyNameArray.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/runtime/PropertyNameArray.cpp

@@ -32 +32 @@
 void PropertyNameArray::add(StringImpl* identifier)
 {
-    ASSERT(!identifier || identifier == StringImpl::empty() || identifier->isAtomic());
+    ASSERT(!identifier || (identifier == StringImpl::empty() || identifier->isAtomic() || identifier->isSymbol()));
     if (!ASSERT_DISABLED) {
-        uint32_t index = PropertyName(Identifier(m_vm, identifier)).asIndex();
+        uint32_t index = PropertyName(Identifier::fromUid(m_vm, identifier)).asIndex();
         ASSERT_UNUSED(index, index == PropertyName::NotAnIndex || index >= m_previouslyEnumeratedLength);
     }