Browser: tab transitions, state update queue

[christianbaroni]

What changed (plus any additional context for devs)

• This PR adds tab transitions when closing or opening tabs and introduces a reanimated-based orchestration layer between the browser UI and the tabStates data, which allows for updates to tabStates to apply successfully and in the correct order the majority of the time, and I think with a bit more work it could be every time. Currently it's scoped to state updates related to closing and opening tabs. Here's what this orchestration layer is doing:

  • It maintains an always-up-to-date shared value array of the currently open tab IDs in currentlyOpenTabIds, which is updated at the exact moment a tab creation or close operation is initiated. This shared value is only ever read and set from worklets, and the positions of the IDs in the array correspond to the tab indexes.
  • That array is then used in conjunction with a blocking queue + useAnimatedReaction to apply the operations to tabStates via runOnJS, avoiding most race conditions and issues caused by stale tabStates data
    • It’s possible we don’t need the queue or could use a simpler mechanism depending on how tabStates evolves/if we swap out the current MMKV setup
  • This also enables tab view transitions (the original idea) — to accomplish that without jank we need access to the tabStates data much earlier than it’s available via tabStates. So now the tabs read their animated indexes from currentlyOpenTabIds and tab positions smoothly transition in the tab view when tabs are closed, and when new tabs are opened they always originate from the right spot. In the future this can also enable things like dragging to rearrange tabs.

• I also converted closeTab and newTab to worklets, which were already being called in most cases from reanimated/gesture handler worklets

• There are a couple bugs still but it works pretty well. I think currentlyOpenTabIds can become out of sync with tabStates if for some reason an operation fails to complete (maybe possible), or operations apply in a different order in currentlyOpenTabIds vs tabStates (resulting in mismatched tab indexes). There’s no cleanup in place in the event that does happen. If we end up going with this approach it’d be easy to add a cleanup condition and some logging to the useAnimatedReaction that coordinates the operation queue, and we can fix the remaining issues.

• I think a lot of this could move out of the BrowserContext or into hooks, I left it there for simplicity and wanted to get a yay/nay on the idea first — am fine to walk away from this if there’s a better alternative, I was just having fun with it over the weekend

• The logic is a little messy/redundant/complex in places but doesn’t seem to have much negative performance impact if any, and tab closing/opening is seemingly reliable no matter how fast you do it (unless you manage to trigger one of the sync bugs I mentioned, in which case a force quit will resolve it)

Screen recordings / screenshots

RPReplay_Final1712042348.mov

[brunobar79]

The video you posted confirmed this is indeed the best way we can approach this from so I support it. Once it's in we can look into what patterns of the existing codebase could fit, and even if they do we need to confirm it doesn't degrade perf.

Aside from that my main question is: Why do we need a queue? Opening many tabs too fast? or closing tabs to fast?

If that's the case, for simplicity purposes, avoiding bugs / race conditions, I think we could simplify this by disabling the buttons (new tab / close tab) for ~ 1 second after interacting with them. While I do think this is really cool seems like overkill for the basic open / close tabs scenarios.

If the reason we have the queue is different, then might be acceptable to keep it.

[christianbaroni]

Bruno and I chatted and decided the queue makes sense given the speed at which tab closing can happen.

I think at least one of the sync bugs was caused by code I’d already commented out, haven’t been able to break it so going to mark this as ready for review and we can move on from this but keep an eye out for issues (very well may exist but should be easily addressable once found). We can circle back to making further improvements later if needed.

Will catch this up with develop in a bit and add in some error logging to catch any failed operations or mismatches between currentlyOpenTabIds and tabStates that don’t get successfully cleared.

[brunobar79]

🔥 🔥 🔥