The main goal of these changes is to ensure we're not doing any unnecessary work
in the unahppy cases of BatchingUploader. We might fail in three general ways:
- encounter a 412 error
- encounter another type of HTTP error
- encounter a GUID in the "failed" array
Currently, in all of these cases, we de-facto abort the session, without performing
an actual abort. E.g. we won't commit a batch, we'll refuse to upload any still-flowing
records. This patch simplifies our unhappy-case behaviour: if something failed, actually
abort the session (triggering a shutdownNow of the work queues), declare store as failed, etc.
It's important to note that our "did the synchronization fail?" login in the SynchronizerSession
depends on the store failure counts, and so this patch maintains the "record failed to store"
delegate chain. However, these counts are largely meaningless. What does it mean to fail to store
50 records, if we abort on the 51st, and prevent the other 100 from flowing (and from being counted
as failed?).
This patch also fixes an omission in the verstion tracking logic:
- prior, if we encountered a record in the "failed" array, we'd continue on with the flow, won't upload
anything, mark the synchronization as failed, but we'd also call into 'onStoreCompleted' which will
trigger an update of syncVersion for outflowing records
- with this patch, we won't call into onStoreCompleted in the case above, and so won't update syncVersion
in case of such failures
- this is the correct behaviour for batching uploads (now enabled on all but one server), but possibly
non-optimal behaviour if batching isn't enabled. However, this behaviour should be safe from a data consistency
point of view regardless of the batching mode.
MozReview-Commit-ID: LIYCPaRX8JA
--HG--
extra : rebase_source : 110224b2db85a383635db933ec6c19b21af886e7
We currently allow the content process to shutdown the IPDL objects on
behalf the parent, and we wait for all of these instances to be freed
before we complete shutdown. This is undesirable because it requires the
parent to trust the child rather than the other way around; the child
can hold shutdown hostage by simply not releasing its instances. The
child should already support the parent closing its graphics IPDL
objects because the GPU process itself can die abruptly and be restored
at a later time.
This is a typo bug of Bug 1053779 Part 2. ConvertListType might return NS_OK
even if ReplaceContainer doesn't return valid value.
So, to clean up code, we should return Element instead of nsresult since out
parameter of this function is Element only.
MozReview-Commit-ID: 44UHETzcdGy
--HG--
extra : rebase_source : ab76486505cb4e7caea3fb99e11fccd606878f02
See commits for details.
Source-Repo: https://github.com/servo/servo
Source-Revision: 4450bbdb0c44637a3caa05bd14a6e2b9d867a74e
--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : f97eac4495b2898422b8648f9dc4a3c15ddc9e76
