Fix exception catching in download resume.

4 files changed, 62 insertions, 19 deletions

diff --git a/android/jni/com/mapswithme/maps/DownloadResourcesActivity.cpp b/android/jni/com/mapswithme/maps/DownloadResourcesActivity.cpp
index 7714fea91c..5084f78d3b 100644
--- a/android/jni/com/mapswithme/maps/DownloadResourcesActivity.cpp
+++ b/android/jni/com/mapswithme/maps/DownloadResourcesActivity.cpp
@@ -23,6 +23,8 @@
 #include "Framework.hpp"
 
 
+using namespace downloader;
+
 /// Special error codes to notify GUI about free space
 //@{
 #define ERR_DOWNLOAD_SUCCESS         0
@@ -47,7 +49,7 @@ struct FileToDownload
 static vector<FileToDownload> g_filesToDownload;
 static int g_totalDownloadedBytes;
 static int g_totalBytesToDownload;
-static shared_ptr<downloader::HttpRequest> g_currentRequest;
+static shared_ptr<HttpRequest> g_currentRequest;
 
 extern "C"
 {
@@ -172,18 +174,18 @@ extern "C"
             pl.GetFileSizeByName(path + WORLD_COASTS_FILE_NAME + DATA_FILE_EXTENSION, dummy));
   }
 
-  void DownloadFileFinished(shared_ptr<jobject> obj, downloader::HttpRequest & req)
+  void DownloadFileFinished(shared_ptr<jobject> obj, HttpRequest const & req)
   {
-    int errorCode = 0;
+    HttpRequest::StatusT const status = req.Status();
+    ASSERT_NOT_EQUAL(status, HttpRequest::EInProgress, ());
 
-    ASSERT(req.Status() != downloader::HttpRequest::EInProgress, ("should be either Completed or Failed"));
-
-    switch (req.Status())
+    int errorCode = 0;
+    switch (status)
     {
-    case downloader::HttpRequest::ECompleted:
+    case HttpRequest::ECompleted:
       errorCode = ERR_DOWNLOAD_SUCCESS;
       break;
-    case downloader::HttpRequest::EFailed:
+    case HttpRequest::EFailed:
       errorCode = ERR_DOWNLOAD_ERROR;
       break;
     };
@@ -209,7 +211,7 @@ extern "C"
     env->CallVoidMethod(*obj.get(), methodID, errorCode);
   }
 
-  void DownloadFileProgress(shared_ptr<jobject> obj, downloader::HttpRequest & req)
+  void DownloadFileProgress(shared_ptr<jobject> obj, HttpRequest const & req)
   {
     //LOG(LDEBUG, (req.Progress().first, "bytes for", g_filesToDownload.back().m_fileName, "was downloaded"));
 
@@ -228,16 +230,16 @@ extern "C"
                          glbTotal, glbProgress);
   }
 
-  typedef downloader::HttpRequest::CallbackT CallbackT;
+  typedef HttpRequest::CallbackT CallbackT;
 
