Bug 57552 – Find client not told when WKPageCountStringMatches exceeds maximum

• Component: WebKit2
• Status: RESOLVED FIXED

[John Sullivan]

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.

[John Sullivan]

Created attachment 87732 [details]
Patch that uses the same technique as the find-string case.

[John Sullivan]

Comment on attachment 87732 [details]
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.

[John Sullivan]

In Radar as <rdar://problem/9214824>

[John Sullivan]

Created attachment 87733 [details]
Take two, featuring non-degenerate ChangeLog entry.

[Adam Roben (:aroben)]

Comment on attachment 87733 [details]
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.

[Darin Adler]

Comment on attachment 87733 [details]
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.

[John Sullivan]

I'll fix the max() case for both this function and findMatchesForString() and submit another patch.

[John Sullivan]

Created attachment 87754 [details]
Take three, now handles numeric_limits<unsigned>::max()

[Adam Roben (:aroben)]

Comment on attachment 87733 [details]
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!

[John Sullivan]

Fixed in <http://trac.webkit.org/changeset/82594>.