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

fix: fix tab manager initialization race condition #1492

Merged
merged 1 commit into from
Dec 6, 2023
Merged

Conversation

birtles
Copy link
Member

@birtles birtles commented Dec 6, 2023

STR:

  1. Start the add-on but don't enable it
  2. Wait for the download to finish
  3. Terminate the background process from about:debugging (or just wait
    for it to terminate)
  4. On any regular Web page, enable the add-on by clicking the icon on the
    toolbar

Expected results:

The add-on is enabled immediately.

Actual results:

It takes two clicks.

Analysis:

We'd hit a race condition where we'd call tabManager.init() while the
tabManager was still being initialized. As a result it would update its
this.enabled member to true but then the initialization steps would
reset it back to false.

The fix is just to wait for the initialization to complete before trying
to toggle the value.

As far as I can tell this does not affect the ActiveTabManager because
its initialization is synchronous.

Fixes #1491.

STR:

1. Start the add-on but don't enable it
2. Wait for the download to finish
3. Terminate the background process from about:debugging (or just wait
   for it to terminate)
4. On any regular Web page, enable the add-on by clicking the icon on the
   toolbar

Expected results:

The add-on is enabled immediately.

Actual results:

It takes two clicks.

Analysis:

We'd hit a race condition where we'd call tabManager.init() while the
tabManager was still being initialized. As a result it would update its
this.enabled member to `true` but then the initialization steps would
reset it back to false.

The fix is just to wait for the initialization to complete before trying
to toggle the value.

As far as I can tell this does not affect the ActiveTabManager because
its initialization is synchronous.

Fixes #1491.
@birtles birtles enabled auto-merge (rebase) December 6, 2023 06:41
@birtles birtles merged commit 9f8644c into main Dec 6, 2023
1 check passed
@birtles birtles deleted the tab-manager-init branch December 6, 2023 06:44
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

Successfully merging this pull request may close these issues.

Clicking browser action icon sometimes takes two attempts
1 participant