OpenBSD, uniquely (as far as I know) of our Unix targets, doesn't
support the function `waitid`, which means that the fix for bug 1793523
broke the build. However, OpenBSD isn't affected by bug 1793523 because
the crash reporter isn't supported, so it can continue to use `waitpid`.
This patch restores the `waitpid` version of `IsProcessDead` under an
ifdef. I've tested it locally on Linux (by changing the ifdef), and as
expected it does exhibit bug 1793523 but otherwise seems to work, so it
should work on OpenBSD.
Differential Revision: https://phabricator.services.mozilla.com/D160113
`AppUpdater.stop()` is not currently well tested, and the new implementation relies on throwing exceptions. It's important that these exceptions be properly caught to ensure that we don't end up handling the exception as an internal error or not handling it properly at all.
Differential Revision: https://phabricator.services.mozilla.com/D159308
We should be careful about calling `cleanupDownloadingUpdate`. We currently call it sometimes when `AUS.downloadUpdate` fails without checking whether we are already downloading an update. We don't want to mess up a download that is already in-progress just because a subsequent call to `downloadUpdate` failed.
It may have been necessary in the past to clean up if `downloadUpdate` failed in order to prevent leaving bits of downloading update state around. But now we don't set that state until the download has actually started, so some of those cleanup instances just aren't necessary anymore. We only really need to clean up if we might be resuming a download, in which case those bits of state will be lying around and ought to be cleaned up on failure.
Differential Revision: https://phabricator.services.mozilla.com/D159305
The Update Service stores information about the current update state in the status file and in the state properties of both downloadingUpdate and readyUpdate. It's important that we log changes to this information in order to better be able to debug tests and user issues.
Differential Revision: https://phabricator.services.mozilla.com/D159304
Also change some of the implementation of UpdateService to use the new currentState API.
This patch also fixes and cleans up a few things that are only tangentially related. Most notably, `_postUpdateProcessing` does a better job of handling the situation where Firefox starts while updating/staging is still in-progress. And `refreshUpdateStatus` does a better job of handling unexpected error situations like the updater binary crashing.
Differential Revision: https://phabricator.services.mozilla.com/D159303
Note that, for the most part, this isn't meant to change behavior. It is just meant to eliminate race conditions and fix some bugs. However, a tiny bit of behavior has been changed.
Previously, the fallback error codes used to potentially populate nsIUpdate.statusText were 200 in Checker.onError (corresponding to "Update XML file malformed") and 404 in Checker.onLoad (corresponding to "Update XML file not found"). These really seem backwards to me. Especially in the Checker.onLoad case, where we basically use that as the fallback if we fail to parse the update XML. So the codes have effectively been reversed in this patch.
Differential Revision: https://phabricator.services.mozilla.com/D159302
In a previous patch in this stack, AppUpdater was changed to remove the checkForUpdates function. This change updates aboutDialog-appUpdater to accomodate this change.
It was probably incorrect for this function to be called previously. Calling into checkForUpdates() is functionally similar to calling into check(), except that checks are bypassed that probably shouldn't be bypassed.
Differential Revision: https://phabricator.services.mozilla.com/D159299
This ended up being a nearly total rewrite of AppUpdater, but I think that the changes are worth it.
The flow is much more linear which makes it easier to follow and reason about.
The API is made much more formal by making most methods and members private.
Many potential race conditions have been removed.
Many more potential errors thrown can now be caught by the try/catch that wraps `AppUpdater.check`, causing those errors to show the "Internal Error" message rather than causing the update interface to just freeze.
Cancelling an in-progress update check works more reliably.
This patch does not change AppUpdater consumers to accomodate these changes. That will come later in the patch stack.
Differential Revision: https://phabricator.services.mozilla.com/D159298
This patch updates all consumers of nsIApplicationUpdateService.downloadUpdate to accomodate the changes made to that API earlier in the stack.
It also updates the implementation to match that API.
Differential Revision: https://phabricator.services.mozilla.com/D159296
This patch misses one notable nsIUpdateChecker consumer: AppUpdater. This patch stack makes major changes to AppUpdater, so those changes will be made in their own patch later in this patch stack.
Differential Revision: https://phabricator.services.mozilla.com/D159295
Broadly speaking, this patch makes 4 API changes:
1. nsIUpdateChecker is substantially changed to accomodate (a) it being a singleton service that can potentially make multiple update checks at once, and (b) communicating the results of the update check via JS promises rather than callback functions.
2. nsIApplicationUpdateService has had functionality added to allow consumers to check the current update state.
3. nsIApplicationUpdateService now exposes disabledByPolicy, which was already being used externally in an inappropriate way.
4. nsIApplicationUpdateService.downloadUpdate is made asynchronous and has its (long-unnecessary) background argument removed
The codebase previously seemed a bit mixed up at times regarding whether the update checker was a singleton or whether a new one would be constructed every time it was needed. This patch makes the answer more definitive by definitively making it a singleton service.
Note that this patch changes the interface definitions only. The changes to API consumers and implementation will come later in this patch stack.
Differential Revision: https://phabricator.services.mozilla.com/D159294
I'm surprised to see that these weren't already marked as singletons. We definitely should never make duplicates of them.
Differential Revision: https://phabricator.services.mozilla.com/D159293
It's causing bug 1794237. Rather than disable it altogether, let's
just hold it back to nightly. Hopefully that way we'll eventually get
a bug report with a URL to help us actually fix the crash.
Differential Revision: https://phabricator.services.mozilla.com/D160253
If we have stroked paths whose bounds cover a lot of screen area, that can lead
to a lot of empty area in the interior that bloats the path cache textures up
with unused pixels that still need to be uploaded. Try to avoid this by not
trying to accelerate paths with the path cache that take up a large amount
of screen area.
Differential Revision: https://phabricator.services.mozilla.com/D160023
Following a gtest timeout, signal the process to generate a minidump, then
dump the crash report to the test log, just like the existing timeout
handling for mochitest, reftest, etc.
This makes no changes for Android; it was already working.
Differential Revision: https://phabricator.services.mozilla.com/D160249
