Bug 55941

Summary: REGRESSION (r80478): broke GTK inspector tests
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, mrobinson, pfeldman, rniwa, yong.li.webkit
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
proposed patch pfeldman: review-

Philippe Normand
Reported 2011-03-08 07:14:48 PST
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).
Attachments
proposed patch (5.65 KB, patch)
2011-03-08 11:44 PST, Philippe Normand 		pfeldman: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2011-03-08 08:26:28 PST
(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
Comment 2 2011-03-08 09:57:02 PST
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
Comment 3 2011-03-08 11:44:29 PST
Created attachment 85069 [details] proposed patch
Pavel Feldman
Comment 4 2011-03-09 08:17:35 PST
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
Comment 5 2011-03-09 14:57:38 PST
*** Bug 56014 has been marked as a duplicate of this bug. ***
Pavel Feldman
Comment 6 2011-03-10 07:19:04 PST
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
Note You need to log in before you can comment on or make changes to this bug.