Bug 55941 – REGRESSION (r80478): broke GTK inspector tests

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Philippe Normand]

Tests crashing:

inspector/console/console-api-on-call-frame.html
inspector/debugger/debug-inlined-scripts.html
inspector/debugger/debugger-cyclic-ref.html
inspector/debugger/debugger-eval-on-call-frame.html
inspector/debugger/debugger-eval-while-paused.html
inspector/debugger/debugger-expand-scope.html
inspector/debugger/debugger-no-nested-pause.html
inspector/debugger/debugger-pause-in-eval-script.html
inspector/debugger/debugger-pause-on-breakpoint.html
inspector/debugger/debugger-pause-on-debugger-statement.html
inspector/debugger/debugger-pause-on-exception.html
inspector/debugger/debugger-proto-property.html	
inspector/debugger/debugger-step-in.html
inspector/debugger/debugger-step-over.html
inspector/debugger/debugger-suspend-active-dom-objects.html
inspector/extensions/extensions-resources.html

back-trace:

Thread 1 (Thread 21305):
#0  0x00007f531a42237e in WebCore::SuspendableTimer::suspend (this=0x7f5308000970) at ../../Source/WebCore/page/SuspendableTimer.cpp:62
#1  0x00007f531a09cfd2 in WebCore::ScriptExecutionContext::suspendActiveDOMObjects (this=0x7f530802fa78, why=WebCore::ActiveDOMObject::JavaScriptDebuggerPaused) at ../../Source/WebCore/dom/ScriptExecutionContext.cpp:247
#2  0x00007f5319e94208 in WebCore::ScriptDebugServer::setJavaScriptPaused (this=0x2825220, frame=0x221d800, paused=true) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:413
#3  0x00007f5319e94100 in WebCore::ScriptDebugServer::setJavaScriptPaused (this=0x2825220, page=0x2212220, paused=true) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:398
#4  0x00007f5319e94027 in WebCore::ScriptDebugServer::setJavaScriptPaused (this=0x2825220, pageGroup=..., paused=true) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:388
#5  0x00007f5319e9470a in WebCore::ScriptDebugServer::pauseIfNeeded (this=0x2825220, page=0x2212220) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:478
#6  0x00007f5319e94571 in WebCore::ScriptDebugServer::updateCallFrameAndPauseIfNeeded (this=0x2825220, debuggerCallFrame=..., sourceID=52220736, lineNumber=3) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:454
#7  0x00007f5319e94b48 in WebCore::ScriptDebugServer::didReachBreakpoint (this=0x2825220, debuggerCallFrame=..., sourceID=52220736, lineNumber=3) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:560
#8  0x00007f531aba1518 in JSC::Interpreter::debug (this=0x2846d50, callFrame=0x7f530c75b038, debugHookID=JSC::DidReachBreakpoint, firstLine=3, lastLine=3) at ../../Source/JavaScriptCore/interpreter/Interpreter.cpp:1186
#9  0x00007f531abdb66d in JSC::cti_op_debug (args=0x7fff0c9d7990) at ../../Source/JavaScriptCore/jit/JITStubs.cpp:3458
#10 0x00007f531abcf86b in JSC::JITThunks::tryCacheGetByID (callFrame=0x7fff0c9d7990, codeBlock=0x0, returnAddress=..., baseValue=..., propertyName=..., slot=..., stubInfo=0x2821500) at ../../Source/JavaScriptCore/jit/JITStubs.cpp:869
#11 0x00007f531aba284b in JSC::JITCode::execute (this=0x2f60b58, registerFile=0x2846d68, callFrame=0x7f530c75b038, globalData=0x2821500) at ../../Source/JavaScriptCore/jit/JITCode.h:77
#12 0x00007f531ab9f91b in JSC::Interpreter::executeCall (this=0x2846d50, callFrame=0x28c4dd8, function=0x7f530c298490, callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/JavaScriptCore/interpreter/Interpreter.cpp:844
#13 0x00007f531ac2ca60 in JSC::call (exec=0x28c4dd8, functionObject=..., callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/JavaScriptCore/runtime/CallData.cpp:38
#14 0x00007f5319e2488f in WebCore::JSMainThreadExecState::call (exec=0x28c4dd8, functionObject=..., callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/WebCore/bindings/js/JSMainThreadExecState.h:48
#15 0x00007f5319e84c92 in WebCore::ScheduledAction::executeFunctionInContext (this=0x2969120, globalObject=0x7f531c748c78, thisValue=..., context=0x7f530802fa78) at ../../Source/WebCore/bindings/js/ScheduledAction.cpp:106
#16 0x00007f5319e84e84 in WebCore::ScheduledAction::execute (this=0x2969120, document=0x7f530802fa10) at ../../Source/WebCore/bindings/js/ScheduledAction.cpp:128
#17 0x00007f5319e84a4e in WebCore::ScheduledAction::execute (this=0x2969120, context=0x7f530802fa78) at ../../Source/WebCore/bindings/js/ScheduledAction.cpp:76
#18 0x00007f531a3b3299 in WebCore::DOMTimer::fired (this=0x306d470) at ../../Source/WebCore/page/DOMTimer.cpp:130
#19 0x00007f531a4da8e0 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x2246310) at ../../Source/WebCore/platform/ThreadTimers.cpp:112
#20 0x00007f531a4da817 in WebCore::ThreadTimers::sharedTimerFired () at ../../Source/WebCore/platform/ThreadTimers.cpp:90
#21 0x00007f5319cbc1c6 in WebCore::timeout_cb () at ../../Source/WebCore/platform/gtk/SharedTimerGtk.cpp:49
#22 0x00007f5316ec0dbb in g_timeout_dispatch (source=0x7f530822c140, callback=0, user_data=0x7f5315d36e00) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3877
#23 0x00007f5316ec0362 in g_main_dispatch (context=0xffff000000000002) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:2440
#24 g_main_context_dispatch (context=0xffff000000000002) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3013
#25 0x00007f5316ec4a28 in g_main_context_iterate (context=0x219e760, block=<value optimized out>, dispatch=<value optimized out>, self=<value optimized out>) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3091
#26 0x00007f5316ec4f35 in g_main_loop_run (loop=0x7f5308013e20) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3299
#27 0x00007f5318df9657 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#28 0x000000000041e525 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:679
#29 0x000000000041dbb7 in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:489
#30 0x000000000041fc9c in main (argc=2, argv=0x7fff0c9d8ba8) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1143

