Closed Bug 383716 Opened 15 years ago Closed 14 years ago
Web Browser Persist::On Stop Request doesn't check |status| for failure
It's possible to get a status failure OnStop, but |status| is only used when passing to |mProgressListener|. Perhaps do something similar to exthandler's OnStop? http://mxr.mozilla.org/mozilla/source/uriloader/exthandler/nsExternalHelperAppService.cpp#2129 If there is a failure, a the |mProgressListener| will get a StatusChange with the status and error string; otherwise, it'll get a StateChange (stop) like usual. Question is.. how many failures are actually arriving OnStop that didn't appear OnDataAvail.
This is necessary for the download manager to get the Link Fingerprint failure status as a StateChange.
- Fix nits (comments, void)
Edward, you'll request reviews once you're happy with the patch, right?
Comment on attachment 269539 [details] [diff] [review] v2 - I think it makes more sense to call EndDownload instead of Cancel - don't cast to void - you need to continue in this function, because Cancel does not set mEventSink etc to null
(In reply to comment #4) > - you need to continue in this function, because Cancel does not set mEventSink > etc to null Done. Instead of returning, it'll reuse the existing code to |delete data| which will get |completed| set to true allowing us to... > - I think it makes more sense to call EndDownload instead of Cancel ... reuse the EndDownload + nulling. Done. > - don't cast to void Removed.
Check if we haven't gotten a failure Result instead of checking if we haven't been canceled yet.
14 years ago
Comment on attachment 273208 [details] [diff] [review] v4 Seems reasonable.
Attachment #273208 - Flags: superreview?(bzbarsky) → superreview+
Checking in embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp; /cvsroot/mozilla/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp,v <-- nsWebBrowserPersist.cpp new revision: 1.130; previous revision: 1.129 done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta1
You need to log in before you can comment on or make changes to this bug.