Race condition when updating addon overriding New tab page

During addon update, Firefox will close all tabs of that addon.
And if user has only one tab opened - with that addon - this will close Firefox.

This can easily happen for addons overriding New tab page, because if you open Firefox in the morning, it’s likely it will have only that tab opened, and Firefox is likely to check for updates.

This has two issues:

  • user confusion - “did my Firefox just crashed???”
  • addon post-update operation can be interrupted!
    For example, async migration scripts which run in the browser.runtime.onInstalled handler. But likely also IndexedDB onupgradeneeded upgrades!

Solutions?

  1. report to bugzilla to change behavior if addon update would cause closing Firefox
  2. use browser.runtime.onUpdateAvailable to stop the update - but I have a bad feeling about this, updating addon during browser start sounds like extra race conditions, especially if the browser tries to open “New tab” page, which is being updated. But maybe I’m wrong, if browser simply loads new version, which actually sounds possible and quite OK…

Oh! That’s a curious problem. I took a quick look at bugzil.la and couldn’t find any bug reports that cover the behavior you described; please file bug reports.

I don’t know for sure, but your concern about race conditions when updating an extension and loading about:newtab pages at browser startup strikes me as reasonable. I’ll see if I can find a more concrete answer.

1 Like

Just chatted with one of the engineers about the case where an extension is updated at browser startup time. They said such updates occur at browser startup and that this process blocks restoring a previous Firefox session for the profile. Also, offhand they totally sure whether about:newtab would use the default New Tab page or the extension’s in this scenario.

They weren’t sure offhand how new tab pages behave across browser restarts. I just did some testing and it seems new tab pages provided by extensions are not retained across browser restarts let alone the update scenario we were discussing. For comparison, my about:newtab tab was restored.

The closest existing ticket I was able to find was Bug 1446912: Custom new tab pages make “restore closed tab” less good. Interestingly, that issue describes a similar but different inconsistency between the default new tab page.

1 Like

Extension tabs won’t close if they were discarded before an extension update. So one easy solution after preventing the update is to create a new active tab with an about:blank URL in each window that a Group Speed Dial tab is active, discard all Group Speed Dial tabs, and then perform the update.

For some reason I thought executeScript wouldn’t work on tabs with about:blank URLs (tabs.executeScript() - Mozilla | MDN), but I just tested it and it does. So you could leave a message that your extension is updating in the about:blank tab.

Another solution would be to update the URL in any active Group Speed Dial tab to about:blank. Then inject a message about the extension updating and that the user should wait a moment before pressing the back arrow. Then update your extension. You could even save the tab ids to local storage and run the tabs.goBack method on them when the extension restarts. If you try this though, I believe tabs.update is bugged (1950028 - Previous URL lost in tabs history after using tabs.update to update URL.) and you’d need use window.location.href to update to the about:blank page.

I forgot to mention that if an update is delayed until the browser restarts, there will be no problems on restart with an extensions new tab page even when it is the only page that loads on startup. However, runtime.onInstalled will not fire for the extension after the browser restarts. If you normally run code there, you may need to work around that. Note, when an update is delayed, the extension may still be updated in the about:addons page as there will be an ‘Update Now’ button next to it. If a user presses that button, all the non-discarded extension tabs will then be closed. Another issue is that people often leave their browser running for long periods and if an update has a bug fix, that could be a drawback to not updating quickly.

There are some additional notes / issues to metion :upside_down_face: .

  1. updating extension using JS while it runs is buggy - you need to call runtime.reload(), which internally uses - disable + enable your extension, and this resets all approved warning messages:
    1462266 - "Your New Tab has changed" message re-appears after calling `browser.runtime.reload()`
    So after update user would had to re-approve “New tabs has changed”, “Your homepage has changed”, “Addon is hiding tabs”, that’s 3 separate warnings, each with a big “Disable addon” button :smiley: .

  2. I’m not that much worried about users with session restore enabled, because - again - Firefox is buggy and it seems to restore tabs before extensions are loaded, so usually none of my extension pages are restored (no matter if they override new tab page or not)

  3. what worries me are users without session restore - because when you start a browser, you are “forced” to see a “New tab” page (well, technically it’s Homepage, but my addon overrides both).
    And Firefox was buggy here too, since it starts without waiting for extensions to load. But it was fixed for this case, so it waits for the extension that overrides New tab page to load.
    Actually, I’m pretty sure this worked too well, and it could actually load the addon page before the background script finished some async initialization.
    Oh, what a memories… I learned so many lessons about race conditions back then :smiley: .

Next time I need to run some migrations during upgrade, I’ll use the onUpdateAvailable to delay it till restart.

PS:
New tab page is pretty buggy also in Chrome, all API that are blocked on “chrome://newtab” are blocked also on extension page if it overrides New tab page. So I can’t even take a screenshot of it:

1 Like

As far as I know, the reason here is slightly different - when you tell Firefox to shutdown (or restart), it unloads all running extensions and at that point closes the tabs that belong to them (see bug 1742771) if that tab has used any WebExtensions APIs (not doing that if it has not is a separate bug). One funny consequence is that they’re closed after browser restart (or shutdown/startup) except if the browser crashed as in that case it hasn’t had a chance to close them.

1 Like

So one easy solution after preventing the update is to create a new active tab with an about:blank URL in each window that a Group Speed Dial tab is active, discard all Group Speed Dial tabs, and then perform the update.

Huh, that’s clever.

For some reason I thought executeScript wouldn’t work on tabs with about:blank URLs (tabs.executeScript() - Mozilla | MDN), but I just tested it and it does.

I looked into this last week and this behavior is inconsistent across browsers. Chrome does not allow injection into about:blank tabs unless they are created by an another page that you have host permissions for via window.create() :face_with_spiral_eyes:

I forgot to mention that if an update is delayed until the browser restarts, there will be no problems on restart with an extensions new tab page even when it is the only page that loads on startup.

I didn’t test this exact scenario. Last week I looked into how new tab pages provided by two popular new tab page extensions behaved across Firefox session with session restore enabled. In my setup I had a single window and one or more new tab pages created by pressing Cmd+T. I observed that Firefox would close one of the extension’s new tab pages each time I quit and re-launched Firefox. I don’t think I tested a scenario where the new tab page was the only tab open before quitting and starting again.

@juraj.masiar, @tanriol, @kg_13, could you open bugzil.la tickets for the various issues you’ve discussed in this thread? I feel like there’s enough subtlety that I may not do a good job of capturing everything that you all have shared.

I’ve created report for the main issue:

1 Like