Bug 170655 – [JSC] Enable JSRunLoopTimer for JSCOnly and Windows

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Enable JSRunLoopTimer for JSCOnly and Windows

[Yusuke Suzuki]

Created attachment 306622 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 306622 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=306622&action=review

> Source/JavaScriptCore/jsc.cpp:3891
>      WTF::initializeMainThread();
>      JSC::initializeThreading();
> +    RunLoop::initializeMainRunLoop();

Do we really need this? JS timers use RunLoop::current() not main(). If we need it, then we can remove WTF::initializeMainThread(); because initializeMainRunLoop already initializes the main thread.

> Source/JavaScriptCore/runtime/JSRunLoopTimer.cpp:190
> +    m_timer.stop();
> +    m_timer.startOneShot(s_decade);

RunLoop::Timer::start() already calls stop().

> Source/JavaScriptCore/runtime/JSRunLoopTimer.h:85
> +    static const Seconds s_decade;
> +    RunLoop::Timer<JSRunLoopTimer> m_timer;

Yes, I think we could adopt this implementation at least for non cocoa based ports. I think when we added support for heap timer, RunLoop was still in WebCore.

[Yusuke Suzuki]

Comment on attachment 306622 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=306622&action=review

Thanks, I'll update the patch.

>> Source/JavaScriptCore/jsc.cpp:3891
>> +    RunLoop::initializeMainRunLoop();
> 
> Do we really need this? JS timers use RunLoop::current() not main(). If we need it, then we can remove WTF::initializeMainThread(); because initializeMainRunLoop already initializes the main thread.

Right, it is not necessary. Dropped.

>> Source/JavaScriptCore/runtime/JSRunLoopTimer.cpp:190
>> +    m_timer.startOneShot(s_decade);
> 
> RunLoop::Timer::start() already calls stop().

Nice, dropped.

>> Source/JavaScriptCore/runtime/JSRunLoopTimer.h:85
>> +    RunLoop::Timer<JSRunLoopTimer> m_timer;
> 
> Yes, I think we could adopt this implementation at least for non cocoa based ports. I think when we added support for heap timer, RunLoop was still in WebCore.

Yeah, non cocoa based ports, I'll change to use this RunLoop::Timer in the subsequent patch.
This patch paves the way for that.

[Yusuke Suzuki]

Created attachment 306667 [details]
Patch

[Yusuke Suzuki]

Committed r215223: <http://trac.webkit.org/changeset/215223>