[storage] Fixed crash.

4 files changed, 49 insertions, 20 deletions

diff --git a/storage/downloading_policy.hpp b/storage/downloading_policy.hpp
index 78fd6119fa..e7e12f6a8b 100644
--- a/storage/downloading_policy.hpp
+++ b/storage/downloading_policy.hpp
@@ -11,6 +11,7 @@ class DownloadingPolicy
 {
 public:
   using TProcessFunc = function<void(storage::TCountriesSet const &)>;
+  virtual ~DownloadingPolicy() = default;
   virtual bool IsDownloadingAllowed() { return true; }
   virtual void ScheduleRetry(storage::TCountriesSet const &, TProcessFunc const &) {}
 };
diff --git a/storage/storage.cpp b/storage/storage.cpp
index 174bb0a248..383c8d1002 100644
--- a/storage/storage.cpp
+++ b/storage/storage.cpp
@@ -595,8 +595,6 @@ void Storage::DownloadNextCountryFromQueue()
       !PreparePlaceForCountryFiles(GetCurrentDataVersion(), m_dataDir, GetCountryFile(countryId)))
   {
     OnMapDownloadFinished(countryId, false /* success */, queuedCountry.GetInitOptions());
-    NotifyStatusChangedForHierarchy(countryId);
-    CorrectJustDownloadedAndQueue(m_queue.begin());
     DownloadNextCountryFromQueue();
     return;
   }
@@ -840,7 +838,8 @@ void Storage::RegisterDownloadedFiles(TCountryId const & countryId, MapOptions o
     m_didDownload(countryId, localFile);
 
     CHECK(!m_queue.empty(), ());
-    CorrectJustDownloadedAndQueue(m_queue.begin());
+    PushToJustDownloaded(m_queue.begin());
+    PeekFromQueue(m_queue.begin());
     SaveDownloadQueue();
 
     m_downloader->Reset();
@@ -1171,17 +1170,9 @@ bool Storage::DeleteCountryFilesFromDownloader(TCountryId const & countryId)
   // Remove country from the queue if there's nothing to download.
   if (queuedCountry->GetInitOptions() == MapOptions::Nothing)
   {
-    auto it = find(m_queue.cbegin(), m_queue.cend(), countryId);
-    ASSERT(it != m_queue.cend(), ());
-    if (m_queue.size() == 1)
-    {  // If m_queue is about to be empty.
-      m_justDownloaded.clear();
-      m_queue.clear();
-    }
-    else
-    {  // A deleted map should not be moved to m_justDownloaded.
-      m_queue.erase(it);
-    }
+    auto it = find(m_queue.begin(), m_queue.end(), countryId);
+    ASSERT(it != m_queue.end(), ());
+    PeekFromQueue(it);
     SaveDownloadQueue();
   }
 
@@ -1776,11 +1767,15 @@ bool Storage::GetUpdateInfo(TCountryId const & countryId, UpdateInfo & updateInf
   return true;
 }
 
-void Storage::CorrectJustDownloadedAndQueue(TQueue::iterator justDownloadedItem)
+void Storage::PushToJustDownloaded(TQueue::iterator justDownloadedItem)
 {
   m_justDownloaded.insert(justDownloadedItem->GetCountryId());
+}
+
+void Storage::PeekFromQueue(TQueue::iterator it)
+{
   CHECK(!m_queue.empty(), ());
-  m_queue.erase(justDownloadedItem);
+  m_queue.erase(it);
   if (m_queue.empty())
     m_justDownloaded.clear();
 }
@@ -1900,9 +1895,9 @@ TMwmSize Storage::GetRemoteSize(CountryFile const & file, MapOptions opt, int64_
 void Storage::OnDownloadFailed(TCountryId const & countryId)
 {
   m_failedCountries.insert(countryId);
-  auto it = find(m_queue.cbegin(), m_queue.cend(), countryId);
-  if (it != m_queue.cend())
-    m_queue.erase(it);
+  auto it = find(m_queue.begin(), m_queue.end(), countryId);
+  if (it != m_queue.end())
+    PeekFromQueue(it);
   NotifyStatusChangedForHierarchy(countryId);
 }
 }  // namespace storage
diff --git a/storage/storage.hpp b/storage/storage.hpp
index 527e083fb0..bc14ad7b2b 100644
--- a/storage/storage.hpp
+++ b/storage/storage.hpp
@@ -648,7 +648,8 @@ private:
                                                   MapFilesDownloader::TProgress const & downloadingMwmProgress,
                                                   TCountriesSet const & mwmsInQueue) const;
 
-  void CorrectJustDownloadedAndQueue(TQueue::iterator justDownloadedItem);
+  void PushToJustDownloaded(TQueue::iterator justDownloadedItem);
+  void PeekFromQueue(TQueue::iterator it);
   template <class ToDo>
   void ForEachAncestorExceptForTheRoot(vector<TCountryTreeNode const *> const & nodes, ToDo && toDo) const;
   /// Returns true if |node.Value().Name()| is a disputed territory and false otherwise.
diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp
index 8e24bc17f6..e246502796 100644
--- a/storage/storage_tests/storage_tests.cpp
+++ b/storage/storage_tests/storage_tests.cpp
@@ -62,6 +62,12 @@ namespace
 {
 using TLocalFilePtr = shared_ptr<LocalCountryFile>;
 
+class DummyDownloadingPolicy : public DownloadingPolicy
+{
+public:
+  bool IsDownloadingAllowed() override { return false; }
+};
+
 string const kSingleMwmCountriesTxt = string(R"({
            "id": "Countries",
            "v": )" + strings::to_string(version::FOR_TESTING_SINGLE_MWM1) + R"(,
@@ -1789,5 +1795,31 @@ UNIT_TEST(StorageTest_GetTopmostNodesForWithLevel)
   TEST_EQUAL(path[0], "France_Burgundy_Saone-et-Loire", ());
 }
 
+UNIT_TEST(StorageTest_FalsePolicy)
+{
+  DummyDownloadingPolicy policy;
+  Storage storage;
+  storage.SetDownloadingPolicy(&policy);
+
+  storage.Init(&OnCountryDownloaded /* didDownload */,
+               [](TCountryId const &, TLocalFilePtr const) { return false; } /* willDelete */);
+  storage.SetDownloaderForTesting(make_unique<TestMapFilesDownloader>());
+  storage.SetCurrentDataVersionForTesting(1234);
+
+  auto const countryId = storage.FindCountryIdByFile("Uruguay");
+  auto const countryFile = storage.GetCountryFile(countryId);
 
+  // To prevent interference from other tests and on other tests it's
+  // better to remove temprorary downloader files.
+  DeleteDownloaderFilesForCountry(storage.GetCurrentDataVersion(), countryFile);
+  MY_SCOPE_GUARD(cleanup, [&]() {
+    DeleteDownloaderFilesForCountry(storage.GetCurrentDataVersion(),
+                                    countryFile);
+  });
+
+  {
+    FailedDownloadingWaiter waiter(storage, countryId);
+    storage.DownloadCountry(countryId, MapOptions::Map);
+  }
+}
 }  // namespace storage