It would be great if the WebContents.findInPage/stopFindInPage could be replaced with this new implementation. ie. just calling webContents.mainFrame.findInFrame internally.

They are different in behavior, hence I haven't replaced them. FindInPage will go over the frames in all WebContents of a page (i-e) if a top-level WebContents can have multiple child WebContents. This api is only for frames in the same WebContents

@deepak1556 @rzhao271 Thanks for looking into this.

I just tried it out for VS Code but one difference I noticed compared to search inside <webviews> is that find seems to focus the iframe. This causes problems when typing in our custom search widget, since the text input keeps losing focus

Any ideas on if that's something this API could change or if we can workaround this?

@deepak1556 are you still interested in landing this?

If so, how do WebContents.findInPage and WebFrameMain.findInFrame interact with one another?

Could we possibly consolidate these two systems into one by having WebFrameMain.findInFrame recurse into webviews that are hosted in subframes of the target frame?

Sorry for the delay in getting back to this PR

Could we possibly consolidate these two systems into one by having WebFrameMain.findInFrame recurse into webviews that are hosted in subframes of the target frame?

Originally I was not sure about merging them #28274 (comment) mainly because the FindRequestManager cannot handle concurrent find sessions within the same root and with FindInPage api user will never hit this scenario as they can only trigger the search from page root. But looking at it now, I can merge the behavior of FindInPage into FindInFrame when the target frame is a mainFrame FindRequestManager and possible throw a user facing error if there was an attempt for a search from another frame within the tree when there is an active session in progress.

are you still interested in landing this?

What is your take on the patch ? Unfortunately, I couldn't get any traction on the upstream issue https://bugs.chromium.org/p/chromium/issues/detail?id=1189789, so this will end up being an electron only patch if the API were to be supported.

What is your take on the patch?

It's not a crazy huge patch and it does add a legitimately useful feature which is sort of Electron-specific. It's a bit of a niche feature and it's likely to run into at least some code shear, so I'm not enthusiastic about it, but begrudgingly willing to take it. It would probably be worth spending some time thinking about whether we can reduce the surface area of the patch. Can we do some subclassing instead? Can we apply the filtering at a different point? Any thoughts?

Can we do some subclassing instead? Can we apply the filtering at a different point? Any thoughts?

The filtering in this case has to be performed at FindRequestManager, basically this class owns a per frame client called FindInPageClient that makes the find request with blink and awaits for response. FindRequestManager then combines the results from all the FindInPageClient for a given tree including out of process frames and allows traversal of the active matches.

I will first update the branch against main since the current patch is outdated and will check if there is a possibility to minimize it.

@deepak1556 is this PR still live?

There is need to upstream the frame find implementation but don't have cycles to visit this anytime soon. Closing for now.