- Revision
- 218381
- Author
- mark....@apple.com
- Date
- 2017-06-15 21:38:35 -0700 (Thu, 15 Jun 2017)
Log Message
Add a JSRunLoopTimer registry in VM.
https://bugs.webkit.org/show_bug.cgi?id=173429
<rdar://problem/31287961>
Reviewed by Filip Pizlo.
Source/_javascript_Core:
This way, we can be sure we've got every JSRunLoopTimer instance covered if we
need to change their run loop (e.g. when setting to the WebThread's run loop).
* heap/Heap.cpp:
(JSC::Heap::Heap):
(JSC::Heap::setRunLoop): Deleted.
* heap/Heap.h:
(JSC::Heap::runLoop): Deleted.
* runtime/JSRunLoopTimer.cpp:
(JSC::JSRunLoopTimer::JSRunLoopTimer):
(JSC::JSRunLoopTimer::setRunLoop):
(JSC::JSRunLoopTimer::~JSRunLoopTimer):
* runtime/VM.cpp:
(JSC::VM::VM):
(JSC::VM::registerRunLoopTimer):
(JSC::VM::unregisterRunLoopTimer):
(JSC::VM::setRunLoop):
* runtime/VM.h:
(JSC::VM::runLoop):
Source/WebCore:
No new tests needed because:
1. it's already covered: it was also originally discovered by our API tests while
running on the iOS simulator. The test was intermittently failing on a debug
build.
2. the issue is racy (it depends on a JSRunLoopTimer firing at the right time).
Hence, it's non trivial to write a better test than the one we already have.
* bindings/js/CommonVM.cpp:
(WebCore::commonVMSlow):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (218380 => 218381)
--- trunk/Source/_javascript_Core/ChangeLog 2017-06-16 04:06:04 UTC (rev 218380)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-06-16 04:38:35 UTC (rev 218381)
@@ -1,3 +1,31 @@
+2017-06-15 Mark Lam <mark....@apple.com>
+
+ Add a JSRunLoopTimer registry in VM.
+ https://bugs.webkit.org/show_bug.cgi?id=173429
+ <rdar://problem/31287961>
+
+ Reviewed by Filip Pizlo.
+
+ This way, we can be sure we've got every JSRunLoopTimer instance covered if we
+ need to change their run loop (e.g. when setting to the WebThread's run loop).
+
+ * heap/Heap.cpp:
+ (JSC::Heap::Heap):
+ (JSC::Heap::setRunLoop): Deleted.
+ * heap/Heap.h:
+ (JSC::Heap::runLoop): Deleted.
+ * runtime/JSRunLoopTimer.cpp:
+ (JSC::JSRunLoopTimer::JSRunLoopTimer):
+ (JSC::JSRunLoopTimer::setRunLoop):
+ (JSC::JSRunLoopTimer::~JSRunLoopTimer):
+ * runtime/VM.cpp:
+ (JSC::VM::VM):
+ (JSC::VM::registerRunLoopTimer):
+ (JSC::VM::unregisterRunLoopTimer):
+ (JSC::VM::setRunLoop):
+ * runtime/VM.h:
+ (JSC::VM::runLoop):
+
2017-06-15 Joseph Pecoraro <pecor...@apple.com>
[Cocoa] Modernize some internal initializers to use instancetype instead of id
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (218380 => 218381)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2017-06-16 04:06:04 UTC (rev 218380)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2017-06-16 04:38:35 UTC (rev 218381)
@@ -289,9 +289,6 @@
// schedule the timer if we've never done a collection.
, m_lastFullGCLength(0.01)
, m_lastEdenGCLength(0.01)
-#if USE(CF)
- , m_runLoop(CFRunLoopGetCurrent())
-#endif // USE(CF)
, m_fullActivityCallback(GCActivityCallback::createFullTimer(this))
, m_edenActivityCallback(GCActivityCallback::createEdenTimer(this))
, m_sweeper(adoptRef(new IncrementalSweeper(this)))
@@ -2589,16 +2586,6 @@
#endif
}
-#if USE(CF)
-void Heap::setRunLoop(CFRunLoopRef runLoop)
-{
- m_runLoop = runLoop;
- m_fullActivityCallback->setRunLoop(runLoop);
- m_edenActivityCallback->setRunLoop(runLoop);
- m_sweeper->setRunLoop(runLoop);
-}
-#endif // USE(CF)
-
void Heap::addCoreConstraints()
{
m_constraintSet->add(
Modified: trunk/Source/_javascript_Core/heap/Heap.h (218380 => 218381)
--- trunk/Source/_javascript_Core/heap/Heap.h 2017-06-16 04:06:04 UTC (rev 218380)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2017-06-16 04:38:35 UTC (rev 218381)
@@ -369,11 +369,6 @@
size_t numOpaqueRoots() const { return m_opaqueRoots.size(); }
-#if USE(CF)
- CFRunLoopRef runLoop() const { return m_runLoop.get(); }
- JS_EXPORT_PRIVATE void setRunLoop(CFRunLoopRef);
-#endif // USE(CF)
-
HeapVerifier* verifier() const { return m_verifier.get(); }
void addHeapFinalizerCallback(const HeapFinalizerCallback&);
@@ -623,9 +618,6 @@
Vector<WeakBlock*> m_logicallyEmptyWeakBlocks;
size_t m_indexOfNextLogicallyEmptyWeakBlockToSweep { WTF::notFound };
-#if USE(CF)
- RetainPtr<CFRunLoopRef> m_runLoop;
-#endif // USE(CF)
RefPtr<FullGCActivityCallback> m_fullActivityCallback;
RefPtr<GCActivityCallback> m_edenActivityCallback;
RefPtr<IncrementalSweeper> m_sweeper;
Modified: trunk/Source/_javascript_Core/runtime/JSRunLoopTimer.cpp (218380 => 218381)
--- trunk/Source/_javascript_Core/runtime/JSRunLoopTimer.cpp 2017-06-16 04:06:04 UTC (rev 218380)
+++ trunk/Source/_javascript_Core/runtime/JSRunLoopTimer.cpp 2017-06-16 04:38:35 UTC (rev 218381)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -69,7 +69,7 @@
: m_vm(vm)
, m_apiLock(&vm->apiLock())
{
- setRunLoop(vm->heap.runLoop());
+ m_vm->registerRunLoopTimer(this);
}
void JSRunLoopTimer::setRunLoop(CFRunLoopRef runLoop)
@@ -81,8 +81,8 @@
m_timer.clear();
}
+ m_runLoop = runLoop;
if (runLoop) {
- m_runLoop = runLoop;
memset(&m_context, 0, sizeof(CFRunLoopTimerContext));
m_context.info = this;
m_timer = adoptCF(CFRunLoopTimerCreate(kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + s_decade.seconds(), s_decade.seconds(), 0, 0, JSRunLoopTimer::timerDidFireCallback, &m_context));
@@ -92,7 +92,7 @@
JSRunLoopTimer::~JSRunLoopTimer()
{
- setRunLoop(0);
+ m_vm->unregisterRunLoopTimer(this);
}
void JSRunLoopTimer::timerDidFireCallback(CFRunLoopTimerRef, void* contextPtr)
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (218380 => 218381)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2017-06-16 04:06:04 UTC (rev 218380)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2017-06-16 04:38:35 UTC (rev 218381)
@@ -156,6 +156,9 @@
VM::VM(VMType vmType, HeapType heapType)
: m_apiLock(adoptRef(new JSLock(this)))
+#if USE(CF)
+ , m_runLoop(CFRunLoopGetCurrent())
+#endif // USE(CF)
, heap(this, heapType)
, auxiliarySpace("Auxiliary", heap, AllocatorAttributes(DoesNotNeedDestruction, HeapCell::Auxiliary))
, cellSpace("JSCell", heap, AllocatorAttributes(DoesNotNeedDestruction, HeapCell::JSCell))
@@ -931,4 +934,29 @@
}
#endif // ENABLE(JIT)
+#if USE(CF)
+void VM::registerRunLoopTimer(JSRunLoopTimer* timer)
+{
+ ASSERT(runLoop());
+ ASSERT(!m_runLoopTimers.contains(timer));
+ m_runLoopTimers.add(timer);
+ timer->setRunLoop(runLoop());
+}
+
+void VM::unregisterRunLoopTimer(JSRunLoopTimer* timer)
+{
+ ASSERT(m_runLoopTimers.contains(timer));
+ m_runLoopTimers.remove(timer);
+ timer->setRunLoop(nullptr);
+}
+
+void VM::setRunLoop(CFRunLoopRef runLoop)
+{
+ ASSERT(runLoop);
+ m_runLoop = runLoop;
+ for (auto timer : m_runLoopTimers)
+ timer->setRunLoop(runLoop);
+}
+#endif // USE(CF)
+
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/VM.h (218380 => 218381)
--- trunk/Source/_javascript_Core/runtime/VM.h 2017-06-16 04:06:04 UTC (rev 218380)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2017-06-16 04:38:35 UTC (rev 218381)
@@ -109,6 +109,7 @@
class JSCustomGetterSetterFunction;
class JSGlobalObject;
class JSObject;
+class JSRunLoopTimer;
class JSWebAssemblyInstance;
class LLIntOffsetsExtractor;
class NativeExecutable;
@@ -279,6 +280,11 @@
private:
RefPtr<JSLock> m_apiLock;
+#if USE(CF)
+ // These need to be initialized before heap below.
+ HashSet<JSRunLoopTimer*> m_runLoopTimers;
+ RetainPtr<CFRunLoopRef> m_runLoop;
+#endif
public:
Heap heap;
@@ -669,6 +675,13 @@
ThreadIdentifier throwingThread() const { return m_throwingThread; }
#endif
+#if USE(CF)
+ CFRunLoopRef runLoop() const { return m_runLoop.get(); }
+ void registerRunLoopTimer(JSRunLoopTimer*);
+ void unregisterRunLoopTimer(JSRunLoopTimer*);
+ JS_EXPORT_PRIVATE void setRunLoop(CFRunLoopRef);
+#endif // USE(CF)
+
private:
friend class LLIntOffsetsExtractor;
Modified: trunk/Source/WebCore/ChangeLog (218380 => 218381)
--- trunk/Source/WebCore/ChangeLog 2017-06-16 04:06:04 UTC (rev 218380)
+++ trunk/Source/WebCore/ChangeLog 2017-06-16 04:38:35 UTC (rev 218381)
@@ -1,3 +1,21 @@
+2017-06-15 Mark Lam <mark....@apple.com>
+
+ Add a JSRunLoopTimer registry in VM.
+ https://bugs.webkit.org/show_bug.cgi?id=173429
+ <rdar://problem/31287961>
+
+ Reviewed by Filip Pizlo.
+
+ No new tests needed because:
+ 1. it's already covered: it was also originally discovered by our API tests while
+ running on the iOS simulator. The test was intermittently failing on a debug
+ build.
+ 2. the issue is racy (it depends on a JSRunLoopTimer firing at the right time).
+ Hence, it's non trivial to write a better test than the one we already have.
+
+ * bindings/js/CommonVM.cpp:
+ (WebCore::commonVMSlow):
+
2017-06-15 Antoine Quint <grao...@apple.com>
REGRESSION: AirPlay button is incorrectly highlighted in inline and fullscreen
Modified: trunk/Source/WebCore/bindings/js/CommonVM.cpp (218380 => 218381)
--- trunk/Source/WebCore/bindings/js/CommonVM.cpp 2017-06-16 04:06:04 UTC (rev 218380)
+++ trunk/Source/WebCore/bindings/js/CommonVM.cpp 2017-06-16 04:38:35 UTC (rev 218381)
@@ -54,7 +54,7 @@
g_commonVMOrNull = &VM::createLeaked(LargeHeap).leakRef();
g_commonVMOrNull->heap.acquireAccess(); // At any time, we may do things that affect the GC.
#if PLATFORM(IOS)
- g_commonVMOrNull->heap.setRunLoop(WebThreadRunLoop());
+ g_commonVMOrNull->setRunLoop(WebThreadRunLoop());
g_commonVMOrNull->heap.machineThreads().addCurrentThread();
#endif
