Title: [205895] trunk/Source
- Revision
- 205895
- Author
- [email protected]
- Date
- 2016-09-13 19:29:22 -0700 (Tue, 13 Sep 2016)
Log Message
Promises aren't resolved properly when making a ObjC API callback
https://bugs.webkit.org/show_bug.cgi?id=161929
Reviewed by Geoffrey Garen.
Source/_javascript_Core:
When we go to call out to an Objective C function registered via the API,
we first drop all JSC locks to make the call. As part of dropping the locks,
we drain the microtask queue that is used among other things for handling deferred
promise resolution. The DropAllLocks scope class that drops the locks while in
scope, resets the current thread's AtomicStringTable to the default table. This
is wrong for two reasons, first it happens before we drain the microtask queue and
second it isn't needed as JSLock::willReleaseLock() restores the current thread's
AtomicStringTable to the table before the lock was acquired.
In fact, the manipulation of the current thread's AtomicStringTable is already
properly handled as a stack in JSLock::didAcquireLock() and willReleaseLock().
Therefore the manipulation of the AtomicStringTable in DropAllLocks constructor
and destructor should be removed.
* API/tests/testapi.mm:
(testObjectiveCAPIMain): Added a new test.
* runtime/JSLock.cpp:
(JSC::JSLock::DropAllLocks::DropAllLocks):
(JSC::JSLock::DropAllLocks::~DropAllLocks):
Source/WTF:
Removed resetCurrentAtomicStringTable() which is no longer referenced.
* wtf/WTFThreadData.h:
(WTF::WTFThreadData::resetCurrentAtomicStringTable): Deleted.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/API/tests/testapi.mm (205894 => 205895)
--- trunk/Source/_javascript_Core/API/tests/testapi.mm 2016-09-14 02:22:35 UTC (rev 205894)
+++ trunk/Source/_javascript_Core/API/tests/testapi.mm 2016-09-14 02:29:22 UTC (rev 205895)
@@ -552,6 +552,15 @@
}
@autoreleasepool {
+ JSVirtualMachine* vm = [[JSVirtualMachine alloc] init];
+ JSContext* context = [[JSContext alloc] initWithVirtualMachine:vm];
+ TestObject* testObject = [TestObject testObject];
+ context[@"testObject"] = testObject;
+ [context evaluateScript:@"result = 0; callbackResult = 0; Promise.resolve(42).then(function (value) { result = value; }); callbackResult = testObject.getString();"];
+ checkResult(@"Microtask is drained with same VM", [context[@"result"] isEqualToObject:@42] && [context[@"callbackResult"] isEqualToObject:@"42"]);
+ }
+
+ @autoreleasepool {
JSContext *context = [[JSContext alloc] init];
JSValue *result = [context evaluateScript:@"({ x:42 })"];
checkResult(@"({ x:42 })", result.isObject && [result[@"x"] isEqualToObject:@42]);
Modified: trunk/Source/_javascript_Core/ChangeLog (205894 => 205895)
--- trunk/Source/_javascript_Core/ChangeLog 2016-09-14 02:22:35 UTC (rev 205894)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-09-14 02:29:22 UTC (rev 205895)
@@ -1,3 +1,30 @@
+2016-09-13 Michael Saboff <[email protected]>
+
+ Promises aren't resolved properly when making a ObjC API callback
+ https://bugs.webkit.org/show_bug.cgi?id=161929
+
+ Reviewed by Geoffrey Garen.
+
+ When we go to call out to an Objective C function registered via the API,
+ we first drop all JSC locks to make the call. As part of dropping the locks,
+ we drain the microtask queue that is used among other things for handling deferred
+ promise resolution. The DropAllLocks scope class that drops the locks while in
+ scope, resets the current thread's AtomicStringTable to the default table. This
+ is wrong for two reasons, first it happens before we drain the microtask queue and
+ second it isn't needed as JSLock::willReleaseLock() restores the current thread's
+ AtomicStringTable to the table before the lock was acquired.
+
+ In fact, the manipulation of the current thread's AtomicStringTable is already
+ properly handled as a stack in JSLock::didAcquireLock() and willReleaseLock().
+ Therefore the manipulation of the AtomicStringTable in DropAllLocks constructor
+ and destructor should be removed.
+
+ * API/tests/testapi.mm:
+ (testObjectiveCAPIMain): Added a new test.
+ * runtime/JSLock.cpp:
+ (JSC::JSLock::DropAllLocks::DropAllLocks):
+ (JSC::JSLock::DropAllLocks::~DropAllLocks):
+
2016-09-13 Filip Pizlo <[email protected]>
Remove Heap::isLive()
Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (205894 => 205895)
--- trunk/Source/_javascript_Core/runtime/JSLock.cpp 2016-09-14 02:22:35 UTC (rev 205894)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp 2016-09-14 02:29:22 UTC (rev 205895)
@@ -267,7 +267,6 @@
{
if (!m_vm)
return;
- wtfThreadData().resetCurrentAtomicStringTable();
RELEASE_ASSERT(!m_vm->apiLock().currentThreadIsHoldingLock() || !m_vm->isCollectorBusy());
m_droppedLockCount = m_vm->apiLock().dropAllLocks(this);
}
@@ -287,7 +286,6 @@
if (!m_vm)
return;
m_vm->apiLock().grabAllLocks(this, m_droppedLockCount);
- wtfThreadData().setCurrentAtomicStringTable(m_vm->atomicStringTable());
}
} // namespace JSC
Modified: trunk/Source/WTF/ChangeLog (205894 => 205895)
--- trunk/Source/WTF/ChangeLog 2016-09-14 02:22:35 UTC (rev 205894)
+++ trunk/Source/WTF/ChangeLog 2016-09-14 02:29:22 UTC (rev 205895)
@@ -1,3 +1,15 @@
+2016-09-13 Michael Saboff <[email protected]>
+
+ Promises aren't resolved properly when making a ObjC API callback
+ https://bugs.webkit.org/show_bug.cgi?id=161929
+
+ Reviewed by Geoffrey Garen.
+
+ Removed resetCurrentAtomicStringTable() which is no longer referenced.
+
+ * wtf/WTFThreadData.h:
+ (WTF::WTFThreadData::resetCurrentAtomicStringTable): Deleted.
+
2016-09-13 Alex Christensen <[email protected]>
Implement URLSearchParams
Modified: trunk/Source/WTF/wtf/WTFThreadData.h (205894 => 205895)
--- trunk/Source/WTF/wtf/WTFThreadData.h 2016-09-14 02:22:35 UTC (rev 205894)
+++ trunk/Source/WTF/wtf/WTFThreadData.h 2016-09-14 02:29:22 UTC (rev 205895)
@@ -71,11 +71,6 @@
return oldAtomicStringTable;
}
- void resetCurrentAtomicStringTable()
- {
- m_currentAtomicStringTable = m_defaultAtomicStringTable;
- }
-
const StackBounds& stack()
{
// We need to always get a fresh StackBounds from the OS due to how fibers work.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