It seems we try to suspend an already-suspended timer. I'm not sure why that ASSERT is needed, really. The SuspendableTimer class should probably have a ::isSuspended() method or some other API so that upfront code can know if it's worth suspending the timer or not.

For now I will skip those tests on GTK as it's the only port hitting that ASSERT currently. I considered rolling out the patch, this could be an option too but I'd prefer advise on the situation before (CC'ed the people involved in r80478).

[Darin Adler]

(In reply to comment #0)
> It seems we try to suspend an already-suspended timer. I'm not sure why that ASSERT is needed, really.

If two different clients try to suspend and then resume a timer, and the suspend/resume mechanism is not based on a count, it’s likely the timer gets resumed too early, when one of the clients still wants the timer suspended. Presumably the assert helps us notice mistakes like that.

[Yong Li]

	4770	    asyncScriptRunner()->resume(); 
 	4771	    resumeActiveDOMObjects(); 
 	4772	    resumeScriptedAnimationControllerCallbacks(); 

I moved ln4770 to there to be exactly the reverse order of the one their "suspend" brothers are called.

This could break inspector if asyncScriptRunner()->resume() immediately executes JS. (that is not a good idea if so).

[Philippe Normand]

Created attachment 85069 [details]
proposed patch

[Pavel Feldman]

Comment on attachment 85069 [details]
proposed patch

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

> Source/WebCore/bindings/js/ScriptDebugServer.cpp:-478
> -    setJavaScriptPaused(page->group(), true);

setJavaScriptPaused was also pausing PluginViews.

> Source/WebCore/bindings/js/ScriptDebugServer.cpp:488
> +    }

PageGroupLoadDeferrer is going to restore the list of deferred frames (m_deferredFrames). Some of them might be dead by now since we are dispatching event loop while on a breakpoint and allow various page interactions by means of console.

[Alexey Proskuryakov]

*** Bug 56014 has been marked as a duplicate of this bug. ***

[Pavel Feldman]

Rolled out r80478 as r80718

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	D	Source/WebCore/manual-tests/database-callback-deferred.html
	M	LayoutTests/ChangeLog
	M	LayoutTests/platform/gtk/Skipped
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/dom/Document.cpp
	M	Source/WebCore/dom/Document.h
	M	Source/WebCore/loader/FrameLoader.cpp
	M	Source/WebCore/page/PageGroupLoadDeferrer.cpp
Committed r80718