fix(ios): stops leaking memory on system keyboard rotation

[sgschantz]

fixes #6547

Rotation of the Keyman system in iOS was causing the app to leak memory as the old web view instances used to implement the system keyboard were not being deallocated. This would cause the Keyman keyboard extension to consume more memory and potentially become unstable.

Before this fix, memory consumption increases with each rotation and is not released:

After this fix, memory is quickly reclaimed after many rotations as shown in Xcode:

[keymanapp-test-bot]

User Test Results

Test specification and instructions

• ✅ TEST_SAFARI_DEVELOPER (PASSED): Keyman instances no longer increase in Safari developer with each rotation

Test Artifacts

• iOS
  • Keyman for iOS (simulator image)
  • FirstVoices Keyboards for iOS (simulator image)

[jahorton]

I presume that the keyboard still has to reload after rotation for your testing setup, right? Just... no memory leak now?

[sgschantz]

Yes, the keyboard is still reloaded whenever I rotate, but is now able to clean up the old keyboard view after the rotation.

[mcdurdin]

LGTM

[mcdurdin]

Note: still need to complete user testing before merge.

[sgschantz]

@mcdurdin How do we define user tests to determine whether memory is leaking or not? I could only do so with development tools.

[mcdurdin]

Re user tests, I'm looking more at regression tests -- making sure there are no unexpected functionality breakages -- in PRs like this one.

[jahorton]

Investigation of #6191 has determined another circumstance that may cause the same behavior that leads to memory leaks. I'd strongly advise checking these changes against the experiments documented in this comment: #6191 (comment).

Additionally, there's an interesting WebView callback documented there that may be worth some consideration.

---

A question: does this anti-leak fix prevent replication of the WebView in Safari's Develop menu? That is...

->

If it does prevent the old instance from being preserved, that sounds like a potential basis for user tests. They'd be a bit technical, and would require setup (b/c Safari's Develop menu), but it'd at least be a viable approach.

[mcdurdin]

They'd be a bit technical, and would require setup (b/c Safari's Develop menu), but it'd at least be a viable approach.

There's no reason why we can't get a dev to run a test -- there's no rule saying only testers can run tests.

So I think in this case, one of the dev team should test the built artifact (that is, @sgschantz or @jahorton most likely). I think it'll be a quick and straightforward test for you.

That's aside from the regression test which is still important for the test team to run.

[sgschantz]

User Testing

• TEST_SAFARI_DEVELOPER: Debug Keyman on a Mac with Safari and rotate the device with the system keyboard open in the iOS Safari browser. Verify that multiple instances of Keyman do not appear in the Develop menu on the Mac.

[sgschantz]

I tested to see whether Keyman instances continue to be added to the Safari Developer menu, and they are being cleaned up. However, two appear when SWKeyboard is first attached. After rotating, it jumps to three, but it drops back down to two again within seconds. This happens for each rotation.

Not sure why I would have two entries for Keyman instead of one. That could still be a problem, but the leak appears to be gone.

My User Test Results are not appearing as I would expect them to...did I fail to specify them correctly?

[mcdurdin]

I tested to see whether Keyman instances continue to be added to the Safari Developer menu, and they are being cleaned up. However, two appear when SWKeyboard is first attached. After rotating, it jumps to three, but it drops back down to two again within seconds. This happens for each rotation.

Not sure why I would have two entries for Keyman instead of one. That could still be a problem, but the leak appears to be gone.

Is one the in-app and the other the system keyboard perhaps?

My User Test Results are not appearing as I would expect them to...did I fail to specify them correctly?

Looks like you specified correctly. This was due to an issue on keymanapp/status.keyman.com#253, now resolved.

[sgschantz]

• TEST_SAFARI_DEVELOPER (PASSED): Keyman instances no longer increase in Safari developer with each rotation

[keyman-server]

Changes in this pull request will be available for download in Keyman version 15.0.251-beta