49096 2010-11-05 14:31:43 -0700 JSCallbackData::invokeCallback triggers layout from a worker thread 2010-11-05 15:48:13 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 47688 1 rniwa rniwa darin dimich jamesr levin oldest_to_newest 305532 0 rniwa 2010-11-05 14:31:43 -0700 We call Document::updateStyleForAllDocuments() in JSCallbackData::invokeCallback without checking that we're in the main thread. Stack trace: #0 0x10109fe3f in WebCore::Document::updateStyleForAllDocuments at Document.cpp:1564 #1 0x10148acb1 in WebCore::JSCallbackData::invokeCallback at JSCallbackData.cpp:73 #2 0x1015e87b6 in WebCore::JSSQLTransactionSyncCallback::handleEvent at JSSQLTransactionSyncCallback.cpp:72 #3 0x1019e5520 in WebCore::SQLTransactionSync::execute at SQLTransactionSync.cpp:153 #4 0x101062abb in WebCore::DatabaseSync::runTransaction at DatabaseSync.cpp:152 #5 0x101062b9d in WebCore::DatabaseSync::transaction at DatabaseSync.cpp:134 #6 0x1014bdcf7 in WebCore::jsDatabaseSyncPrototypeFunctionTransaction at JSDatabaseSync.cpp:182 #7 0x3f93858041b8 in ?? #8 0x1001b65bd in JSC::JITCode::execute at JITCode.h:77 #9 0x1001b2648 in JSC::Interpreter::execute at Interpreter.cpp:759 #10 0x100181fa6 in JSC::evaluate at Completion.cpp:62 #11 0x101b43026 in WebCore::WorkerScriptController::evaluate at WorkerScriptController.cpp:128 #12 0x101b4320c in WebCore::WorkerScriptController::evaluate at WorkerScriptController.cpp:107 #13 0x101b44a48 in WebCore::WorkerThread::workerThread at WorkerThread.cpp:134 #14 0x101b44b41 in WebCore::WorkerThread::workerThreadStart at WorkerThread.cpp:117 #15 0x1002c974d in WTF::threadEntryPoint at Threading.cpp:65 305571 1 73128 rniwa 2010-11-05 15:13:25 -0700 Created attachment 73128 fixes the bug 305573 2 73128 levin 2010-11-05 15:15:54 -0700 Comment on attachment 73128 fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=73128&action=review > WebCore/bindings/js/JSCallbackData.cpp:73 > + if (context->isDocument()) It would be nice to not call this virtual function nearly twice in a row. Consider storing the value in a bool and using in both places. 305574 3 rniwa 2010-11-05 15:16:40 -0700 Mn... somehow fast/workers/storage/use-same-database-in-page-and-workers.html always crash with the following error: ERROR: Unable to turn on incremental auto-vacuum for database /var/folders/++/++3qwE++6+0++4RjPqRgNE+-Rj2/-Tmp-/DumpRenderTree-tplq1u/Databases/file__0/000000000000001d.db (/Volumes/Data/webkit4/WebCore/storage/AbstractDatabase.cpp:251 virtual bool WebCore::AbstractDatabase::performOpenAndVerify(bool, WebCore::ExceptionCode&)) ASSERTION FAILED: !protectedObjectCount() (/Volumes/Data/webkit4/JavaScriptCore/runtime/Collector.cpp:262 void JSC::Heap::freeBlocks()) Should we fix this problem first? 305589 4 rniwa 2010-11-05 15:40:20 -0700 (In reply to comment #2) > (From update of attachment 73128 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=73128&action=review > > > WebCore/bindings/js/JSCallbackData.cpp:73 > > + if (context->isDocument()) > > It would be nice to not call this virtual function nearly twice in a row. Consider storing the value in a bool and using in both places. Will do and land: - JSValue result = context->isDocument() + bool contextIsDocument = context->isDocument(); + JSValue result = contextIsDocument ? JSMainThreadExecState::call(exec, function, callType, callData, callback(), args) : JSC::call(exec, function, callType, callData, callback(), args); globalObject()->globalData().timeoutChecker.stop(); - Document::updateStyleForAllDocuments(); + if (contextIsDocument) + Document::updateStyleForAllDocuments(); 305594 5 rniwa 2010-11-05 15:48:13 -0700 Committed r71455: <http://trac.webkit.org/changeset/71455> 73128 2010-11-05 15:13:25 -0700 2010-11-05 15:15:53 -0700 fixes the bug bug-49096-20101105151323.patch text/plain 2628 rniwa SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA3MTQ1MikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjIgQEAKKzIwMTAtMTEtMDUgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSlND YWxsYmFja0RhdGE6Omludm9rZUNhbGxiYWNrIHRyaWdnZXJzIGxheW91dCBmcm9tIGEgd29ya2Vy IHRocmVhZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NDkwOTYKKworICAgICAgICBGaXhlZCB0aGUgYnVnIGJ5IG9ubHkgY2FsbGluZyB1cGRhdGVTdHls ZUZvckFsbERvY3VtZW50cyB3aGVuIHRoZSBjb250ZXh0IGlzIGRvY3VtZW50IGluCisgICAgICAg IEpTQ2FsbGJhY2tEYXRhOjppbnZva2VDYWxsYmFjay4KKworICAgICAgICBObyBuZXcgdGVzdHMg YXJlIGFkZGVkIHNpbmNlIG1hbnkgb2YgZXhpc3Rpbmcgd29ya2VycyB0ZXN0cyBoaXQgdGhlIGFk ZGVkIGFzc2VydGlvbiBpbiBkZWJ1ZyBidWlsZHMuCisKKyAgICAgICAgKiBiaW5kaW5ncy9qcy9K U0NhbGxiYWNrRGF0YS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpKU0NhbGxiYWNrRGF0YTo6aW52 b2tlQ2FsbGJhY2spOiBObyBsb25nZXIgY2FsbHMgdXBkYXRlU3R5bGVGb3JBbGxEb2N1bWVudHMg dW5jb25kaXRpb25hbGx5LgorICAgICAgICAqIGRvbS9Eb2N1bWVudC5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpEb2N1bWVudDo6dXBkYXRlU3R5bGVJZk5lZWRlZCk6IEFkZGVkIEFTU0VSVChpc01h aW5UaHJlYWQoKSkuCisgICAgICAgIChXZWJDb3JlOjpEb2N1bWVudDo6dXBkYXRlU3R5bGVGb3JB bGxEb2N1bWVudHMpOiBEaXR0by4KKyAgICAgICAgKFdlYkNvcmU6OkRvY3VtZW50Ojp1cGRhdGVM YXlvdXQpOiBEaXR0by4KKwogMjAxMC0xMS0wNSAgSWx5YSBTaGVybWFuICA8aXNoZXJtYW5AY2hy b21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFNpbW9uIEZyYXNlci4KSW5kZXg6IFdl YkNvcmUvYmluZGluZ3MvanMvSlNDYWxsYmFja0RhdGEuY3BwCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNv cmUvYmluZGluZ3MvanMvSlNDYWxsYmFja0RhdGEuY3BwCShyZXZpc2lvbiA3MTQzMykKKysrIFdl YkNvcmUvYmluZGluZ3MvanMvSlNDYWxsYmFja0RhdGEuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC03 MCw3ICs3MCw4IEBAIEpTVmFsdWUgSlNDYWxsYmFja0RhdGE6Omludm9rZUNhbGxiYWNrKE0KICAg ICAgICAgOiBKU0M6OmNhbGwoZXhlYywgZnVuY3Rpb24sIGNhbGxUeXBlLCBjYWxsRGF0YSwgY2Fs bGJhY2soKSwgYXJncyk7CiAgICAgZ2xvYmFsT2JqZWN0KCktPmdsb2JhbERhdGEoKS50aW1lb3V0 Q2hlY2tlci5zdG9wKCk7CiAKLSAgICBEb2N1bWVudDo6dXBkYXRlU3R5bGVGb3JBbGxEb2N1bWVu dHMoKTsKKyAgICBpZiAoY29udGV4dC0+aXNEb2N1bWVudCgpKQorICAgICAgICBEb2N1bWVudDo6 dXBkYXRlU3R5bGVGb3JBbGxEb2N1bWVudHMoKTsKIAogICAgIGlmIChleGVjLT5oYWRFeGNlcHRp b24oKSkgewogICAgICAgICByZXBvcnRDdXJyZW50RXhjZXB0aW9uKGV4ZWMpOwpJbmRleDogV2Vi Q29yZS9kb20vRG9jdW1lbnQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvZG9tL0RvY3VtZW50 LmNwcAkocmV2aXNpb24gNzE0MzMpCisrKyBXZWJDb3JlL2RvbS9Eb2N1bWVudC5jcHAJKHdvcmtp bmcgY29weSkKQEAgLTE1NDEsNiArMTU0MSw3IEBAIGJhaWxfb3V0OgogCiB2b2lkIERvY3VtZW50 Ojp1cGRhdGVTdHlsZUlmTmVlZGVkKCkKIHsKKyAgICBBU1NFUlQoaXNNYWluVGhyZWFkKCkpOwog ICAgIEFTU0VSVCghdmlldygpIHx8ICghdmlldygpLT5pc0luTGF5b3V0KCkgJiYgIXZpZXcoKS0+ aXNQYWludGluZygpKSk7CiAgICAgCiAgICAgaWYgKCghbV9wZW5kaW5nU3R5bGVSZWNhbGNTaG91 bGRGb3JjZSAmJiAhY2hpbGROZWVkc1N0eWxlUmVjYWxjKCkpIHx8IGluUGFnZUNhY2hlKCkpCkBA IC0xNTU4LDYgKzE1NTksNyBAQCB2b2lkIERvY3VtZW50Ojp1cGRhdGVTdHlsZUlmTmVlZGVkKCkK IAogdm9pZCBEb2N1bWVudDo6dXBkYXRlU3R5bGVGb3JBbGxEb2N1bWVudHMoKQogeworICAgIEFT U0VSVChpc01haW5UaHJlYWQoKSk7CiAgICAgaWYgKCFkb2N1bWVudHNUaGF0TmVlZFN0eWxlUmVj YWxjKQogICAgICAgICByZXR1cm47CiAKQEAgLTE1NzEsNiArMTU3Myw3IEBAIHZvaWQgRG9jdW1l bnQ6OnVwZGF0ZVN0eWxlRm9yQWxsRG9jdW1lbnQKIAogdm9pZCBEb2N1bWVudDo6dXBkYXRlTGF5 b3V0KCkKIHsKKyAgICBBU1NFUlQoaXNNYWluVGhyZWFkKCkpOwogICAgIGlmIChFbGVtZW50KiBv ZSA9IG93bmVyRWxlbWVudCgpKQogICAgICAgICBvZS0+ZG9jdW1lbnQoKS0+dXBkYXRlTGF5b3V0 KCk7CiAK