Bug 49401

Created attachment 73654 [details] the patch

Comment on attachment 73654 [details] the patch test case should be rewritten

Attachment 73654 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebCore/ChangeLog', u'WebCore/dom/Document.cpp', u'WebCore/dom/Document.h', u'WebCore/manual-tests/database-callback-deferred.html', u'WebCore/page/PageGroupLoadDeferrer.cpp']" exit_code: 1 WebCore/dom/Document.cpp:592: Missing space before ( in for( [whitespace/parens] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 73657 [details] updated

Created attachment 73658 [details] fix style

Attachment 73657 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebCore/ChangeLog', u'WebCore/dom/Document.cpp', u'WebCore/dom/Document.h', u'WebCore/manual-tests/database-callback-deferred.html', u'WebCore/page/PageGroupLoadDeferrer.cpp']" exit_code: 1 WebCore/dom/Document.cpp:592: Missing space before ( in for( [whitespace/parens] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Attachment 73654 [details] did not build on mac: Build output: http://queues.webkit.org/results/5702008

Attachment 73658 [details] did not build on chromium: Build output: http://queues.webkit.org/results/5670006

Attachment 73658 [details] did not build on mac: Build output: http://queues.webkit.org/results/5771004

Created attachment 74385 [details] remove an unused variable in test case

Attachment 74385 [details] did not build on chromium: Build output: http://queues.webkit.org/results/6229071

Attachment 74385 [details] did not build on mac: Build output: http://queues.webkit.org/results/6197085

Attachment 74385 [details] did not build on mac: Build output: http://queues.webkit.org/results/6204053

Comment on attachment 74385 [details] remove an unused variable in test case Would break mac.

Created attachment 76389 [details] try again

Attachment 76389 [details] did not build on chromium: Build output: http://queues.webkit.org/results/6984090

Attachment 76389 [details] did not build on chromium: Build output: http://queues.webkit.org/results/6891121

Attachment 76389 [details] did not build on mac: Build output: http://queues.webkit.org/results/7042024

Created attachment 76407 [details] finally found the warning that fails build

Comment on attachment 76407 [details] finally found the warning that fails build View in context: https://bugs.webkit.org/attachment.cgi?id=76407&action=review > WebCore/dom/Document.cpp:600 > + for (size_t i = 0; i < m_pendingTasks.size(); ++i) > + delete m_pendingTasks[i]; There’s a function for this called deleteAllValues. But more importantly, Vector<OwnPtr> works just fine, so we should use it here rather than ll those leakPtr calls. > WebCore/dom/Document.cpp:4696 > -static void performTask(void* ctx) > +void Document::didReceiveTask(void* ctx) Lets use a word here instead of the “ctx” abbreviation. Maybe contextPtr? Yes, that uses an abbrevation too, so I may be a hypocrite. > WebCore/dom/Document.cpp:4700 > + OwnPtr<PerformTaskContext> context(reinterpret_cast<PerformTaskContext*>(ctx)); This should be using adoptPtr and should use static_cast rather than reinterpret_cast. > WebCore/dom/Document.cpp:4728 > + OwnPtr<Task> task(m_pendingTasks[0]); This should use adoptPtr, but would actually use release if it was a Vector<OwnPtr>. Since we use this by adding to the end and removing from the start, this should be a Deque rather than a Vector. If it was a Deque we could probably use the takeFirst function rather than writing it like this. > WebCore/dom/Document.cpp:4738 > + suspendActiveDOMObjects(ActiveDOMObject::WillShowDialog); > + asyncScriptRunner()->suspend(); > + m_pendingTasksTimer.stop(); Perhaps suspendActiveDOMObjects could be made private now? > WebCore/dom/Document.cpp:4746 > + resumeActiveDOMObjects(); Perhaps resumeActiveDOMObjects could be made private now? > WebCore/dom/Document.h:1067 > + virtual void willDeferLoading(); > + virtual void didResumeLoading(); Why are these functions virtual? It would be better if these names paired up a bit better. > WebCore/page/PageGroupLoadDeferrer.cpp:50 > - for (Frame* frame = otherPage->mainFrame(); frame; frame = frame->tree()->traverseNext()) { > - frame->document()->suspendActiveDOMObjects(ActiveDOMObject::WillShowDialog); > - frame->document()->asyncScriptRunner()->suspend(); > - } > + for (Frame* frame = otherPage->mainFrame(); frame; frame = frame->tree()->traverseNext()) > + frame->document()->willDeferLoading(); Each frame already gets a call about deferring loading in response to the Page::setDefersLoading call. Specifically, FrameLoader::setDefersLoading. Could we put the responsibility for calling these functions there? Is there some other call site that calls setDefersLoading but does not want this additional work to be done? I find the willDefer and didResume names a little confusing and this is existing code is strangely disconnected from the main setDefersLoading feature.

