Closed Bug 462549 Opened 13 years ago Closed 13 years ago

Verify that an appcache manifest hasn't changed at the end of an update

Categories

(Core :: Networking: Cache, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dcamp, Assigned: dcamp)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch v1 (obsolete) — Splinter Review
The offline spec requires that at the end of a successful cache update, we re-download the manifest to make sure it hasn't changed.  This is to prevent us from grabbing a partial snapshot of the app during a server push.
Attachment #345762 - Flags: superreview?(cbiesinger)
Attachment #345762 - Flags: review?(cbiesinger)
Attachment #345762 - Attachment is patch: true
Attachment #345762 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 345762 [details] [diff] [review]
v1

+NS_IMPL_ISUPPORTS4(nsManifestCheck,
+                   nsIRequestObserver,
+                   nsIStreamListener,
+                   nsIChannelEventSink,
+                   nsIInterfaceRequestor);

shouldn't put a semicolon at the end here, some compilers don't like that

+    nsCString foo;
+    foo.Assign(aFromSegment, aCount);

this seems unused?

+    PRUint32 bytesRead = 0;
+    aStream->ReadSegments(ReadManifest, this, aCount, &bytesRead);

you don't have to initialize bytesRead

+    // Redirects should cause the load (and therefore the update) to fail.
+    return NS_ERROR_FAILURE;

you have to allow REDIRECT_INTERNAL redirects here
Attachment #345762 - Flags: superreview?(cbiesinger)
Attachment #345762 - Flags: superreview+
Attachment #345762 - Flags: review?(cbiesinger)
Attachment #345762 - Flags: review+
Attached patch patch as landed (obsolete) — Splinter Review
Includes the review fixes and properly includes the test file.
Attachment #345762 - Attachment is obsolete: true
Attached patch patch as landedSplinter Review
err, this was the new patch.
Attachment #346774 - Attachment is obsolete: true
Landed as http://hg.mozilla.org/mozilla-central/rev/037702607ad8
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.