Move stackIndex existence check at if condition in SampleTooltipContents

[theoniko]

Solves #5290

[canova]

Thanks for the PR! It looks like the logic needs to be changed, see my comment below.

Also see my comment #5290 (comment) to how to reproduce it locally, so you can test it.

[canova]

|| needs to be && here. We need to make sure that stack index is not null AND not undefined. (also there is an extra space on the right side of the or operator).

[mstange]

I'm not sure about this patch. If sampleIndex can be an invalid index, it means there's a bug somewhere else.

[julienw]

I'm not sure about this patch. If sampleIndex can be an invalid index, it means there's a bug somewhere else.

yeah, null could happen when it's filtered out (this is what @canova hits with this STR), but undefined as in the original bug, i'm not so sure. Even null IMO shouldn't happen because we should not show the tooltip at all and therefore not render the component in the first place... therefore the condition might need to be in the SampleGraph itself for Nazim's case.

[canova]

Yeah, for the SampleGraph case, we were passing the filteredThread initially directly because they get removed once you drop samples etc. That time it made sense to just get the filtered thread and use that since we don't need the unfiltered one / range filtered one.

There is this case now:

1. You hover over the sample in the sample graph and show the tooltip.
2. You don't move the mouse so the hovered sample doesn't get updated.
3. You drop the sample.

So in this case we still have the same hoveredSample in the sample graph. And we only check for the sample itself being null or not in the SampleGraph case. We might want to pass only the rangeFilteredThread to the SampleTooltipContents, but i don't think it's a great solution either as we will still continue to see the removed sample tooltip when we drop samples.
So SampleGraph solution might be to check the stack itself and add the tooltip only if it's not null during its render.

Even null IMO shouldn't happen because we should not show the tooltip at all and therefore not render the component in the first place...

We actually show the tooltip in case the sample is not there, but this case is slightly different. For example when you profile without the stack sampling, but you still capture the CPU usage, we will not sample; but every sample will only contain the CPU usage values. You can try this case locally by checking the No Periodic Sampling option.

---

For the undefined case, it's really weird and probably coming from somewhere else. Not so sure where it comes from.