Skip to content

Upgrade WebKit to 5c046702474d08edd6319a653919392e461d76ab #21606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Upgrade WebKit to 5c046702474d08edd6319a653919392e461d76ab #21606

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3843c96
Upgrade WebKit to 5c046702474d08edd6319a653919392e461d76ab
Aug 4, 2025
651debe
[autofix.ci] apply automated fixes
autofix-ci[bot] Aug 4, 2025
20584f3
Merge branch 'main' into claude/webkit-upgrade-5c046702
alii Aug 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmake/tools/SetupWebKit.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ option(WEBKIT_VERSION "The version of WebKit to use")
option(WEBKIT_LOCAL "If a local version of WebKit should be used instead of downloading")

if(NOT WEBKIT_VERSION)
set(WEBKIT_VERSION 642e2252f6298387edb6d2f991a0408fd0320466)
set(WEBKIT_VERSION 5c046702474d08edd6319a653919392e461d76ab)
endif()

string(SUBSTRING ${WEBKIT_VERSION} 0 16 WEBKIT_VERSION_PREFIX)
Expand Down
36 changes: 24 additions & 12 deletions src/bun.js/bindings/NodeVMSyntheticModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,24 +94,36 @@ void NodeVMSyntheticModule::createModuleRecord(JSGlobalObject* globalObject)
{
VM& vm = globalObject->vm();

SyntheticModuleRecord* moduleRecord = SyntheticModuleRecord::create(globalObject, vm, globalObject->syntheticModuleRecordStructure(), Identifier::fromString(vm, identifier()));
// Convert export names to Vector<Identifier>
Vector<Identifier, 4> exportIdentifiers;
for (const String& exportName : m_exportNames) {
exportIdentifiers.append(Identifier::fromString(vm, exportName));
}

m_moduleRecord.set(vm, this, moduleRecord);
// Create empty export values - they will be filled later during evaluation
MarkedArgumentBuffer exportValues;
for (size_t i = 0; i < m_exportNames.size(); ++i) {
exportValues.append(jsUndefined());
}

SymbolTable* exportSymbolTable = SymbolTable::create(vm);
// Use the new API that handles module environment creation internally
SyntheticModuleRecord* moduleRecord = SyntheticModuleRecord::tryCreateWithExportNamesAndValues(
globalObject,
Identifier::fromString(vm, identifier()),
exportIdentifiers,
exportValues);

ScopeOffset offset = exportSymbolTable->takeNextScopeOffset(NoLockingNecessary);
exportSymbolTable->set(NoLockingNecessary, vm.propertyNames->starNamespacePrivateName.impl(), SymbolTableEntry(VarOffset(offset)));
if (!moduleRecord) {
// Fallback to old approach if new API fails
moduleRecord = SyntheticModuleRecord::create(globalObject, vm, globalObject->syntheticModuleRecordStructure(), Identifier::fromString(vm, identifier()));

for (const String& exportName : m_exportNames) {
auto offset = exportSymbolTable->takeNextScopeOffset(NoLockingNecessary);
Identifier exportIdentifier = Identifier::fromString(vm, exportName);
moduleRecord->addExportEntry(SyntheticModuleRecord::ExportEntry::createLocal(exportIdentifier, exportIdentifier));
exportSymbolTable->set(NoLockingNecessary, exportIdentifier.releaseImpl().get(), SymbolTableEntry(VarOffset(offset)));
for (const String& exportName : m_exportNames) {
Identifier exportIdentifier = Identifier::fromString(vm, exportName);
moduleRecord->addExportEntry(SyntheticModuleRecord::ExportEntry::createLocal(exportIdentifier, exportIdentifier));
}
Comment on lines +116 to +123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback implementation is missing critical components from the original code. If tryCreateWithExportNamesAndValues() fails, the current fallback creates a module record and adds export entries, but omits:

  1. Symbol table creation
  2. Module environment setup
  3. The setModuleEnvironment() call

For complete backward compatibility, the fallback should include the full original implementation:

// Fallback to old approach if new API fails
moduleRecord = SyntheticModuleRecord::create(globalObject, vm, globalObject->syntheticModuleRecordStructure(), Identifier::fromString(vm, identifier()));

SymbolTable* exportSymbolTable = SymbolTable::create(vm);
ScopeOffset offset = exportSymbolTable->takeNextScopeOffset(NoLockingNecessary);
exportSymbolTable->set(NoLockingNecessary, vm.propertyNames->starNamespacePrivateName.impl(), SymbolTableEntry(VarOffset(offset)));

for (const String& exportName : m_exportNames) {
    auto offset = exportSymbolTable->takeNextScopeOffset(NoLockingNecessary);
    Identifier exportIdentifier = Identifier::fromString(vm, exportName);
    moduleRecord->addExportEntry(SyntheticModuleRecord::ExportEntry::createLocal(exportIdentifier, exportIdentifier));
    exportSymbolTable->set(NoLockingNecessary, exportIdentifier.releaseImpl().get(), SymbolTableEntry(VarOffset(offset)));
}

JSModuleEnvironment* moduleEnvironment = JSModuleEnvironment::create(vm, globalObject, nullptr, exportSymbolTable, jsTDZValue(), moduleRecord);
moduleRecord->setModuleEnvironment(globalObject, moduleEnvironment);
Suggested change
if (!moduleRecord) {
// Fallback to old approach if new API fails
moduleRecord = SyntheticModuleRecord::create(globalObject, vm, globalObject->syntheticModuleRecordStructure(), Identifier::fromString(vm, identifier()));
for (const String& exportName : m_exportNames) {
auto offset = exportSymbolTable->takeNextScopeOffset(NoLockingNecessary);
Identifier exportIdentifier = Identifier::fromString(vm, exportName);
moduleRecord->addExportEntry(SyntheticModuleRecord::ExportEntry::createLocal(exportIdentifier, exportIdentifier));
exportSymbolTable->set(NoLockingNecessary, exportIdentifier.releaseImpl().get(), SymbolTableEntry(VarOffset(offset)));
for (const String& exportName : m_exportNames) {
Identifier exportIdentifier = Identifier::fromString(vm, exportName);
moduleRecord->addExportEntry(SyntheticModuleRecord::ExportEntry::createLocal(exportIdentifier, exportIdentifier));
}
if (!moduleRecord) {
// Fallback to old approach if new API fails
moduleRecord = SyntheticModuleRecord::create(globalObject, vm, globalObject->syntheticModuleRecordStructure(), Identifier::fromString(vm, identifier()));
SymbolTable* exportSymbolTable = SymbolTable::create(vm);
ScopeOffset offset = exportSymbolTable->takeNextScopeOffset(NoLockingNecessary);
exportSymbolTable->set(NoLockingNecessary, vm.propertyNames->starNamespacePrivateName.impl(), SymbolTableEntry(VarOffset(offset)));
for (const String& exportName : m_exportNames) {
auto offset = exportSymbolTable->takeNextScopeOffset(NoLockingNecessary);
Identifier exportIdentifier = Identifier::fromString(vm, exportName);
moduleRecord->addExportEntry(SyntheticModuleRecord::ExportEntry::createLocal(exportIdentifier, exportIdentifier));
exportSymbolTable->set(NoLockingNecessary, exportIdentifier.releaseImpl().get(), SymbolTableEntry(VarOffset(offset)));
}
JSModuleEnvironment* moduleEnvironment = JSModuleEnvironment::create(vm, globalObject, nullptr, exportSymbolTable, jsTDZValue(), moduleRecord);
moduleRecord->setModuleEnvironment(globalObject, moduleEnvironment);

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

}

JSModuleEnvironment* moduleEnvironment = JSModuleEnvironment::create(vm, globalObject, nullptr, exportSymbolTable, jsTDZValue(), moduleRecord);
moduleRecord->setModuleEnvironment(globalObject, moduleEnvironment);
m_moduleRecord.set(vm, this, moduleRecord);
}

void NodeVMSyntheticModule::ensureModuleRecord(JSGlobalObject* globalObject)
Expand Down