Fix bug 314803: fix crash on download failure, caused by DownloadDone() getting called twice. Added error reporting for common downloading errors (no permissions in destination dir, and disk full) via a sheet on the downloads window.

Implemented nsIInterfaceRequestor on nsDownloadListener and nsHeaderSniffer because I was seeing assertions about this, and to avoid possible uninitialized value usage (bug 315246).

camino/src/download/ProgressDlgController.h

@@ -103,7 +103,7 @@
 -(int)numDownloadsInProgress;
 -(void)clearAllDownloads;
 -(void)didStartDownload:(id <CHDownloadProgressDisplay>)progressDisplay;
--(void)didEndDownload:(id <CHDownloadProgressDisplay>)progressDisplay withSuccess:(BOOL)completedOK;
+-(void)didEndDownload:(id <CHDownloadProgressDisplay>)progressDisplay withSuccess:(BOOL)completedOK statusCode:(nsresult)status;
 -(void)removeDownload:(id <CHDownloadProgressDisplay>)progressDisplay;
 -(NSApplicationTerminateReply)allowTerminate;
 -(void)saveProgressViewControllers;

camino/src/download/ProgressDlgController.mm

@@ -38,6 +38,9 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
+#include "nsNetError.h"
+
+#import "NSString+Utils.h"
 #import "NSView+Utils.h"
 
 #import "ProgressDlgController.h"
@@ -49,6 +52,7 @@ static NSString* const kProgressWindowFrameSaveName = @"ProgressWindow";
 
 @interface ProgressDlgController(PrivateProgressDlgController)
 
+-(void)showErrorSheetForDownload:(id <CHDownloadProgressDisplay>)progressDisplay withStatus:(nsresult)inStatus;
 -(void)rebuildViews;
 -(NSMutableArray*)getSelectedProgressViewControllers;
 -(void)deselectDLInstancesInArray:(NSArray*)instances;
@@ -480,7 +484,7 @@ static id gSharedProgressController = nil;
   [self makeDLInstanceVisibleIfItsNotAlready:progressDisplay];
 }
 
--(void)didEndDownload:(id <CHDownloadProgressDisplay>)progressDisplay withSuccess:(BOOL)completedOK
+-(void)didEndDownload:(id <CHDownloadProgressDisplay>)progressDisplay withSuccess:(BOOL)completedOK statusCode:(nsresult)status
 {
   [self rebuildViews]; // to swap in the completed view
   [[[self window] toolbar] validateVisibleItems]; // force update which doesn't always happen
@@ -496,10 +500,68 @@ static id gSharedProgressController = nil;
                       // for the option key and try to close all windows
     }
   }
+  else if (NS_FAILED(status) && status != NS_BINDING_ABORTED)  // if it's an error, and not just canceled, show sheet
+  {
+    [self showErrorSheetForDownload:progressDisplay withStatus:status];
+  }
   
   [self saveProgressViewControllers];
 }
 
