57552 2011-03-31 08:44:26 -0700 Find client not told when WKPageCountStringMatches exceeds maximum 2011-03-31 10:40:00 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 sullivan sullivan oldest_to_newest 377177 0 sullivan 2011-03-31 08:44:26 -0700 If the number of string matches on a page exceeds the maximum passed to WKPageCountStringMatches, the callback will report the passed maximum, rather than the special value kWKMoreThanMaximumMatchCount. 377179 1 87732 sullivan 2011-03-31 08:46:21 -0700 Created attachment 87732 Patch that uses the same technique as the find-string case. 377180 2 87732 sullivan 2011-03-31 08:47:21 -0700 Comment on attachment 87732 Patch that uses the same technique as the find-string case. Oops, forgot to save the ChangeLog before creating the patch. New one coming up shortly. 377182 3 sullivan 2011-03-31 08:48:12 -0700 In Radar as <rdar://problem/9214824> 377183 4 87733 sullivan 2011-03-31 08:49:43 -0700 Created attachment 87733 Take two, featuring non-degenerate ChangeLog entry. 377184 5 87733 aroben 2011-03-31 08:53:46 -0700 Comment on attachment 87733 Take two, featuring non-degenerate ChangeLog entry. View in context: https://bugs.webkit.org/attachment.cgi?id=87733&action=review > Source/WebKit2/WebProcess/WebPage/FindController.cpp:72 > void FindController::countStringMatches(const String& string, FindOptions options, unsigned maxMatchCount) > { > - unsigned matchCount = m_webPage->corePage()->markAllMatchesForText(string, core(options), false, maxMatchCount); > + unsigned matchCount = m_webPage->corePage()->markAllMatchesForText(string, core(options), false, maxMatchCount + 1); > m_webPage->corePage()->unmarkAllTextMatches(); > > + // Check if we have more matches than allowed. > + if (matchCount > maxMatchCount) > + matchCount = static_cast<unsigned>(kWKMoreThanMaximumMatchCount); Do we need to worry about someone passing numeric_limits<unsigned>::max() as the maxMatchCount argument to this function? That would result in underflow. 377221 6 87733 darin 2011-03-31 09:15:50 -0700 Comment on attachment 87733 Take two, featuring non-degenerate ChangeLog entry. View in context: https://bugs.webkit.org/attachment.cgi?id=87733&action=review >> Source/WebKit2/WebProcess/WebPage/FindController.cpp:72 >> + matchCount = static_cast<unsigned>(kWKMoreThanMaximumMatchCount); > > Do we need to worry about someone passing numeric_limits<unsigned>::max() as the maxMatchCount argument to this function? That would result in underflow. I think you probably mean overflow, resulting in the number 0. An easy way to handle that overflow would be changing max() to max() - 1. 377270 7 sullivan 2011-03-31 09:58:47 -0700 I'll fix the max() case for both this function and findMatchesForString() and submit another patch. 377273 8 87754 sullivan 2011-03-31 10:04:46 -0700 Created attachment 87754 Take three, now handles numeric_limits<unsigned>::max() 377277 9 87733 aroben 2011-03-31 10:06:52 -0700 Comment on attachment 87733 Take two, featuring non-degenerate ChangeLog entry. View in context: https://bugs.webkit.org/attachment.cgi?id=87733&action=review >>> Source/WebKit2/WebProcess/WebPage/FindController.cpp:72 >>> + matchCount = static_cast<unsigned>(kWKMoreThanMaximumMatchCount); >> >> Do we need to worry about someone passing numeric_limits<unsigned>::max() as the maxMatchCount argument to this function? That would result in underflow. > > I think you probably mean overflow, resulting in the number 0. > > An easy way to handle that overflow would be changing max() to max() - 1. Right, overflow! 377321 10 sullivan 2011-03-31 10:40:00 -0700 Fixed in <http://trac.webkit.org/changeset/82594>. 87732 2011-03-31 08:46:21 -0700 2011-03-31 10:04:46 -0700 Patch that uses the same technique as the find-string case. countMatchesMaximum_patch.txt text/plain 1540 sullivan SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDgyNTgyKQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTIgQEAKKzIwMTEtMDMtMzEgIEpvaG4gU3Vs bGl2YW4gIDxzdWxsaXZhbkBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgTmVlZCBhIHNob3J0IGRlc2NyaXB0aW9uIGFuZCBidWcgVVJM IChPT1BTISkKKworICAgICAgICAqIFdlYlByb2Nlc3MvV2ViUGFnZS9GaW5kQ29udHJvbGxlci5j cHA6CisgICAgICAgIChXZWJLaXQ6OkZpbmRDb250cm9sbGVyOjpjb3VudFN0cmluZ01hdGNoZXMp OgorCiAyMDExLTAzLTMxICBFdmFuIE1hcnRpbiAgPGV2YW5AY2hyb21pdW0ub3JnPgogCiAgICAg ICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsLgpJbmRleDogU291cmNlL1dlYktpdDIvV2ViUHJv Y2Vzcy9XZWJQYWdlL0ZpbmRDb250cm9sbGVyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvRmluZENvbnRyb2xsZXIuY3BwCShyZXZpc2lvbiA4MjQz MSkKKysrIFNvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9GaW5kQ29udHJvbGxlci5j cHAJKHdvcmtpbmcgY29weSkKQEAgLTY0LDkgKzY0LDEzIEBAIEZpbmRDb250cm9sbGVyOjp+Rmlu ZENvbnRyb2xsZXIoKQogCiB2b2lkIEZpbmRDb250cm9sbGVyOjpjb3VudFN0cmluZ01hdGNoZXMo Y29uc3QgU3RyaW5nJiBzdHJpbmcsIEZpbmRPcHRpb25zIG9wdGlvbnMsIHVuc2lnbmVkIG1heE1h dGNoQ291bnQpCiB7Ci0gICAgdW5zaWduZWQgbWF0Y2hDb3VudCA9IG1fd2ViUGFnZS0+Y29yZVBh Z2UoKS0+bWFya0FsbE1hdGNoZXNGb3JUZXh0KHN0cmluZywgY29yZShvcHRpb25zKSwgZmFsc2Us IG1heE1hdGNoQ291bnQpOworICAgIHVuc2lnbmVkIG1hdGNoQ291bnQgPSBtX3dlYlBhZ2UtPmNv cmVQYWdlKCktPm1hcmtBbGxNYXRjaGVzRm9yVGV4dChzdHJpbmcsIGNvcmUob3B0aW9ucyksIGZh bHNlLCBtYXhNYXRjaENvdW50ICsgMSk7CiAgICAgbV93ZWJQYWdlLT5jb3JlUGFnZSgpLT51bm1h cmtBbGxUZXh0TWF0Y2hlcygpOwogCisgICAgLy8gQ2hlY2sgaWYgd2UgaGF2ZSBtb3JlIG1hdGNo ZXMgdGhhbiBhbGxvd2VkLgorICAgIGlmIChtYXRjaENvdW50ID4gbWF4TWF0Y2hDb3VudCkKKyAg ICAgICAgbWF0Y2hDb3VudCA9IHN0YXRpY19jYXN0PHVuc2lnbmVkPihrV0tNb3JlVGhhbk1heGlt dW1NYXRjaENvdW50KTsKKyAgICAKICAgICBtX3dlYlBhZ2UtPnNlbmQoTWVzc2FnZXM6OldlYlBh Z2VQcm94eTo6RGlkQ291bnRTdHJpbmdNYXRjaGVzKHN0cmluZywgbWF0Y2hDb3VudCkpOwogfQog Cg== 87733 2011-03-31 08:49:43 -0700 2011-03-31 10:06:52 -0700 Take two, featuring non-degenerate ChangeLog entry. countMatchesMaximum_patch.txt text/plain 1747 sullivan SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDgyNTgyKQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTEtMDMtMzEgIEpvaG4gU3Vs bGl2YW4gIDxzdWxsaXZhbkBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzkyMTQ4MjQ+IEZpbmQgY2xpZW50 IG5vdCB0b2xkIHdoZW4gV0tQYWdlQ291bnRTdHJpbmdNYXRjaGVzIGV4Y2VlZHMgbWF4aW11bQor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTc1NTIKKwor ICAgICAgICAqIFdlYlByb2Nlc3MvV2ViUGFnZS9GaW5kQ29udHJvbGxlci5jcHA6CisgICAgICAg IChXZWJLaXQ6OkZpbmRDb250cm9sbGVyOjpjb3VudFN0cmluZ01hdGNoZXMpOgorICAgICAgICBV c2UgdGhlIHNhbWUgdGVjaG5pcXVlIGFzIGZpbmRTdHJpbmcoKSB0byByZXBvcnQga1dLTW9yZVRo YW5NYXhpbXVtTWF0Y2hDb3VudCB3aGVuIGFwcHJvcHJpYXRlLgorCiAyMDExLTAzLTMxICBFdmFu IE1hcnRpbiAgPGV2YW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEVyaWMg U2VpZGVsLgpJbmRleDogU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL0ZpbmRDb250 cm9sbGVyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBh Z2UvRmluZENvbnRyb2xsZXIuY3BwCShyZXZpc2lvbiA4MjQzMSkKKysrIFNvdXJjZS9XZWJLaXQy L1dlYlByb2Nlc3MvV2ViUGFnZS9GaW5kQ29udHJvbGxlci5jcHAJKHdvcmtpbmcgY29weSkKQEAg LTY0LDkgKzY0LDEzIEBAIEZpbmRDb250cm9sbGVyOjp+RmluZENvbnRyb2xsZXIoKQogCiB2b2lk IEZpbmRDb250cm9sbGVyOjpjb3VudFN0cmluZ01hdGNoZXMoY29uc3QgU3RyaW5nJiBzdHJpbmcs IEZpbmRPcHRpb25zIG9wdGlvbnMsIHVuc2lnbmVkIG1heE1hdGNoQ291bnQpCiB7Ci0gICAgdW5z aWduZWQgbWF0Y2hDb3VudCA9IG1fd2ViUGFnZS0+Y29yZVBhZ2UoKS0+bWFya0FsbE1hdGNoZXNG b3JUZXh0KHN0cmluZywgY29yZShvcHRpb25zKSwgZmFsc2UsIG1heE1hdGNoQ291bnQpOworICAg IHVuc2lnbmVkIG1hdGNoQ291bnQgPSBtX3dlYlBhZ2UtPmNvcmVQYWdlKCktPm1hcmtBbGxNYXRj aGVzRm9yVGV4dChzdHJpbmcsIGNvcmUob3B0aW9ucyksIGZhbHNlLCBtYXhNYXRjaENvdW50ICsg MSk7CiAgICAgbV93ZWJQYWdlLT5jb3JlUGFnZSgpLT51bm1hcmtBbGxUZXh0TWF0Y2hlcygpOwog CisgICAgLy8gQ2hlY2sgaWYgd2UgaGF2ZSBtb3JlIG1hdGNoZXMgdGhhbiBhbGxvd2VkLgorICAg IGlmIChtYXRjaENvdW50ID4gbWF4TWF0Y2hDb3VudCkKKyAgICAgICAgbWF0Y2hDb3VudCA9IHN0 YXRpY19jYXN0PHVuc2lnbmVkPihrV0tNb3JlVGhhbk1heGltdW1NYXRjaENvdW50KTsKKyAgICAK ICAgICBtX3dlYlBhZ2UtPnNlbmQoTWVzc2FnZXM6OldlYlBhZ2VQcm94eTo6RGlkQ291bnRTdHJp bmdNYXRjaGVzKHN0cmluZywgbWF0Y2hDb3VudCkpOwogfQogCg== 87754 2011-03-31 10:04:46 -0700 2011-03-31 10:06:24 -0700 Take three, now handles numeric_limits<unsigned>::max() countMatchesMaximum_patch.txt text/plain 2583 sullivan SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDgyNTkzKQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDMtMzEgIEpvaG4gU3Vs bGl2YW4gIDxzdWxsaXZhbkBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzkyMTQ4MjQ+IEZpbmQgY2xpZW50 IG5vdCB0b2xkIHdoZW4gV0tQYWdlQ291bnRTdHJpbmdNYXRjaGVzIGV4Y2VlZHMgbWF4aW11bQor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTc1NTIKKwor ICAgICAgICAqIFdlYlByb2Nlc3MvV2ViUGFnZS9GaW5kQ29udHJvbGxlci5jcHA6CisgICAgICAg IChXZWJLaXQ6OkZpbmRDb250cm9sbGVyOjpjb3VudFN0cmluZ01hdGNoZXMpOgorICAgICAgICBV c2UgdGhlIHNhbWUgdGVjaG5pcXVlIGFzIGZpbmRTdHJpbmcoKSB0byByZXBvcnQga1dLTW9yZVRo YW5NYXhpbXVtTWF0Y2hDb3VudCB3aGVuIGFwcHJvcHJpYXRlLgorICAgICAgICAoV2ViS2l0OjpG aW5kQ29udHJvbGxlcjo6ZmluZFN0cmluZyk6CisgICAgICAgIENvbnZlcnQgbnVtZXJpY19saW1p dHM6Om1heCgpIHRvIG1heCgpIC0gMSB0byBhdm9pZCBvdmVyZmxvdyBjYXNlLgorCiAyMDExLTAz LTMxICBFdmFuIE1hcnRpbiAgPGV2YW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2Vk IGJ5IEVyaWMgU2VpZGVsLgpJbmRleDogU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdl L0ZpbmRDb250cm9sbGVyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0Mi9XZWJQcm9j ZXNzL1dlYlBhZ2UvRmluZENvbnRyb2xsZXIuY3BwCShyZXZpc2lvbiA4MjQzMSkKKysrIFNvdXJj ZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9GaW5kQ29udHJvbGxlci5jcHAJKHdvcmtpbmcg Y29weSkKQEAgLTM4LDYgKzM4LDcgQEAKICNpbmNsdWRlIDxXZWJDb3JlL0dyYXBoaWNzQ29udGV4 dC5oPgogI2luY2x1ZGUgPFdlYkNvcmUvUGFnZS5oPgogCit1c2luZyBuYW1lc3BhY2Ugc3RkOwog dXNpbmcgbmFtZXNwYWNlIFdlYkNvcmU7CiAKIG5hbWVzcGFjZSBXZWJLaXQgewpAQCAtNjQsOSAr NjUsMTYgQEAgRmluZENvbnRyb2xsZXI6On5GaW5kQ29udHJvbGxlcigpCiAKIHZvaWQgRmluZENv bnRyb2xsZXI6OmNvdW50U3RyaW5nTWF0Y2hlcyhjb25zdCBTdHJpbmcmIHN0cmluZywgRmluZE9w dGlvbnMgb3B0aW9ucywgdW5zaWduZWQgbWF4TWF0Y2hDb3VudCkKIHsKLSAgICB1bnNpZ25lZCBt YXRjaENvdW50ID0gbV93ZWJQYWdlLT5jb3JlUGFnZSgpLT5tYXJrQWxsTWF0Y2hlc0ZvclRleHQo c3RyaW5nLCBjb3JlKG9wdGlvbnMpLCBmYWxzZSwgbWF4TWF0Y2hDb3VudCk7CisgICAgaWYgKG1h eE1hdGNoQ291bnQgPT0gbnVtZXJpY19saW1pdHM8dW5zaWduZWQ+OjptYXgoKSkKKyAgICAgICAg LS1tYXhNYXRjaENvdW50OworICAgIAorICAgIHVuc2lnbmVkIG1hdGNoQ291bnQgPSBtX3dlYlBh Z2UtPmNvcmVQYWdlKCktPm1hcmtBbGxNYXRjaGVzRm9yVGV4dChzdHJpbmcsIGNvcmUob3B0aW9u cyksIGZhbHNlLCBtYXhNYXRjaENvdW50ICsgMSk7CiAgICAgbV93ZWJQYWdlLT5jb3JlUGFnZSgp LT51bm1hcmtBbGxUZXh0TWF0Y2hlcygpOwogCisgICAgLy8gQ2hlY2sgaWYgd2UgaGF2ZSBtb3Jl IG1hdGNoZXMgdGhhbiBhbGxvd2VkLgorICAgIGlmIChtYXRjaENvdW50ID4gbWF4TWF0Y2hDb3Vu dCkKKyAgICAgICAgbWF0Y2hDb3VudCA9IHN0YXRpY19jYXN0PHVuc2lnbmVkPihrV0tNb3JlVGhh bk1heGltdW1NYXRjaENvdW50KTsKKyAgICAKICAgICBtX3dlYlBhZ2UtPnNlbmQoTWVzc2FnZXM6 OldlYlBhZ2VQcm94eTo6RGlkQ291bnRTdHJpbmdNYXRjaGVzKHN0cmluZywgbWF0Y2hDb3VudCkp OwogfQogCkBAIC0xMDIsNiArMTEwLDkgQEAgdm9pZCBGaW5kQ29udHJvbGxlcjo6ZmluZFN0cmlu Zyhjb25zdCBTdAogICAgICAgICBzaG91bGRTaG93T3ZlcmxheSA9IG9wdGlvbnMgJiBGaW5kT3B0 aW9uc1Nob3dPdmVybGF5OwogCiAgICAgICAgIGlmIChzaG91bGRTaG93T3ZlcmxheSkgeworICAg ICAgICAgICAgaWYgKG1heE1hdGNoQ291bnQgPT0gbnVtZXJpY19saW1pdHM8dW5zaWduZWQ+Ojpt YXgoKSkKKyAgICAgICAgICAgICAgICAtLW1heE1hdGNoQ291bnQ7CisgICAgICAgICAgICAKICAg ICAgICAgICAgIHVuc2lnbmVkIG1hdGNoQ291bnQgPSBtX3dlYlBhZ2UtPmNvcmVQYWdlKCktPm1h cmtBbGxNYXRjaGVzRm9yVGV4dChzdHJpbmcsIGNvcmUob3B0aW9ucyksIGZhbHNlLCBtYXhNYXRj aENvdW50ICsgMSk7CiAKICAgICAgICAgICAgIC8vIENoZWNrIGlmIHdlIGhhdmUgbW9yZSBtYXRj aGVzIHRoYW4gYWxsb3dlZC4K