311266 - Fix for bug 306961 was too aggressive, causes partial update to keep failing in some cases where a full update should be downloaded

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Darin Fisher]

Fix for bug 306961 was too aggressive.  I chose to treat any I/O error as a
temporary error, but updater.cpp generates an I/O error if, for example, the old
file being patched doesn't exist.  In that case, the update service should give
up trying to apply the partial patch and failover to downloading the complete
archive.  The fix, I think, is to adjust the error codes generated by
updater.cpp when it fails to apply a patch.  It might make sense to distinguish
write errors from read errors to solve this bug.

Comment 1

[Scott MacGregor]

Darin/Chase and I think we need to block on this bug.

Comment 2

[Darin Fisher]

Attachment: v1 patch

Comment 3

[Darin Fisher]

When we hit a failure to apply a partial update, there are two paths we may
take.  First, we may decide that the failure is intermittent (see bug 306961),
in which case we want to leave the patch as is to be applied later.  Otherwise,
we decide to skip the partial patch and proceed to download a complete patch.

I've tested the following error cases on Windows and Linux:

1) Attempt partial patch, but one of the source files cannot be modified.
2) Attempt partial patch, but one of the source files does not exist.
3) Attempt partial patch, but one of the source directories does not exist. 
(variant on #2)
4) Attempt partial patch, but one of the source files does not contain the data
expected by the patch.

With this patch, testcase #1 results in Firefox prompting the user to "correct
the problem and retry."  In the other testcases, Firefox reverts to downloading
the complete patch.

Without this patch, testcases #2 and #3 are not handled properly.

The patch for this bug looks large, but it is actually pretty straightforward. 
I'm basically just replacing IO_ERROR with READ_ERROR or WRITE_ERROR to better
describe the specific failure.  Then, I only invoke the "retry" prompt when we
encounter a WRITE_ERROR.  As part of this patch I also got rid of the BSP_ error
codes in favor of the ones defined in errors.h.

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 198645 [details] [diff] [review]
v1 patch

r=[e-mail]

Comment 5

[Scott MacGregor]

Comment on attachment 198645 [details] [diff] [review]
v1 patch

we're taking this now but we still need benjamin to sign off on the patch.
Please review this ASAP Benjamin.

If he finds any issues we'll have to consider another respin again, but we'd
rather get builds we can start testing with instead of blocking the respin
until we can find benjamin.

Comment 6

[Darin Fisher]

fixed-on-trunk, fixed1.8