+-(void)showErrorSheetForDownload:(id <CHDownloadProgressDisplay>)progressDisplay withStatus:(nsresult)inStatus
+{
+  NSString* errorMsgFmt = NSLocalizedString(@"DownloadErrorMsgFmt", @"");
+  NSString* errorExplString = nil;
+  
+  NSString* destFilePath = [progressDisplay destinationPath];
+  NSString* fileName = [destFilePath displayNameOfLastPathComponent];
+  
+  NSString* errorMsg = [NSString stringWithFormat:errorMsgFmt, fileName];
+
+  switch (inStatus)
+  {
+    case NS_ERROR_FILE_DISK_FULL:
+    case NS_ERROR_FILE_NO_DEVICE_SPACE:
+      {
+        NSString* fmtString = NSLocalizedString(@"DownloadErrorNoDiskSpaceOnVolumeFmt", @"");
+        errorExplString = [NSString stringWithFormat:fmtString, [destFilePath volumeNamePathComponent]];
+      }
+      break;
+      
+    case NS_ERROR_FILE_ACCESS_DENIED:
+      {
+        NSString* fmtString = NSLocalizedString(@"DownloadErrorDestinationWriteProtectedFmt", @"");
+        NSString* destDirPath = [destFilePath stringByDeletingLastPathComponent];
+        errorExplString = [NSString stringWithFormat:fmtString, [destDirPath displayNameOfLastPathComponent]];
+      }
+      break;
+
+    case NS_ERROR_FILE_TOO_BIG:
+    case NS_ERROR_FILE_READ_ONLY:
+    default:
+      {
+        errorExplString = NSLocalizedString(@"DownloadErrorOther", @"");
+        NSLog(@"Download failure code: %X", inStatus);
+      }
+      break;
+  }
+  
+  NSBeginAlertSheet(errorMsg,
+                    nil,    // default button ("OK")
+                    nil,    // alt button (none)
+                    nil,    // other button (nil)
+                    [self window],
+                    self,
+                    @selector(downloadErrorSheetDidEnd:returnCode:contextInfo:),
+                    nil,    // didDismissSelector
+                    NULL,   // context info
+                    errorExplString);
+}
+
+-(void)downloadErrorSheetDidEnd:(NSWindow *)sheet returnCode:(int)returnCode contextInfo:(void *)contextInfo
+{
+}
+
 -(void)removeDownload:(id <CHDownloadProgressDisplay>)progressDisplay
 {
   [mProgressViewControllers removeObject:progressDisplay];

camino/src/download/ProgressViewController.mm

@@ -548,12 +548,12 @@ enum {
   [mProgressBar startAnimation:self];
 }
 
--(void)onEndDownload:(BOOL)completedOK
+- (void)onEndDownload:(BOOL)completedOK statusCode:(nsresult)aStatus
 {
   mDownloadingError = !completedOK;
-  
+
   [self downloadDidEnd];
-  [mProgressWindowController didEndDownload:self withSuccess:completedOK];
+  [mProgressWindowController didEndDownload:self withSuccess:completedOK statusCode:aStatus];
 }
 
 -(void)setProgressTo:(long long)aCurProgress ofMax:(long long)aMaxProgress
@@ -629,6 +629,11 @@ enum {
   //[self tryToSetFinderComments];
 }
 
+- (NSString*)sourceURL
+{
+  return mSourceURL;
+}
+
 -(void)setDestinationPath:(NSString*)aDestPath
 {
   [mDestPath autorelease];
@@ -636,4 +641,9 @@ enum {
   //[self tryToSetFinderComments];
 }
 
+- (NSString*)destinationPath
+{
+  return mDestPath;
+}
+
 @end

camino/src/download/SaveHeaderSniffer.h

@@ -41,6 +41,7 @@
 #import <Appkit/Appkit.h>
 
 #include "nsString.h"
+#include "nsIInterfaceRequestor.h"
 #include "nsIWebProgressListener.h"
 #include "nsIWebBrowserPersist.h"
 #include "nsIURI.h"
@@ -50,7 +51,9 @@
 
 
 // Implementation of a header sniffer class that is used when saving Web pages and images.
-class nsHeaderSniffer : public nsIWebProgressListener
+// NB. GetInterface() for nsIProgressEventSink is called on this class, if we wanted to implement it.
+class nsHeaderSniffer : public nsIInterfaceRequestor,
+                        public nsIWebProgressListener
 {
 public:
     nsHeaderSniffer(nsIWebBrowserPersist* aPersist, nsIFile* aFile, nsIURI* aURL,
@@ -60,6 +63,7 @@ public:
     virtual ~nsHeaderSniffer();
 
     NS_DECL_ISUPPORTS
+    NS_DECL_NSIINTERFACEREQUESTOR
     NS_DECL_NSIWEBPROGRESSLISTENER
   
 protected:

camino/src/download/SaveHeaderSniffer.mm

@@ -79,7 +79,14 @@ nsHeaderSniffer::~nsHeaderSniffer()
 {
 }
 
-NS_IMPL_ISUPPORTS1(nsHeaderSniffer, nsIWebProgressListener)
+NS_IMPL_ISUPPORTS2(nsHeaderSniffer, nsIInterfaceRequestor, nsIWebProgressListener)
+
+// Implementation of nsIInterfaceRequestor
+NS_IMETHODIMP 
+nsHeaderSniffer::GetInterface(const nsIID &aIID, void** aInstancePtr)
+{
+  return QueryInterface(aIID, aInstancePtr);
+}
 
 #pragma mark -
 

camino/src/download/nsDownloadListener.h

@@ -45,6 +45,7 @@
 #import "CHDownloadProgressDisplay.h"
 
 #include "nsString.h"
+#include "nsIInterfaceRequestor.h"
 #include "nsIDownload.h"
 #include "nsIWebBrowserPersist.h"
 #include "nsIURI.h"
@@ -55,8 +56,9 @@
 
 
 // maybe this should replace nsHeaderSniffer too?
-
+// NB. GetInterface() for nsIProgressEventSink is called on this class, if we wanted to implement it.
 class nsDownloadListener :  public CHDownloader,
+                            public nsIInterfaceRequestor,
                             public nsIDownload
 {
 public:
@@ -64,6 +66,7 @@ public:
     virtual ~nsDownloadListener();
 
     NS_DECL_ISUPPORTS_INHERITED
+    NS_DECL_NSIINTERFACEREQUESTOR
     NS_DECL_NSIDOWNLOAD
     NS_DECL_NSITRANSFER
     NS_DECL_NSIWEBPROGRESSLISTENER

camino/src/download/nsDownloadListener.mm

@@ -62,10 +62,19 @@ nsDownloadListener::~nsDownloadListener()
 {
 }
 
-NS_IMPL_ISUPPORTS_INHERITED4(nsDownloadListener, CHDownloader, nsIDownload,
+NS_IMPL_ISUPPORTS_INHERITED5(nsDownloadListener, CHDownloader, 
+                             nsIInterfaceRequestor,
+                             nsIDownload,
                              nsITransfer, nsIWebProgressListener,
                              nsIWebProgressListener2)
 
+// Implementation of nsIInterfaceRequestor
+NS_IMETHODIMP 
+nsDownloadListener::GetInterface(const nsIID &aIID, void** aInstancePtr)
+{
+  return QueryInterface(aIID, aInstancePtr);
+}
+
 #pragma mark -
 
 /* void init (in nsIURI aSource, in nsIURI aTarget, in AString aDisplayName, in wstring openingWith, in long long startTime, in nsICancelable aCancelable); */
@@ -238,7 +247,7 @@ nsDownloadListener::OnStatusChange(nsIWebProgress *aWebProgress,
   // aMessage contains an error string, but it's so crappy that we don't want to use it.
   if (NS_FAILED(aStatus))
     DownloadDone(aStatus);
-  
+
   return NS_OK;
 }
 
@@ -359,14 +368,15 @@ nsDownloadListener::DownloadDone(nsresult aStatus)
   if (NS_FAILED(aStatus))
   {
     // delete the file we created in CHBrowserService::Show
-    mDestinationFile->Remove(PR_FALSE);
-    mDestinationFile = nsnull;
+    if (mDestinationFile)
+    {
+      mDestinationFile->Remove(PR_FALSE);
+      mDestinationFile = nsnull;
+    }
     mDestination = nsnull;
   }
   
-  // XXX propagate the error, so that the UI can show strings for
-  // disk full etc.
-  [mDownloadDisplay onEndDownload:(NS_SUCCEEDED(aStatus) && !mUserCanceled)];
+  [mDownloadDisplay onEndDownload:(NS_SUCCEEDED(aStatus) && !mUserCanceled) statusCode:aStatus];
 }
 
 //

camino/src/embedding/CHDownloadProgressDisplay.h

@@ -110,13 +110,16 @@ class CHDownloader;
 @protocol CHDownloadProgressDisplay
 
 - (void)onStartDownload:(BOOL)isFileSave;
-- (void)onEndDownload:(BOOL)completedOK;
+- (void)onEndDownload:(BOOL)completedOK statusCode:(nsresult)aStatus;
 
 - (void)setProgressTo:(long long)aCurProgress ofMax:(long long)aMaxProgress;
 
 - (void)setDownloadListener:(CHDownloader*)aDownloader;
 - (void)setSourceURL:(NSString*)aSourceURL;
+- (NSString*)sourceURL;
+
 - (void)setDestinationPath:(NSString*)aDestPath;
+- (NSString*)destinationPath;
 
 @end