34573 2010-02-04 04:00:19 -0800 [Qt] layoutTestController.counterValueForElementById crashes for a nonexistent ID 2010-04-25 09:17:14 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 hamaji webkit-unassigned diegohcg hausmann kenneth tonikitoo oldest_to_newest 187726 0 hamaji 2010-02-04 04:00:19 -0800 This was added in Bug 33840. 187738 1 48133 hamaji 2010-02-04 04:42:19 -0800 Created attachment 48133 Patch v1 187740 2 48133 hamaji 2010-02-04 04:46:40 -0800 Comment on attachment 48133 Patch v1 > diff --git a/WebKit/qt/Api/qwebframe.cpp b/WebKit/qt/Api/qwebframe.cpp > index aeb7a22..bc00f1b 100644 > --- a/WebKit/qt/Api/qwebframe.cpp > +++ b/WebKit/qt/Api/qwebframe.cpp > @@ -214,12 +214,23 @@ QString QWEBKIT_EXPORT qt_drt_counterValueForElementById(QWebFrame* qFrame, cons > { > Frame* frame = QWebFramePrivate::core(qFrame); > if (Document* document = frame->document()) { > - Element* element = document->getElementById(id); > - return WebCore::counterValueForElement(element); > + if (Element* element = document->getElementById(id)) > + return WebCore::counterValueForElement(element); This change is not related to this change, but I think it's small enough to fix with the main change. This change avoids crash when a nonexistent element ID is specified. If a reviewer considers this should be done in a separate change, I'm happy to split. 189468 3 48133 hausmann 2010-02-10 14:35:57 -0800 Comment on attachment 48133 Patch v1 Nice! Thanks :) 189469 4 hausmann 2010-02-10 14:36:15 -0800 (In reply to comment #2) > (From update of attachment 48133 [details]) > > diff --git a/WebKit/qt/Api/qwebframe.cpp b/WebKit/qt/Api/qwebframe.cpp > > index aeb7a22..bc00f1b 100644 > > --- a/WebKit/qt/Api/qwebframe.cpp > > +++ b/WebKit/qt/Api/qwebframe.cpp > > @@ -214,12 +214,23 @@ QString QWEBKIT_EXPORT qt_drt_counterValueForElementById(QWebFrame* qFrame, cons > > { > > Frame* frame = QWebFramePrivate::core(qFrame); > > if (Document* document = frame->document()) { > > - Element* element = document->getElementById(id); > > - return WebCore::counterValueForElement(element); > > + if (Element* element = document->getElementById(id)) > > + return WebCore::counterValueForElement(element); > > This change is not related to this change, but I think it's small enough to fix > with the main change. This change avoids crash when a nonexistent element ID is > specified. If a reviewer considers this should be done in a separate change, > I'm happy to split. No worries and good catch :) 189477 5 kenneth 2010-02-10 14:48:45 -0800 Didn't diego do the exact same patch today? 189573 6 tonikitoo 2010-02-10 19:01:50 -0800 (In reply to comment #5) > Didn't diego do the exact same patch today? yes, see bug 34777, which has landed. *** This bug has been marked as a duplicate of bug 34777 *** 216837 7 hamaji 2010-04-25 09:15:40 -0700 Though the main change was done by Diego, I'll land the r+ed patch only with the crash fix. 216838 8 hamaji 2010-04-25 09:17:14 -0700 Committed r58230: <http://trac.webkit.org/changeset/58230> 48133 2010-02-04 04:42:19 -0800 2010-02-10 14:35:57 -0800 Patch v1 bug-34573-20100204214217.patch text/plain 5766 hamaji ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA1ZmI4NTg0Li4zNzJjMDc0IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTIgQEAKKzIwMTAtMDIt MDQgIFNoaW5pY2hpcm8gSGFtYWppICA8aGFtYWppQGNocm9taXVtLm9yZz4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbUXRdIEltcGxlbWVudCBsYXlv dXRUZXN0Q29udHJvbGxlci5wYWdlTnVtYmVyRm9yRWxlbWVudEJ5SWQKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM0NTczCisKKyAgICAgICAgKiBwbGF0 Zm9ybS9xdC9Ta2lwcGVkOgorCiAyMDEwLTAyLTAyICBDc2FiYSBPc3p0cm9nb27DoWMgIDxvc3N5 QHdlYmtpdC5vcmc+CiAKICAgICAgICAgW1F0XSBmYXN0L2ZyYW1lcy9zZXQtdW5sb2FkZWQtZnJh bWUtbG9jYXRpb24uaHRtbCB0aW1lZCBvdXQgb24gUXQgTGludXggUmVsZWFzZSBCdWlsZCBCb3QK ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL3BsYXRmb3JtL3F0L1NraXBwZWQgYi9MYXlvdXRUZXN0 cy9wbGF0Zm9ybS9xdC9Ta2lwcGVkCmluZGV4IDdjNTIyZjMuLmYzNTY2YmIgMTAwNjQ0Ci0tLSBh L0xheW91dFRlc3RzL3BsYXRmb3JtL3F0L1NraXBwZWQKKysrIGIvTGF5b3V0VGVzdHMvcGxhdGZv cm0vcXQvU2tpcHBlZApAQCAtNTA2Niw3ICs1MDY2LDMgQEAgY3NzMi4xL3QxNTA1LWM1MjQtZm9u dC12YXItMDAtYi5odG1sCiAKICMgUmVsaWVzIG9uIFdlYktpdCBBUEkgW1dlYlZpZXcgX2xvYWRC YWNrRm9yd2FyZExpc3RGcm9tT3RoZXJWaWV3Ol0KIGZhc3QvbG9hZGVyL2NyYXNoLWNvcHlpbmct YmFja2ZvcndhcmRsaXN0Lmh0bWwKLQotIyBJbXBsZW1lbnQgTGF5b3V0VGVzdENvbnRyb2xsZXI6 OnBhZ2VOdW1iZXJGb3JFbGVtZW50QnlJZCgpLgotcHJpbnRpbmcvcGFnZS1icmVhay1hbHdheXMu aHRtbAotcHJpbnRpbmcvcGFnZU51bWVyRm9yRWxlbWVudEJ5SWQuaHRtbApkaWZmIC0tZ2l0IGEv V2ViS2l0L3F0L0FwaS9xd2ViZnJhbWUuY3BwIGIvV2ViS2l0L3F0L0FwaS9xd2ViZnJhbWUuY3Bw CmluZGV4IGFlYjdhMjIuLmJjMDBmMWIgMTAwNjQ0Ci0tLSBhL1dlYktpdC9xdC9BcGkvcXdlYmZy YW1lLmNwcAorKysgYi9XZWJLaXQvcXQvQXBpL3F3ZWJmcmFtZS5jcHAKQEAgLTIxNCwxMiArMjE0 LDIzIEBAIFFTdHJpbmcgUVdFQktJVF9FWFBPUlQgcXRfZHJ0X2NvdW50ZXJWYWx1ZUZvckVsZW1l bnRCeUlkKFFXZWJGcmFtZSogcUZyYW1lLCBjb25zCiB7CiAgICAgRnJhbWUqIGZyYW1lID0gUVdl YkZyYW1lUHJpdmF0ZTo6Y29yZShxRnJhbWUpOwogICAgIGlmIChEb2N1bWVudCogZG9jdW1lbnQg PSBmcmFtZS0+ZG9jdW1lbnQoKSkgewotICAgICAgICBFbGVtZW50KiBlbGVtZW50ID0gZG9jdW1l bnQtPmdldEVsZW1lbnRCeUlkKGlkKTsKLSAgICAgICAgcmV0dXJuIFdlYkNvcmU6OmNvdW50ZXJW YWx1ZUZvckVsZW1lbnQoZWxlbWVudCk7CisgICAgICAgIGlmIChFbGVtZW50KiBlbGVtZW50ID0g ZG9jdW1lbnQtPmdldEVsZW1lbnRCeUlkKGlkKSkKKyAgICAgICAgICAgIHJldHVybiBXZWJDb3Jl Ojpjb3VudGVyVmFsdWVGb3JFbGVtZW50KGVsZW1lbnQpOwogICAgIH0KICAgICByZXR1cm4gUVN0 cmluZygpOwogfQogCisvLyBSZXR1cm5zIHRoZSBudW1iZXIgb2YgcGFnZSB0aGUgZWxlbWVudCBz cGVjaWZpZWQgYnkgXGEgaWQgd2lsbCBiZSBwdXQuCitpbnQgUVdFQktJVF9FWFBPUlQgcXRfZHJ0 X3BhZ2VOdW1iZXJGb3JFbGVtZW50QnlJZChRV2ViRnJhbWUqIHFGcmFtZSwgY29uc3QgUVN0cmlu ZyYgaWQsIGZsb2F0IHBhZ2VXaWR0aCwgZmxvYXQgcGFnZUhlaWdodCkKK3sKKyAgICBGcmFtZSog ZnJhbWUgPSBRV2ViRnJhbWVQcml2YXRlOjpjb3JlKHFGcmFtZSk7CisgICAgaWYgKERvY3VtZW50 KiBkb2N1bWVudCA9IGZyYW1lLT5kb2N1bWVudCgpKSB7CisgICAgICAgIGlmIChFbGVtZW50KiBl bGVtZW50ID0gZG9jdW1lbnQtPmdldEVsZW1lbnRCeUlkKGlkKSkKKyAgICAgICAgICAgIHJldHVy biBQcmludENvbnRleHQ6OnBhZ2VOdW1iZXJGb3JFbGVtZW50KGVsZW1lbnQsIEZsb2F0U2l6ZShw YWdlV2lkdGgsIHBhZ2VIZWlnaHQpKTsKKyAgICB9CisgICAgcmV0dXJuIC0xOworfQorCiAvLyBT dXNwZW5kIGFjdGl2ZSBET00gb2JqZWN0cyBpbiB0aGlzIGZyYW1lLgogdm9pZCBRV0VCS0lUX0VY UE9SVCBxdF9zdXNwZW5kQWN0aXZlRE9NT2JqZWN0cyhRV2ViRnJhbWUqIHFGcmFtZSkKIHsKZGlm ZiAtLWdpdCBhL1dlYktpdC9xdC9DaGFuZ2VMb2cgYi9XZWJLaXQvcXQvQ2hhbmdlTG9nCmluZGV4 IDYzZTQwMDAuLmNlMjUzOWIgMTAwNjQ0Ci0tLSBhL1dlYktpdC9xdC9DaGFuZ2VMb2cKKysrIGIv V2ViS2l0L3F0L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDEwLTAyLTA0ICBTaGluaWNo aXJvIEhhbWFqaSAgPGhhbWFqaUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW1F0XSBJbXBsZW1lbnQgbGF5b3V0VGVzdENvbnRy b2xsZXIucGFnZU51bWJlckZvckVsZW1lbnRCeUlkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zNDU3MworCisgICAgICAgICogQXBpL3F3ZWJmcmFtZS5j cHA6CisgICAgICAgIChxdF9kcnRfY291bnRlclZhbHVlRm9yRWxlbWVudEJ5SWQpOgorICAgICAg ICAocXRfZHJ0X3BhZ2VOdW1iZXJGb3JFbGVtZW50QnlJZCk6CisKIDIwMTAtMDItMDIgIE5vYW0g Um9zZW50aGFsICA8bm9hbS5yb3NlbnRoYWxAbm9raWEuY29tPgogCiAgICAgICAgIFJldmlld2Vk IGJ5IEtlbm5ldGggUm9oZGUgQ2hyaXN0aWFuc2VuLgpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMv Q2hhbmdlTG9nIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCmluZGV4IGY3MTI0OGMuLmFiNDM0M2Ug MTAwNjQ0Ci0tLSBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZworKysgYi9XZWJLaXRUb29scy9DaGFu Z2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMC0wMi0wNCAgU2hpbmljaGlybyBIYW1hamkgIDxo YW1hamlAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIFtRdF0gSW1wbGVtZW50IGxheW91dFRlc3RDb250cm9sbGVyLnBhZ2VOdW1i ZXJGb3JFbGVtZW50QnlJZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MzQ1NzMKKworICAgICAgICAqIER1bXBSZW5kZXJUcmVlL3F0L0xheW91dFRlc3RD b250cm9sbGVyUXQuY3BwOgorICAgICAgICAoTGF5b3V0VGVzdENvbnRyb2xsZXI6OnBhZ2VOdW1i ZXJGb3JFbGVtZW50QnlJZCk6CisgICAgICAgICogRHVtcFJlbmRlclRyZWUvcXQvTGF5b3V0VGVz dENvbnRyb2xsZXJRdC5oOgorCiAyMDEwLTAyLTAxICBDaHJpcyBKZXJkb25layAgPGNqZXJkb25l a0B3ZWJraXQub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFNoaW5pY2hpcm8gSGFtYWppLgpk aWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMvRHVtcFJlbmRlclRyZWUvcXQvTGF5b3V0VGVzdENvbnRy b2xsZXJRdC5jcHAgYi9XZWJLaXRUb29scy9EdW1wUmVuZGVyVHJlZS9xdC9MYXlvdXRUZXN0Q29u dHJvbGxlclF0LmNwcAppbmRleCBjYTQ4MDAzLi4yNDlhZTcxIDEwMDY0NAotLS0gYS9XZWJLaXRU b29scy9EdW1wUmVuZGVyVHJlZS9xdC9MYXlvdXRUZXN0Q29udHJvbGxlclF0LmNwcAorKysgYi9X ZWJLaXRUb29scy9EdW1wUmVuZGVyVHJlZS9xdC9MYXlvdXRUZXN0Q29udHJvbGxlclF0LmNwcApA QCAtNDcsNiArNDcsNyBAQCBleHRlcm4gdm9pZCBxdF9kcnRfc2V0RG9tYWluUmVsYXhhdGlvbkZv cmJpZGRlbkZvclVSTFNjaGVtZShib29sIGZvcmJpZGRlbiwgY29ucwogCiBleHRlcm4gdm9pZCBx dF9kcnRfd2hpdGVMaXN0QWNjZXNzRnJvbU9yaWdpbihjb25zdCBRU3RyaW5nJiBzb3VyY2VPcmln aW4sIGNvbnN0IFFTdHJpbmcmIGRlc3RpbmF0aW9uUHJvdG9jb2wsIGNvbnN0IFFTdHJpbmcmIGRl c3RpbmF0aW9uSG9zdCwgYm9vbCBhbGxvd0Rlc3RpbmF0aW9uU3ViZG9tYWlucyk7CiBleHRlcm4g UVN0cmluZyBxdF9kcnRfY291bnRlclZhbHVlRm9yRWxlbWVudEJ5SWQoUVdlYkZyYW1lKiBxRnJh bWUsIGNvbnN0IFFTdHJpbmcmIGlkKTsKK2V4dGVybiBpbnQgcXRfZHJ0X3BhZ2VOdW1iZXJGb3JF bGVtZW50QnlJZChRV2ViRnJhbWUqIHFGcmFtZSwgY29uc3QgUVN0cmluZyYgaWQsIGZsb2F0IHBh Z2VXaWR0aCwgZmxvYXQgcGFnZUhlaWdodCk7CiBleHRlcm4gaW50IHF0X2RydF93b3JrZXJUaHJl YWRDb3VudCgpOwogCiBMYXlvdXRUZXN0Q29udHJvbGxlcjo6TGF5b3V0VGVzdENvbnRyb2xsZXIo V2ViQ29yZTo6RHVtcFJlbmRlclRyZWUqIGRydCkKQEAgLTEzNSw2ICsxMzYsMTYgQEAgUVN0cmlu ZyBMYXlvdXRUZXN0Q29udHJvbGxlcjo6Y291bnRlclZhbHVlRm9yRWxlbWVudEJ5SWQoY29uc3Qg UVN0cmluZyYgaWQpCiAgICAgcmV0dXJuIHF0X2RydF9jb3VudGVyVmFsdWVGb3JFbGVtZW50QnlJ ZChtX2RydC0+d2ViUGFnZSgpLT5tYWluRnJhbWUoKSwgaWQpOwogfQogCitpbnQgTGF5b3V0VGVz dENvbnRyb2xsZXI6OnBhZ2VOdW1iZXJGb3JFbGVtZW50QnlJZChjb25zdCBRU3RyaW5nJiBpZCkK K3sKKyAgICByZXR1cm4gcGFnZU51bWJlckZvckVsZW1lbnRCeUlkKGlkLCA4MDAsIDYwMCk7Cit9 CisKK2ludCBMYXlvdXRUZXN0Q29udHJvbGxlcjo6cGFnZU51bWJlckZvckVsZW1lbnRCeUlkKGNv bnN0IFFTdHJpbmcmIGlkLCBmbG9hdCBwYWdlV2lkdGgsIGZsb2F0IHBhZ2VIZWlnaHQpCit7Cisg ICAgcmV0dXJuIHF0X2RydF9wYWdlTnVtYmVyRm9yRWxlbWVudEJ5SWQobV9kcnQtPndlYlBhZ2Uo KS0+bWFpbkZyYW1lKCksIGlkLCBwYWdlV2lkdGgsIHBhZ2VIZWlnaHQpOworfQorCiBpbnQgTGF5 b3V0VGVzdENvbnRyb2xsZXI6OndlYkhpc3RvcnlJdGVtQ291bnQoKQogewogICAgIGlmICghbV93 ZWJIaXN0b3J5KQpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMvRHVtcFJlbmRlclRyZWUvcXQvTGF5 b3V0VGVzdENvbnRyb2xsZXJRdC5oIGIvV2ViS2l0VG9vbHMvRHVtcFJlbmRlclRyZWUvcXQvTGF5 b3V0VGVzdENvbnRyb2xsZXJRdC5oCmluZGV4IDM2NTY0MGQuLjA1Njg5ZTkgMTAwNjQ0Ci0tLSBh L1dlYktpdFRvb2xzL0R1bXBSZW5kZXJUcmVlL3F0L0xheW91dFRlc3RDb250cm9sbGVyUXQuaAor KysgYi9XZWJLaXRUb29scy9EdW1wUmVuZGVyVHJlZS9xdC9MYXlvdXRUZXN0Q29udHJvbGxlclF0 LmgKQEAgLTg4LDYgKzg4LDggQEAgcHVibGljIHNsb3RzOgogICAgIHZvaWQgc2V0Q2FuT3Blbldp bmRvd3MoKSB7IG1fY2FuT3BlbldpbmRvd3MgPSB0cnVlOyB9CiAgICAgdm9pZCB3YWl0VW50aWxE b25lKCk7CiAgICAgUVN0cmluZyBjb3VudGVyVmFsdWVGb3JFbGVtZW50QnlJZChjb25zdCBRU3Ry aW5nJiBpZCk7CisgICAgaW50IHBhZ2VOdW1iZXJGb3JFbGVtZW50QnlJZChjb25zdCBRU3RyaW5n JiBpZCk7CisgICAgaW50IHBhZ2VOdW1iZXJGb3JFbGVtZW50QnlJZChjb25zdCBRU3RyaW5nJiBp ZCwgZmxvYXQgcGFnZVdpZHRoLCBmbG9hdCBwYWdlSGVpZ2h0KTsKICAgICBpbnQgd2ViSGlzdG9y eUl0ZW1Db3VudCgpOwogICAgIHZvaWQga2VlcFdlYkhpc3RvcnkoKTsKICAgICB2b2lkIG5vdGlm eURvbmUoKTsK