Bug 89946 – Enforce WebFrameImpl::scopeStringMatches reset flag to start a new search

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit Misc.

[Carlos]

Related to fixing Chromium issue: http://code.google.com/p/chromium/issues/detail?id=74332

Reset flag in WebFrameImpl::scopeStringMatches (+1680 Source/WebKit/chromium/src/WebFrameImpl.cpp) means we want a brand new search. There is a early return condition which does not consider the reset flag. Situations like in the chromium report we need to start a brand new search, even if shouldScopeMatches(searchText) is false because content changed in a frame.

Steps to reproduce: Same as chromium issue

[Carlos]

Created attachment 149463 [details]
Patch

[Finnur Thorarinsson]

I don't think this is the right fix either. 

Before this change, frames that completed the Find operation and reported 0 matches will not be searched again if you just add more characters to the search. That's no longer the case with this change (basically this disables the optimization altogether). We should be very careful in changing that because in previous tests making that change killed Find performance in large applications like Gmail, if I recall.

I guess what needs to happen is that we need to figure out the performance impact of removing this optimization. If it is negligible we can remove all the code below the ASSERT in shouldScopeMatches (and just return true), which might on its own be enough to fix this bug. 

If that isn't working, then we need to find a way on LOAD_COMMITTED to notify webkit that the frame needs to be searched, even if it returned 0 matches last time. I don't remember if we can trigger something to happen on LOAD_COMMITTED in Webkit (we could then clear the "dont-search-again" flag, I guess), but if not then we'd need to plumb a message down to Webkit from the Browser process to ask it to clear the flag for a particular frame.

[Darin Adler]

Comment on attachment 149463 [details]
Patch

Clearing flag from a Chromium-specific patch.

[Darin Adler]

Chromium-specific.