-  void DownloadURLListFinished(downloader::HttpRequest & req,
+  void DownloadURLListFinished(HttpRequest const & req,
                                CallbackT const & onFinish, CallbackT const & onProgress)
   {
     FileToDownload & curFile = g_filesToDownload.back();
 
     LOG(LINFO, ("Finished URL list download for", curFile.m_fileName));
 
-    downloader::GetServerListFromRequest(req, curFile.m_urls);
+    GetServerListFromRequest(req, curFile.m_urls);
 
     storage::Storage const & storage = g_framework->Storage();
     for (size_t i = 0; i < curFile.m_urls.size(); ++i)
@@ -246,7 +248,7 @@ extern "C"
       LOG(LINFO, (curFile.m_urls[i]));
     }
 
-    g_currentRequest.reset(downloader::HttpRequest::GetFile(
+    g_currentRequest.reset(HttpRequest::GetFile(
         curFile.m_urls, curFile.m_pathOnSdcard, curFile.m_fileSize,
         onFinish, onProgress,
         64 * 1024, false));
@@ -273,7 +275,7 @@ extern "C"
     CallbackT onFinish(bind(&DownloadFileFinished, jni::make_global_ref(observer), _1));
     CallbackT onProgress(bind(&DownloadFileProgress, jni::make_global_ref(observer), _1));
 
-    g_currentRequest.reset(downloader::HttpRequest::PostJson(
+    g_currentRequest.reset(HttpRequest::PostJson(
         GetPlatform().ResourcesMetaServerUrl(), curFile.m_fileName,
         bind(&DownloadURLListFinished, _1, onFinish, onProgress)));
 
diff --git a/android/jni/com/mapswithme/platform/HttpThread.cpp b/android/jni/com/mapswithme/platform/HttpThread.cpp
index f20af60a67..2aa63b5d0c 100644
--- a/android/jni/com/mapswithme/platform/HttpThread.cpp
+++ b/android/jni/com/mapswithme/platform/HttpThread.cpp
@@ -19,6 +19,7 @@ public:
   {
     /// should create java object here.
     JNIEnv * env = jni::GetEnv();
+    ASSERT ( env, () );
 
     jclass klass = env->FindClass("com/mapswithme/maps/downloader/DownloadChunkTask");
     ASSERT ( klass, () );
@@ -49,7 +50,11 @@ public:
   ~HttpThread()
   {
     JNIEnv * env = jni::GetEnv();
+    ASSERT ( env, () );
+
     jmethodID methodId = jni::GetJavaMethodID(env, m_self, "cancel", "(Z)Z");
+    ASSERT ( methodId, () );
+
     env->CallBooleanMethod(m_self, methodId, false);
 
     env->DeleteGlobalRef(m_self);
diff --git a/platform/chunks_download_strategy.cpp b/platform/chunks_download_strategy.cpp
index 042d94abd9..5c0cf1e281 100644
--- a/platform/chunks_download_strategy.cpp
+++ b/platform/chunks_download_strategy.cpp
@@ -73,7 +73,7 @@ void ChunksDownloadStrategy::SaveChunks(int64_t fileSize, string const & fName)
       w.Write(&m_chunks[0], sizeof(ChunkT) * m_chunks.size());
       return;
     }
-    catch(FileWriter::Exception const & e)
+    catch (FileWriter::Exception const & e)
     {
       LOG(LERROR, (e.Msg()));
     }
@@ -119,9 +119,9 @@ int64_t ChunksDownloadStrategy::LoadOrInitChunks( string const & fName,
       return downloadedSize;
     }
   }
-  catch(FileReader::Exception const & e)
+  catch (RootException const & e)
   {
-    // Usually - file not exists.
+    // Usually - file not exists. May be SourceOutOfBoundsException.
     LOG(LDEBUG, (e.Msg()));
   }
 
diff --git a/platform/http_request.cpp b/platform/http_request.cpp
index 6492cd0280..5cbc6158e7 100644
--- a/platform/http_request.cpp
+++ b/platform/http_request.cpp
@@ -247,11 +247,13 @@ public:
                   CallbackT const & onFinish, CallbackT const & onProgress,
                   int64_t chunkSize, bool doCleanProgressFiles)
     : HttpRequest(onFinish, onProgress), m_strategy(urls), m_filePath(filePath),
-      m_writer(new FileWriter(filePath + DOWNLOADING_FILE_EXTENSION, FileWriter::OP_WRITE_EXISTING)),
       m_goodChunksCount(0), m_doCleanProgressFiles(doCleanProgressFiles)
   {
     ASSERT ( !urls.empty(), () );
 
+    // Open file here. Avoid throwing exceptions from constructor's initialization list.
+    m_writer.reset(new FileWriter(filePath + DOWNLOADING_FILE_EXTENSION, FileWriter::OP_WRITE_EXISTING));
+
     m_progress.first = m_strategy.LoadOrInitChunks(m_filePath + RESUME_FILE_EXTENSION,
                                                    fileSize, chunkSize);
     m_progress.second = fileSize;
@@ -310,11 +312,45 @@ HttpRequest * HttpRequest::PostJson(string const & url, string const & postData,
   return new MemoryHttpRequest(url, postData, onFinish, onProgress);
 }
 
+namespace
+{
+  class ErrorHttpRequest : public HttpRequest
+  {
+    string m_filePath;
+  public:
+    ErrorHttpRequest(string const & filePath)
+      : HttpRequest(CallbackT(), CallbackT()), m_filePath(filePath)
+    {
+      m_status = EFailed;
+    }
+
+    virtual string const & Data() const { return m_filePath; }
+  };
+}
+
 HttpRequest * HttpRequest::GetFile(vector<string> const & urls, string const & filePath, int64_t fileSize,
                                    CallbackT const & onFinish, CallbackT const & onProgress,
                                    int64_t chunkSize, bool doCleanProgressFiles)
 {
-  return new FileHttpRequest(urls, filePath, fileSize, onFinish, onProgress, chunkSize, doCleanProgressFiles);
+  FileHttpRequest * p = 0;
+  try
+  {
+    p = new FileHttpRequest(urls, filePath, fileSize, onFinish, onProgress, chunkSize, doCleanProgressFiles);
+  }
+  catch (FileWriter::Exception const & e)
+  {
+    // Can't create file for writing.
+    LOG(LERROR, ("Can't create FileHttpRequest for", filePath, e.what()));
+
+    delete p;
+    p = 0;
+
+    // Mark the end of download with error.
+    ErrorHttpRequest error(filePath);
+    onFinish(error);
+  }
+
+  return p;
 }
 
 } // namespace downloader