ReviewOS

craft-native/craft

fix(ios): an answer to a call made before a page reload can settle a different call after it

#226
Closed glennmichael123 opened this 10 days ago ·
10 days ago

Every iOS page call is identified by cb_<n>, and <n> restarts at 0 whenever the page script is injected again: a reload, a navigation, or webViewWebContentProcessDidTerminate reloading after a web-content crash (window.craft = { … _callbackId: 0 … } in CraftApp.swift). Zig carries the same number as its request id.

Native keeps no record of which page load a call came from. So an answer to a call made before a reload is delivered to whichever call on the new page drew the same number:

  1. The page calls something slow as cb_7, for example craft.siri.remove(), or getCurrentPosition with a long timeout.
  2. The page reloads before it is answered.
  3. The new page's seventh call is also cb_7, perhaps getDeviceInfo().
  4. The first call's answer arrives and settles the getDeviceInfo promise with a Siri or location result, or rejects it with that call's TIMEOUT.

Where it can happen

  • Any late completion on either runtime. It always could.
  • The native deadlines: Swift's getCurrentPosition timeout, and since #211 the removeSiriShortcut deadline on both runtimes. A deadline turns "a completion that never comes" into an answer that always arrives, so after a reload it reliably misfires where it previously never did. That is why this is filed alongside #211 rather than fixed piecemeal inside it.

Suggested fix

Make ids unique across page loads, not just within one. The cheapest version keeps everything numeric, because Zig parses cb_<n>: start _callbackId at a random or time-derived offset for each injection, so a new page cannot draw an id an old call still holds. More robust: include a page-load generation in each call, and have native drop answers whose generation is stale. On the Swift side, didStartProvisionalNavigation is a natural place to cancel pending deadline work items.

Sign in to comment on this issue.