1729371 - [structured analysis search regression fixes] searching for the function nsDisplayScrollInfoLayer::UpdateScrollData doesn't find everything

Status: NEW

(Webtools :: Searchfox, defect)

Description

[Timothy Nikkel (:tnikkel)]

It works properly when I search mozilla-beta, but not central.

Compare
https://searchfox.org/mozilla-beta/search?q=symbol:_ZN7mozilla24nsDisplayScrollInfoLayer16UpdateScrollDataEPNS_6layers19WebRenderScrollDataEPNS1_24WebRenderLayerScrollDataE%2C_ZN7mozilla13nsDisplayItem16UpdateScrollDataEPNS_6layers19WebRenderScrollDataEPNS1_24WebRenderLayerScrollDataE&redirect=false
to
https://searchfox.org/mozilla-central/search?q=symbol:_ZN7mozilla24nsDisplayScrollInfoLayer16UpdateScrollDataEPNS_6layers19WebRenderScrollDataEPNS1_24WebRenderLayerScrollDataE&redirect=false
which are the urls I get when I click on that function and choose search function.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Thank you for filing this and providing a specific symbol with comparison!

This is a more complicated regression scenario from the representation changes in bug 1641372 than bug 1729164 which just wants a single layer of overrides. It's also exists somewhere along a continuum with bug 1466692 where the result set in this case is small enough that the original behavior was good.

Also it looks like these searches came from click on a "search for function" context menu, like clicking on the definition point here in beta rather than a search on the identifier, for example a search on nsDisplayScrollInfoLayer::UpdateScrollData. (Behaviorally this ends up the same for the qualified class, but is notable because as discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=1728376#c2 the context menu is a slightly different scenario since it effectively pre-translates identifiers to (sets of) symbols. Also note that just searching on UpdateScrollData I think captures everything, but doesn't fold them into a single definition group.)

In this case, the override hierarchy is as follows:

• nsDisplayItem::UpdateScrollData (2 uses) is the root and lists the following "Overridden by" instances each with their own "Symbol Search This" (which I'm using as the links)
  • nsDisplayRemote::UpdateScrollData
  • nsDisplayOwnLayer::UpdateScrollData (3 uses) which in turns has the following "Overridden by" instances:
    • nsDisplayFixedPosition::UpdateScrollData
    • nsDisplayStickyPosition::UpdateScrollData
    • nsDisplayAsyncZoom::UpdateScrollData
  • nsDisplayScrollInfoLayer::UpdateScrollData
  • nsDisplayTransform::UpdateScrollData

In mozilla-beta the override behavior (for the link in comment 0) places all 8 definitions in the single result set (and has the 2 nsDisplayList uses, but not the 3 nsDisplayOwnLayer uses), whereas the structured analysis changes provides a local view of each symbol and allows/requires manual navigation through the hierarchy via "symbol search this".

The bug 1729164 heuristics proposed would potentially do something more useful than what is currently happening for m-c here (it could walk up the "overrides" relationship and show the top-level uses), but it would still diverge from m-b since we'd never display cousin overrides. A limiting factor continues to be how data is carved up and presented for the existing "search" endpoint which really only knows how to display a flat hierarchy. That said, it might be possible to add a second column to the search results that could express the override hierarchy in a concise tree like the above manually created hierarchy does, including the use counts.

Absent a fancy tree, the heuristics could be tweaked to be much more generous at merging in results from inside the override relationship graph, but a problem with this is that the output would look much more like searching on UpdateScrollData (minus the other false prefix match) because they would be tagged to their actual symbol instead of the transitive closure of all the involved symbols.

:tnikkel, can you elaborate on what you were doing and what you were trying to understand that led to this original discovery? I'd like to try and figure out if there are reasonable heuristics we can use here that provide most of what people are looking for at first, and then if the new "symbol search this" mechanism can help provide the additional things they're looking for and/or other search upsells like "show all override descendants" or something.) Like do you actually want to see all of the related definitions? Or would just the ancestral chain of uses and definitions, plus knowing the list of direct methods that override the currently searched method work?

Comment 2

[Timothy Nikkel (:tnikkel)]

Specifically at that time I wanted to see all callers of UpdateScrollData, the reason for that was I wanted to know how feasible it was to pass a StackingContextHelper to UpdateScrollData.

But also today at a different time I wanted to find all implementations of UpdateScrollData and I happened to be starting from nsDisplayScrollInfoLayer::UpdateScrollData. Usually in this scenario I want all implementations no matter where I start from. But I can see in more complicated scenarios just wanting the implementations that are either an ancestor of descendant of the current one would be good.

Does that answer your question?

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Yes, perfect, thank you!