Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve CozyWebview and CozyIntent lifecycle #73

Open
Ldoppea opened this issue Jan 24, 2022 · 1 comment
Open

Improve CozyWebview and CozyIntent lifecycle #73

Ldoppea opened this issue Jan 24, 2022 · 1 comment

Comments

@Ldoppea
Copy link
Member

Ldoppea commented Jan 24, 2022

This issue's goal is to keep track of how we handle CozyWebviews lifecycle and how they are registered into CozyIntent

We want to ensure that all CozyWebviews are correctly registered into CozyIntent when created and unregistered when destroy


Current implementation does the following:

  • When a CozyWebview is created, it registers itself to CozyIntent
  • CozyIntent checks the CozyWebview's hostname in the registration process
  • if this hostname already exists, an early return is done and the CozyWebview is not registered

This behavior is coded in cozy-intent/src/api/services/NativeService.ts:

  public registerWebview = (webviewRef: WebviewRef): void => {
    const uri = StaticService.getUri(webviewRef)

    if (this.messengerRegister[uri]) throw new Error('LOREM IPSUM')

    this.messengerRegister = {
      ...this.messengerRegister,
      [uri]: { messenger: new this.messengerService(webviewRef) }
    }
  }

This behaviour may be problematic as a silent fail may break the app communication without any message or log to understand what is happening

Also we except this situation to never happen, but we don't have enough knowledge yet about all inter-app redirections that may occur in the app lifecycle. So the probability of having 2 webviews with same hostname should not be considered as null.

Some improvements have been considered:

  • Remove the if/return so instead of an early return we replace the old registered webview by a new one
    • This may have the same effect than previous behaviour but instead of breaking the new webview it would be the old one
    • However this event seems to have a lower probability (more probable scenario is that a component did not unregister successfully when destroyed)
  • Replace the if/return by a if/throw
    • This would not fix potential problems, but this would prevent having an instable app -> it is preferable to break it with an explicit error
  • Implement a mechanism that cleans the registration state and starts a new handshake process when a duplicate ID is detected
    • This would enforce a clean state and reduce probability of memory leak/ghost listeners

For now we chose to implement the if/throw option in order to verify if this scenario happens and how often.
The objective is to implement the clean state mechanism later when confident enough to implement it.

@acezard
Copy link
Contributor

acezard commented Feb 7, 2023

@Ldoppea do you think this issue is still relevant? (I'm available to talk about it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants