49342 2010-11-10 13:49:11 -0800 [Qt] QWebScriptWorld::world() may crash 2010-11-11 06:51:07 -0800 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) PC All RESOLVED FIXED P2 Minor --- 0 max.hong.shen webkit-unassigned commit-queue kling oldest_to_newest 307529 0 max.hong.shen 2010-11-10 13:49:11 -0800 It might be trivial but we'd better check the value of the private data in the QWebScriptWorld::world() to prevent the possible crash -- 'd' may have an empty data if USE(JSC) is undefined for some platform. QWebScriptWorld::QWebScriptWorld() { #if USE(JSC) // d can have an empty data if this macro fails d = new QWebScriptWorldPrivate(ScriptController::createWorld()); #endif } 307532 1 73536 max.hong.shen 2010-11-10 13:52:18 -0800 Created attachment 73536 first try 307786 2 73536 kenneth 2010-11-11 01:34:23 -0800 Comment on attachment 73536 first try View in context: https://bugs.webkit.org/attachment.cgi?id=73536&action=review > WebKit/qt/Api/qwebscriptworld.cpp:53 > + return d.data() ? d->world.get() : NULL; huh? how does world depend on data() ? Also we do not use NULL in C++ code. 307803 3 73593 max.hong.shen 2010-11-11 03:07:43 -0800 Created attachment 73593 second try 307805 4 max.hong.shen 2010-11-11 03:10:32 -0800 (In reply to comment #2) > (From update of attachment 73536 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=73536&action=review > > > WebKit/qt/Api/qwebscriptworld.cpp:53 > > + return d.data() ? d->world.get() : NULL; > > huh? how does world depend on data() ? Also we do not use NULL in C++ code. Thanks, you are right. I updated my patch, which returns 0 when the internal pointer to the shared data object is not null. 307806 5 73593 kling 2010-11-11 03:11:25 -0800 Comment on attachment 73593 second try This is a purely artificial problem since we always build with USE(JSC) and the V8 port will most likely require large changes to QWebScriptWorld anyway. That said, I don't see any harm in adding this check. r=me 307891 6 73593 commit-queue 2010-11-11 06:51:01 -0800 Comment on attachment 73593 second try Clearing flags on attachment: 73593 Committed r71818: <http://trac.webkit.org/changeset/71818> 307892 7 commit-queue 2010-11-11 06:51:07 -0800 All reviewed patches have been landed. Closing bug. 73536 2010-11-10 13:52:18 -0800 2010-11-11 03:07:43 -0800 first try 49342.patch text/plain 969 max.hong.shen SW5kZXg6IFdlYktpdC9xdC9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViS2l0L3F0L0NoYW5n ZUxvZwkocmV2aXNpb24gNzE3NjkpCisrKyBXZWJLaXQvcXQvQ2hhbmdlTG9nCSh3b3JraW5nIGNv cHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTAtMTEtMTAgIFlpIFNoZW4gIDx5aS40LnNoZW5Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFtRdF0gUVdlYlNjcmlwdFdvcmxkOjp3b3JsZCgpIG1heSBjcmFzaAorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDkzNDIKKworICAgICAgICAqIEFwaS9x d2Vic2NyaXB0d29ybGQuY3BwOgorICAgICAgICAoUVdlYlNjcmlwdFdvcmxkOjp3b3JsZCk6CisK IDIwMTAtMTEtMTAgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgog CiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHI3MTczMy4KSW5kZXg6IFdlYktpdC9x dC9BcGkvcXdlYnNjcmlwdHdvcmxkLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXQvcXQvQXBpL3F3 ZWJzY3JpcHR3b3JsZC5jcHAJKHJldmlzaW9uIDcxNzU5KQorKysgV2ViS2l0L3F0L0FwaS9xd2Vi c2NyaXB0d29ybGQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01MCw3ICs1MCw3IEBAIFFXZWJTY3Jp cHRXb3JsZCAmUVdlYlNjcmlwdFdvcmxkOjpvcGVyYXQKIAogRE9NV3JhcHBlcldvcmxkKiBRV2Vi U2NyaXB0V29ybGQ6OndvcmxkKCkgY29uc3QKIHsKLSAgICByZXR1cm4gZC0+d29ybGQuZ2V0KCk7 CisgICAgcmV0dXJuIGQuZGF0YSgpID8gZC0+d29ybGQuZ2V0KCkgOiBOVUxMOwogfQogCiAvKiEK 73593 2010-11-11 03:07:43 -0800 2010-11-11 06:51:01 -0800 second try 49342.patch text/plain 990 max.hong.shen SW5kZXg6IFdlYktpdC9xdC9DaGFuZ2VMb2cNCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0NCi0tLSBXZWJLaXQvcXQvQ2hh bmdlTG9nCShyZXZpc2lvbiA3MTc2OSkNCisrKyBXZWJLaXQvcXQvQ2hhbmdlTG9nCSh3b3JraW5n IGNvcHkpDQpAQCAtMSwzICsxLDEzIEBADQorMjAxMC0xMS0xMCAgWWkgU2hlbiAgPHlpLjQuc2hl bkBub2tpYS5jb20+DQorDQorICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4NCisN CisgICAgICAgIFtRdF0gUVdlYlNjcmlwdFdvcmxkOjp3b3JsZCgpIG1heSBjcmFzaA0KKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ5MzQyDQorDQorICAg ICAgICAqIEFwaS9xd2Vic2NyaXB0d29ybGQuY3BwOg0KKyAgICAgICAgKFFXZWJTY3JpcHRXb3Js ZDo6d29ybGQpOg0KKw0KIDIwMTAtMTEtMTAgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5i b3RAZ21haWwuY29tPg0KIA0KICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjcxNzMz Lg0KSW5kZXg6IFdlYktpdC9xdC9BcGkvcXdlYnNjcmlwdHdvcmxkLmNwcA0KPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQ0K LS0tIFdlYktpdC9xdC9BcGkvcXdlYnNjcmlwdHdvcmxkLmNwcAkocmV2aXNpb24gNzE3NTkpDQor KysgV2ViS2l0L3F0L0FwaS9xd2Vic2NyaXB0d29ybGQuY3BwCSh3b3JraW5nIGNvcHkpDQpAQCAt NTAsNyArNTAsNyBAQCBRV2ViU2NyaXB0V29ybGQgJlFXZWJTY3JpcHRXb3JsZDo6b3BlcmF0DQog DQogRE9NV3JhcHBlcldvcmxkKiBRV2ViU2NyaXB0V29ybGQ6OndvcmxkKCkgY29uc3QNCiB7DQot ICAgIHJldHVybiBkLT53b3JsZC5nZXQoKTsNCisgICAgcmV0dXJuIGQgPyBkLT53b3JsZC5nZXQo KSA6IDA7DQogfQ0KIA0KIC8qIQ0K