(In reply to comment #20) Many cool ideas. Working on another patch. suspendActiveDOMObjects cannot be private because it is from parent class: ScriptExecutionContext

Created attachment 76435 [details] updated cannot use Deque<OwnPtr<T> >

Attachment 76435 [details] did not build on chromium: Build output: http://queues.webkit.org/results/6993098

(In reply to comment #23) > Attachment 76435 [details] did not build on chromium: > Build output: http://queues.webkit.org/results/6993098 I don't think it is caused by this patch. Where can I find the detailed error message?

You can re-post the patch, we recently fixed a logging problem with the mac-based EWS bots. Hopefully they'll show errors better now.

Created attachment 76562 [details] rerun builtbot

Comment on attachment 76562 [details] rerun builtbot View in context: https://bugs.webkit.org/attachment.cgi?id=76562&action=review Change looks OK, but this is a super-tricky area; I’d prefer it if we had more tests. > WebCore/dom/Document.cpp:4693 > +void Document::didReceiveTask(void* rawContext) I used the name “untypedContext” for this when I recently wrote some code using the same idiom. Your name is good too. Just wanted to share mine. > WebCore/dom/Document.cpp:4710 > + document->m_pendingTasks.append(context->task.leakPtr()); The leakPtr here is not what you want. You want to call release here instead. This only compiles because of LOOSE_OWN_PTR. > WebCore/dom/Document.cpp:4728 > + while (!m_pendingTasks.isEmpty()) { > + OwnPtr<Task> task = m_pendingTasks[0].release(); > + m_pendingTasks.remove(0); > + task->performTask(this); > + } This function is an argument for using the Deque class. Deque has a takeFirst function that is perfect for idioms like this. > WebCore/loader/FrameLoader.cpp:260 > + // This code is not logically part of load deferring, but we do not want JS code executed beneath modal > + // windows or sheets, which is exactly when PageGroupLoadDeferrer is used. > + // NOTE: if PageGroupLoadDeferrer is ever used for tasks other than showing a modal window or sheet, It no longer makes sense to mention PageGroupLoadDeferrer here. This comment is now far enough away from PageGroupLoadDeferrer that people will not see the comment. The word “if” should be capitalized here at the start of a sentence. > WebCore/loader/FrameLoader.cpp:265 > + if (defers) > + m_frame->document()->suspendScheduledTasks(); > + else > + m_frame->document()->resumeScheduledTasks(); What’s the guarantee that we won’t change the document of a frame while suspended? If we don’t have that guarantee we could end up calling suspendScheduledTasks on a document and never calling resumeScheduledTasks on it.

(In reply to comment #27) > (From update of attachment 76562 [details]) > > WebCore/dom/Document.cpp:4710 > > + document->m_pendingTasks.append(context->task.leakPtr()); > The leakPtr here is not what you want. You want to call release here instead. This only compiles because of LOOSE_OWN_PTR. forgot that I'm using Vector<OwnPtr> now. > > WebCore/dom/Document.cpp:4728 > > + while (!m_pendingTasks.isEmpty()) { > > + OwnPtr<Task> task = m_pendingTasks[0].release(); > > + m_pendingTasks.remove(0); > > + task->performTask(this); > > + } > This function is an argument for using the Deque class. Deque has a takeFirst function that is perfect for idioms like this. I tried Deque<OwnPtr>, but it doesn't compile. Probably it is worth reporting another bug. > > WebCore/loader/FrameLoader.cpp:265 > > + if (defers) > > + m_frame->document()->suspendScheduledTasks(); > > + else > > + m_frame->document()->resumeScheduledTasks(); > What’s the guarantee that we won’t change the document of a frame while suspended? If we don’t have that guarantee we could end up calling suspendScheduledTasks on a document and never calling resumeScheduledTasks on it. From my understanding, when loading is deferred (for example a modal dialog is up), frame should never change its document except there is another bug, because if it can open new document, it must also be able to run JS, load resources, ...

Created attachment 77150 [details] updated

Comment on attachment 77150 [details] updated Rejecting attachment 77150 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--no-update', '--non-interactive', 77150]" exit_code: 2 Last 500 characters of output: with fuzz 2 (offset 5 lines). Hunk #2 succeeded at 1137 (offset -2 lines). Hunk #3 succeeded at 1388 (offset -3 lines). patching file WebCore/loader/FrameLoader.cpp patching file WebCore/manual-tests/database-callback-deferred.html patching file WebCore/page/PageGroupLoadDeferrer.cpp Hunk #1 FAILED at 40. 1 out of 1 hunk FAILED -- saving rejects to file WebCore/page/PageGroupLoadDeferrer.cpp.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/7303085

Created attachment 77919 [details] fix merge error

Created attachment 77920 [details] the right one

Comment on attachment 77920 [details] the right one Rejecting attachment 77920 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-sf-cq', 'apply-attachment', '--no-update', '--non-interactive', 77920]" exit_code: 2 Last 500 characters of output: bCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebCore/dom/Document.cpp patching file WebCore/dom/Document.h patching file WebCore/loader/FrameLoader.cpp patching file WebCore/manual-tests/database-callback-deferred.html patching file WebCore/page/PageGroupLoadDeferrer.cpp Hunk #1 FAILED at 40. 1 out of 1 hunk FAILED -- saving rejects to file WebCore/page/PageGroupLoadDeferrer.cpp.rej Failed to run "[u'/Projects/CommitQueue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/7266419

Created attachment 78032 [details] try again

Comment on attachment 78032 [details] try again Rejecting attachment 78032 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']" exit_code: 2 Last 500 characters of output: .................................... fast/text/whitespace .......................................... fast/tokenizer ........................................ fast/transforms ........................ fast/url .................... fast/workers ......................... fast/workers/worker-close-more.html -> failed Exiting early after 1 failures. 16401 tests run. 320.18s total testing time 16400 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 6 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/7200441

Comment on attachment 78032 [details] try again It's possible that simply flaked twice.

Comment on attachment 78032 [details] try again Rejecting attachment 78032 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']" exit_code: 2 Last 500 characters of output: .................................... fast/text/whitespace .......................................... fast/tokenizer ........................................ fast/transforms ........................ fast/url .................... fast/workers ......................... fast/workers/worker-close-more.html -> failed Exiting early after 1 failures. 16401 tests run. 339.88s total testing time 16400 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 7 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/7362002

Created attachment 78117 [details] ertry

(In reply to comment #36) > (From update of attachment 78032 [details]) > It's possible that simply flaked twice. seems it is a real issue. where can I find the actual result?

Created attachment 78129 [details] Change the expected result of worker-close-more test The only change is to add this line to the expected result of worker-close-more test: CONSOLE MESSAGE: line 31: JavaScript execution exceeded timeout This message is caused by the 31st line in worker-close.js: while (true) { } So I think it was just missed and now we can catch it. However I haven't been able to see the result diff on the bots... not sure if it is the same story.

Comment on attachment 76562 [details] rerun builtbot Cleared Darin Adler's review+ from obsolete attachment 76562 [details] so that this bug does not appear in http://webkit.org/pending-commit.

Comment on attachment 78129 [details] Change the expected result of worker-close-more test I'll update the patch because the files are moved to Source now

Created attachment 82647 [details] Based on latest code

Comment on attachment 82647 [details] Based on latest code still fail the test

Created attachment 82660 [details] Let commit log test the patch

Comment on attachment 82660 [details] Let commit log test the patch Darnn. still fail the test

Comment on attachment 82647 [details] Based on latest code Rejecting attachment 82647 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-..." exit_code: 2 Last 500 characters of output: t.h Hunk #1 succeeded at 1119 with fuzz 2 (offset 10 lines). Hunk #2 succeeded at 1168 (offset 10 lines). Hunk #3 FAILED at 1431. 1 out of 3 hunks FAILED -- saving rejects to file Source/WebCore/dom/Document.h.rej patching file Source/WebCore/loader/FrameLoader.cpp patching file Source/WebCore/manual-tests/database-callback-deferred.html patching file Source/WebCore/page/PageGroupLoadDeferrer.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/8084426

Created attachment 84630 [details] up-to-date

Created attachment 84950 [details] Try again

Comment on attachment 84950 [details] Try again Clearing flags on attachment: 84950 Committed r80478: <http://trac.webkit.org/changeset/80478>

All reviewed patches have been landed. Closing bug.

http://trac.webkit.org/changeset/80478 might have broken Qt Linux Release

(In reply to comment #52) > http://trac.webkit.org/changeset/80478 might have broken Qt Linux Release buildfix landed in http://trac.webkit.org/changeset/80480

It seems like 16 inspector tests have been crashing sine this patch is landed: http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/20099 http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r80480%20(20099)/results.html

Here's what I believe is the stack for these crashes: #0 0x00007fdb4d7c11b6 in WebCore::SuspendableTimer::suspend (this=0x7fdb3c003ab0) at ../../Source/WebCore/page/SuspendableTimer.cpp:62 62 ASSERT(!m_suspended); #1 0x00007fdb4d43be0a in WebCore::ScriptExecutionContext::suspendActiveDOMObjects (this=0x7fdb3c00a808, why=WebCore::ActiveDOMObject::JavaScriptDebuggerPaused) at ../../Source/WebCore/dom/ScriptExecutionContext.cpp:247 #2 0x00007fdb4d233040 in WebCore::ScriptDebugServer::setJavaScriptPaused (this=0x20d9660, frame=0x1ade000, paused=true) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:413 #3 0x00007fdb4d232f38 in WebCore::ScriptDebugServer::setJavaScriptPaused (this=0x20d9660, page=0x1ad1ac0, paused=true) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:398 #4 0x00007fdb4d232e5f in WebCore::ScriptDebugServer::setJavaScriptPaused (this=0x20d9660, pageGroup=..., paused=true) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:388 #5 0x00007fdb4d233542 in WebCore::ScriptDebugServer::pauseIfNeeded (this=0x20d9660, page=0x1ad1ac0) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:478 #6 0x00007fdb4d2333a9 in WebCore::ScriptDebugServer::updateCallFrameAndPauseIfNeeded (this=0x20d9660, debuggerCallFrame=..., sourceID=40408256, lineNumber=10) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:454 #7 0x00007fdb4d233980 in WebCore::ScriptDebugServer::didReachBreakpoint (this=0x20d9660, debuggerCallFrame=..., sourceID=40408256, lineNumber=10) at ../../Source/WebCore/bindings/js/ScriptDebugServer.cpp:560 #8 0x00007fdb4df40350 in JSC::Interpreter::debug (this=0x20d82d0, callFrame=0x7fdafb9fd038, debugHookID=JSC::DidReachBreakpoint, firstLine=10, lastLine=10) at ../../Source/JavaScriptCore/interpreter/Interpreter.cpp:1186 #9 0x00007fdb4df7a4a5 in JSC::cti_op_debug (args=0x7fff74a467d0) at ../../Source/JavaScriptCore/jit/JITStubs.cpp:3458 #10 0x00007fdb4df6e6a3 in JSC::JITThunks::tryCacheGetByID (callFrame=0x7fff74a467d0, codeBlock=0x7fff74a467d0, returnAddress=..., baseValue=..., propertyName=..., slot=..., stubInfo=0x1abbe00) at ../../Source/JavaScriptCore/jit/JITStubs.cpp:869 #11 0x00007fdb4df41683 in JSC::JITCode::execute (this=0x256a128, registerFile=0x20d82e8, callFrame=0x7fdafb9fd038, globalData=0x1abbe00) at ../../Source/JavaScriptCore/jit/JITCode.h:77 #12 0x00007fdb4df3e753 in JSC::Interpreter::executeCall (this=0x20d82d0, callFrame=0x21f8408, function=0x7fdb4fb48d90, callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/JavaScriptCore/interpreter/Interpreter.cpp:844 #13 0x00007fdb4dfcb898 in JSC::call (exec=0x21f8408, functionObject=..., callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/JavaScriptCore/runtime/CallData.cpp:38 #14 0x00007fdb4d1c36c7 in WebCore::JSMainThreadExecState::call (exec=0x21f8408, functionObject=..., callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/WebCore/bindings/js/JSMainThreadExecState.h:48 #15 0x00007fdb4d223aca in WebCore::ScheduledAction::executeFunctionInContext (this=0x2f2d050, globalObject=0x7fdafb178c78, thisValue=..., context=0x7fdb3c00a808) at ../../Source/WebCore/bindings/js/ScheduledAction.cpp:106 #16 0x00007fdb4d223cbc in WebCore::ScheduledAction::execute (this=0x2f2d050, document=0x7fdb3c00a7a0) at ../../Source/WebCore/bindings/js/ScheduledAction.cpp:128 #17 0x00007fdb4d223886 in WebCore::ScheduledAction::execute (this=0x2f2d050, context=0x7fdb3c00a808) at ../../Source/WebCore/bindings/js/ScheduledAction.cpp:76 #18 0x00007fdb4d7520d1 in WebCore::DOMTimer::fired (this=0x2ecc510) at ../../Source/WebCore/page/DOMTimer.cpp:130 #19 0x00007fdb4d879718 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x1b06c00) at ../../Source/WebCore/platform/ThreadTimers.cpp:112 #20 0x00007fdb4d87964f in WebCore::ThreadTimers::sharedTimerFired () at ../../Source/WebCore/platform/ThreadTimers.cpp:90 #21 0x00007fdb4d05b0fa in WebCore::timeout_cb () at ../../Source/WebCore/platform/gtk/SharedTimerGtk.cpp:49 #22 0x00007fdb4a25fdbb in g_timeout_dispatch (source=0x2712720, callback=0, user_data=0x7fdb490d5e00) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3877 #23 0x00007fdb4a25f362 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 0x00007fdb4a263a28 in g_main_context_iterate (context=0x1a52760, 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 0x00007fdb4a263f35 in g_main_loop_run (loop=0x7fdb3c001f40) at /tmp/buildd/glib2.0-2.27.91/./glib/gmain.c:3299 #27 0x00007fdb4c198657 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 #28 0x000000000041e485 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:679 #29 0x000000000041db17 in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:489 #30 0x000000000041fbfc in main (argc=2, argv=0x7fff74a479e8) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1143

Is someone looking at the inspector crashes caused by this patch?

This change broke inspector debugger tests on the Leopard Debug Tests bot. #0 WebCore::SuspendableTimer::suspend (this=0x1196c26a0) at Source/WebCore/page/SuspendableTimer.cpp:62 #1 0x0000000101b349cf in WebCore::ScriptExecutionContext::suspendActiveDOMObjects (this=0x1073cc668, why=WebCore::ActiveDOMObject::WillShowDialog) at Source/WebCore/dom/ScriptExecutionContext.cpp:247 #2 0x00000001011c13fc in WebCore::Document::suspendScheduledTasks (this=0x1073cc600) at Source/WebCore/dom/Document.cpp:4758 #3 0x00000001013785ae in WebCore::FrameLoader::setDefersLoading (this=0x10607e8b8, defers=true) at Source/WebCore/loader/FrameLoader.cpp:260 #4 0x00000001019187c1 in WebCore::Page::setDefersLoading (this=0x105e83e10, defers=true) at Source/WebCore/page/Page.cpp:549 #5 0x0000000101b276fc in WebCore::ScriptDebugServer::setJavaScriptPaused (this=0x105ed9920, page=0x105e83e10, paused=true) at Source/WebCore/bindings/js/ScriptDebugServer.cpp:395 #6 0x0000000101b277ad in WebCore::ScriptDebugServer::setJavaScriptPaused (this=0x105ed9920, pageGroup=@0x105e6b280, paused=true) at Source/WebCore/bindings/js/ScriptDebugServer.cpp:388 #7 0x0000000101b2835f in WebCore::ScriptDebugServer::pauseIfNeeded (this=0x105ed9920, page=0x105e83e10) at Source/WebCore/bindings/js/ScriptDebugServer.cpp:478 #8 0x0000000101b28501 in WebCore::ScriptDebugServer::updateCallFrameAndPauseIfNeeded (this=0x105ed9920, debuggerCallFrame=@0x7fff5fbfe0e0, sourceID=4407861248, lineNumber=13) at Source/WebCore/bindings/js/ScriptDebugServer.cpp:454 #9 0x0000000101b2854d in WebCore::ScriptDebugServer::didReachBreakpoint (this=0x105ed9920, debuggerCallFrame=@0x7fff5fbfe0e0, sourceID=4407861248, lineNumber=13) at Source/WebCore/bindings/js/ScriptDebugServer.cpp:560 #10 0x00000001001bfb20 in JSC::Interpreter::debug (this=0x106b2e9c0, callFrame=0x1186bc038, debugHookID=JSC::DidReachBreakpoint, firstLine=13, lastLine=13) at Interpreter.cpp:1186 #11 0x00000001001e82e7 in cti_op_debug (args=0x7fff5fbfe1c0) at JITStubs.cpp:3460 #12 0x00000001001e73ef in WTF::doubleHash (key=Could not find the frame base for "WTF::doubleHash(unsigned int)". ) at HashTable.h:447 #13 0x00000001001c58b1 in JSC::JITCode::execute (this=0x106fff9b8, registerFile=0x106b2e9d8, callFrame=0x1186bc038, globalData=0x107026000) at JITCode.h:77 #14 0x00000001001c055a in JSC::Interpreter::executeCall (this=0x106b2e9c0, callFrame=0x11d1967b8, function=0x118e8f180, callType=JSC::CallTypeJS, callData=@0x7fff5fbfe5e0, thisValue={m_ptr = 0x118e73440}, args=@0x7fff5fbfe5d0) at Interpreter.cpp:844 #15 0x000000010017b9b1 in JSC::call (exec=0x11d1967b8, functionObject={m_ptr = 0x118e8f180}, callType=JSC::CallTypeJS, callData=@0x7fff5fbfe5e0, thisValue={m_ptr = 0x118e73440}, args=@0x7fff5fbfe5d0) at CallData.cpp:38 #16 0x0000000101cfa451 in WebCore::JSMainThreadExecState::call (exec=0x11d1967b8, functionObject={m_ptr = 0x118e8f180}, callType=JSC::CallTypeJS, callData=@0x7fff5fbfe5e0, thisValue={m_ptr = 0x118e73440}, args=@0x7fff5fbfe5d0) at JSMainThreadExecState.h:48 #17 0x0000000101b18884 in WebCore::ScheduledAction::executeFunctionInContext (this=0x11a4723d0, globalObject=0x118ae65c8, thisValue={m_ptr = 0x118e73440}, context=0x1073cc668) at Source/WebCore/bindings/js/ScheduledAction.cpp:106 #18 0x0000000101b18dfc in WebCore::ScheduledAction::execute (this=0x11a4723d0, document=0x1073cc600) at Source/WebCore/bindings/js/ScheduledAction.cpp:128 #19 0x0000000101b18ed0 in WebCore::ScheduledAction::execute (this=0x11a4723d0, context=0x1073cc668) at Source/WebCore/bindings/js/ScheduledAction.cpp:76 #20 0x00000001012ac1d5 in WebCore::DOMTimer::fired (this=0x11d180c40) at Source/WebCore/page/DOMTimer.cpp:130 #21 0x0000000101cae3ea in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x105e6ac50) at Source/WebCore/platform/ThreadTimers.cpp:112 #22 0x0000000101cae605 in WebCore::ThreadTimers::sharedTimerFired () at Source/WebCore/platform/ThreadTimers.cpp:90 #23 0x0000000101b7fd08 in WebCore::timerFired () at Source/WebCore/platform/mac/SharedTimerMac.mm:166 #24 0x00007fff870c9678 in __CFRunLoopRun () #25 0x00007fff870c784f in CFRunLoopRunSpecific () #26 0x00007fff86e45a18 in -[NSRunLoop(NSRunLoop) runMode:beforeDate:] () #27 0x00000001000116d1 in runTest (testPathOrURL=@0x7fff5fbff2e0) at Tools/DumpRenderTree/mac/DumpRenderTree.mm:1127 #28 0x0000000100011bc8 in runTestingServerLoop () at Tools/DumpRenderTree/mac/DumpRenderTree.mm:615 #29 0x0000000100011fe2 in dumpRenderTree (argc=2, argv=0x7fff5fbffbf8) at Tools/DumpRenderTree/mac/DumpRenderTree.mm:671 #30 0x0000000100012204 in main (argc=2, argv=0x7fff5fbffbf8) at Tools/DumpRenderTree/mac/DumpRenderTree.mm:712 inspector/console/console-api-on-call-frame.html -> crashed ................... inspector/debugger . inspector/debugger/debug-inlined-scripts.html -> crashed ... inspector/debugger/debugger-cyclic-ref.html -> crashed . inspector/debugger/debugger-eval-on-call-frame.html -> crashed . inspector/debugger/debugger-eval-while-paused.html -> crashed . inspector/debugger/debugger-expand-scope.html -> crashed . inspector/debugger/debugger-no-nested-pause.html -> crashed . inspector/debugger/debugger-pause-in-eval-script.html -> crashed . inspector/debugger/debugger-pause-on-breakpoint.html -> crashed . inspector/debugger/debugger-pause-on-debugger-statement.html -> crashed . inspector/debugger/debugger-pause-on-exception.html -> crashed . inspector/debugger/debugger-proto-property.html -> crashed .. inspector/debugger/debugger-step-in.html -> crashed . inspector/debugger/debugger-step-out.html -> crashed . inspector/debugger/debugger-step-over.html -> crashed . inspector/debugger/debugger-suspend-active-dom-objects.html -> crashed ..

(In reply to comment #57) > This change broke inspector debugger tests on the Leopard Debug Tests bot. > Pavel, see patch in bug 55941. It fixes the inspector tests on GTK at least.

This checkin seems to have caused bug 56073

(In reply to comment #59) > This checkin seems to have caused bug 56073 Given inspector breakage and now engadget.com breakage, do we want to roll it out? If Leopard Debug bots were alive at the time of the commit, this would have been rolled out immediately. So if I hear no objections, I'll roll it out in couple of hours and reopen the bug.

Rolled out 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

These regressions also mean PageGroupLoadDeferrer is frequently used by WebKit2 / inspector, so the patch is really needed. Regressions should be fixed though

Now I got it. My patch moves the following lines to FrameLoader::setDefersLoading, then Page::setDefersLoading() runs them. If someone calls Page::setDefersLoading() directly, it will conflict with PageGroupLoadDeferrer. for (Frame* frame = otherPage->mainFrame(); frame; frame = frame->tree()->traverseNext()) { frame->document()->suspendScriptedAnimationControllerCallbacks(); frame->document()->suspendActiveDOMObjects(ActiveDOMObject::WillShowDialog); frame->document()->scriptRunner()->suspend(); if (DocumentParser* parser = frame->document()->parser()) parser->suspendScheduledTasks(); }

Created attachment 118230 [details] Remove the part that causes conflict with other callers of Page::setDefersLoading().

Comment on attachment 118230 [details] Remove the part that causes conflict with other callers of Page::setDefersLoading(). Attachment 118230 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10753211

Comment on attachment 118230 [details] Remove the part that causes conflict with other callers of Page::setDefersLoading(). Attachment 118230 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10767138

Created attachment 118241 [details] fix the && || warning

Comment on attachment 118241 [details] fix the && || warning Clearing flags on attachment: 118241 Committed r102278: <http://trac.webkit.org/changeset/102278>

All reviewed patches have been landed. Closing bug.

>If someone calls Page::setDefersLoading() directly, it will conflict with PageGroupLoadDeferrer. It's unfortunate that a bit of exploration wasn't made to demonstrate that Page::setDefersLoading() has been used directly in WebKit API/SPI since the beginning of the WebKit open source project. (See bug https://bugs.webkit.org/show_bug.cgi?id=84488)