Refactoring of maps update after downloading:

- Delay maps deleting and updating if mwm is busy;
- Fix bugs with different mwm maps status;
- Add better function for country bounds;
- Simplify Storage logic;

6 files changed, 166 insertions, 232 deletions

diff --git a/storage/country.cpp b/storage/country.cpp
index 054b66bcef..af21ffc747 100644
--- a/storage/country.cpp
+++ b/storage/country.cpp
@@ -1,8 +1,8 @@
 #include "country.hpp"
 
-#include "../indexer/data_factory.hpp"
+//#include "../indexer/data_factory.hpp"
 
-#include "../coding/file_container.hpp"
+//#include "../coding/file_container.hpp"
 
 #include "../platform/platform.hpp"
 
@@ -29,6 +29,7 @@ uint32_t CountryFile::GetFileSize() const
     return 0;
 }
 
+/*
 class CountryBoundsCalculator
 {
   m2::RectD & m_bounds;
@@ -54,6 +55,7 @@ m2::RectD Country::Bounds() const
   std::for_each(m_files.begin(), m_files.end(), CountryBoundsCalculator(bounds));
   return bounds;
 }
+*/
 
 LocalAndRemoteSizeT Country::Size() const
 {
@@ -234,6 +236,7 @@ void SaveImpl(T const & v, json_t * jParent)
 {
   size_t const siblingsCount = v.SiblingsCount();
   CHECK_GREATER(siblingsCount, 0, ());
+
   my::Json jArray(json_array());
   for (size_t i = 0; i < siblingsCount; ++i)
   {
@@ -244,22 +247,27 @@ void SaveImpl(T const & v, json_t * jParent)
     string const strFlag = v[i].Value().Flag();
     if (!strFlag.empty())
       json_object_set_new(jCountry, "c", json_string(strFlag.c_str()));
-    CHECK_LESS_OR_EQUAL(v[i].Value().Files().size(), 1, ("Not supporting more than 1 file for the country at the moment"));
-    if (v[i].Value().Files().size())
+
+    size_t countriesCount = v[i].Value().GetFilesCount();
+    ASSERT_LESS_OR_EQUAL(countriesCount, 1, ());
+    if (countriesCount > 0)
     {
-      int64_t const price = v[i].Value().Files()[0].m_price;
+      CountryFile const & file = v[i].Value().GetFile();
+      int64_t const price = file.m_price;
       CHECK_GREATER_OR_EQUAL(price, 0, ("Invalid price"));
       json_object_set_new(jCountry, "p", json_integer(price));
-      string const strFile = v[i].Value().Files()[0].m_fileName;
+      string const strFile = file.m_fileName;
       if (strFile != strName)
         json_object_set_new(jCountry, "f", json_string(strFile.c_str()));
-      json_object_set_new(jCountry, "s", json_integer(v[i].Value().Files()[0].m_remoteSize));
+      json_object_set_new(jCountry, "s", json_integer(file.m_remoteSize));
     }
+
     if (v[i].SiblingsCount())
       SaveImpl(v[i], jCountry);
 
     json_array_append(jArray, jCountry);
   }
+
   json_object_set(jParent, "g", jArray);
 }
 
diff --git a/storage/country.hpp b/storage/country.hpp
index acf9e8ddfa..d927d72fdc 100644
--- a/storage/country.hpp
+++ b/storage/country.hpp
@@ -57,14 +57,31 @@ namespace storage
     bool operator<(Country const & other) const { return Name() < other.Name(); }
 
     void AddFile(CountryFile const & file);
-    FilesContainerT const & Files() const { return m_files; }
+
+    size_t GetFilesCount() const { return m_files.size(); }
+
+    /*
+    template <class ToDo> void ForEachFile(ToDo toDo) const
+    {
+      for (FilesContainerT::const_iterator i = m_files.begin(); i != m_files.end(); ++i)
+        toDo(i->GetFileWithExt());
+    }
+    */
+
+    /// This function valid for current logic - one file for one country (region).
+    /// If the logic will be changed, replace GetFile with ForEachFile.
+    CountryFile const & GetFile() const
+    {
+      ASSERT_EQUAL ( m_files.size(), 1, () );
+      return m_files.front();
+    }
 
     string const & Name() const { return m_name; }
     string const & Flag() const { return m_flag; }
     int64_t Price() const;
 
     /// @return bounds for downloaded parts of the country or empty rect
-    m2::RectD Bounds() const;
+    //m2::RectD Bounds() const;
     LocalAndRemoteSizeT Size() const;
   };
 
diff --git a/storage/country_info.cpp b/storage/country_info.cpp
index a475debf17..88fc5a7afd 100644
--- a/storage/country_info.cpp
+++ b/storage/country_info.cpp
@@ -13,12 +13,50 @@
 
 namespace storage
 {
+  /*
+  class LessCountryDef
+  {
+    bool CompareStrings(string const & s1, string const & s2) const
+    {
+      // Do this stuff because of 'Guinea-Bissau.mwm' goes before 'Guinea.mwm'
+      // in file system (and in PACKED_POLYGONS_FILE also).
+      size_t n = min(s1.size(), s2.size());
+      return lexicographical_compare(s1.begin(), s1.begin() + n,
+                                     s2.begin(), s2.begin() + n);
+    }
+
+  public:
+    bool operator() (CountryDef const & r1, string const & r2) const
+    {
+      return CompareStrings(r1.m_name, r2);
+    }
+    bool operator() (string const & r1, CountryDef const & r2) const
+    {
+      return CompareStrings(r1, r2.m_name);
+    }
+    bool operator() (CountryDef const & r1, CountryDef const & r2) const
+    {
+      return CompareStrings(r1.m_name, r2.m_name);
+    }
+  };
+  */
+
   CountryInfoGetter::CountryInfoGetter(ModelReaderPtr polyR, ModelReaderPtr countryR)
     : m_reader(polyR), m_cache(2)
   {
     ReaderSource<ModelReaderPtr> src(m_reader.GetReader(PACKED_POLYGONS_INFO_TAG));
     rw::Read(src, m_countries);
 
+/*
+    // We can't change the order of countries.
+#ifdef DEBUG
+    LessCountryDef check;
+    for (size_t i = 0; i < m_countries.size() - 1; ++i)
+      ASSERT ( !check(m_countries[i+1], m_countries[i]),
+               (m_countries[i].m_name, m_countries[i+1].m_name) );
+#endif
+*/
+
     string buffer;
     countryR.ReadAsString(buffer);
     LoadCountryFile2CountryInfo(buffer, m_id2info);
@@ -111,30 +149,60 @@ namespace storage
     CountryInfo::FileName2FullName(info.m_name);
   }
 
-  void CountryInfoGetter::CalcUSALimitRect(m2::RectD rects[3]) const
+  template <class ToDo> void CountryInfoGetter::ForEachCountry(string const & prefix, ToDo toDo) const
   {
-    for (size_t i = 0; i < m_countries.size(); ++i)
+    typedef vector<CountryDef>::const_iterator IterT;
+
+    for (IterT i = m_countries.begin(); i != m_countries.end(); ++i)
+    {
+      if (i->m_name.find(prefix) != string::npos)
+        toDo(*i);
+    }
+
+    /// @todo Store sorted list of polygons in PACKED_POLYGONS_FILE.
+    /*
+    pair<IterT, IterT> r = equal_range(m_countries.begin(), m_countries.end(), file, LessCountryDef());
+    while (r.first != r.second)
+    {
+      toDo(r.first->m_name);
+      ++r.first;
+    }
+    */
+  }
+
+  namespace
+  {
+    class DoCalcUSA
     {
-      if (m_countries[i].m_name.find("USA_") == 0)
+      m2::RectD * m_rects;
+    public:
+      DoCalcUSA(m2::RectD * rects) : m_rects(rects) {}
+      void operator() (CountryDef const & c)
       {
-        if (m_countries[i].m_name == "USA_Alaska")
-          rects[1] = m_countries[i].m_rect;
-        else if (m_countries[i].m_name == "USA_Hawaii")
-          rects[2] = m_countries[i].m_rect;
+        if (c.m_name == "USA_Alaska")
+          m_rects[1] = c.m_rect;
+        else if (c.m_name == "USA_Hawaii")
+          m_rects[2] = c.m_rect;
         else
-          rects[0].Add(m_countries[i].m_rect);
+          m_rects[0].Add(c.m_rect);
       }
+    };
+
+    void AddRect(m2::RectD & r, CountryDef const & c)
+    {
+      r.Add(c.m_rect);
     }
   }
 
+  void CountryInfoGetter::CalcUSALimitRect(m2::RectD rects[3]) const
+  {
+    ForEachCountry("USA_", DoCalcUSA(rects));
+  }
+
   m2::RectD CountryInfoGetter::CalcLimitRect(string const & prefix) const
   {
     m2::RectD r;
-    for (size_t i = 0; i < m_countries.size(); ++i)
-    {
-      if (m_countries[i].m_name.find(prefix) == 0)
-        r.Add(m_countries[i].m_rect);
-    }
+    ForEachCountry(prefix, bind(&AddRect, ref(r), _1));
     return r;
   }
 }
diff --git a/storage/country_info.hpp b/storage/country_info.hpp
index 2525f1c9d2..5dcd95d970 100644
--- a/storage/country_info.hpp
+++ b/storage/country_info.hpp
@@ -16,6 +16,9 @@ namespace storage
     FilesContainerR m_reader;
 
     vector<CountryDef> m_countries;
+
+    template <class ToDo> void ForEachCountry(string const & prefix, ToDo toDo) const;
+
     /// ID - is a country file name without an extension.
     map<string, CountryInfo> m_id2info;
 
diff --git a/storage/storage.cpp b/storage/storage.cpp
index eff36a1870..8a734c32ea 100644
--- a/storage/storage.cpp
+++ b/storage/storage.cpp
@@ -69,11 +69,9 @@ namespace storage
   }
 
   ////////////////////////////////////////////////////////////////////////////
-  void Storage::Init(TAddMapFunction addFunc, TRemoveMapFunction removeFunc, TUpdateRectFunction updateRectFunc)
+  void Storage::Init(TUpdateAfterDownload const & updateFn)
   {
-    m_addMap = addFunc;
-    m_removeMap = removeFunc;
-    m_updateRect = updateRectFunc;
+    m_updateAfterDownload = updateFn;
   }
 
   CountriesContainerT const & NodeFromIndex(CountriesContainerT const & root, TIndex const & index)
@@ -132,17 +130,7 @@ namespace storage
     if (m_failedCountries.count(index) > 0)
       return EDownloadFailed;
 
-    //if (m_indexGeneration.count(index) > 0)
-    //  return EGeneratingIndex;
-
-    LocalAndRemoteSizeT const size = CountryByIndex(index).Size();
-    if (size.first == 0)
-      return ENotDownloaded;
-
-    if (size.second == 0)
-      return EUnknown;
-
-    return (size.first == size.second ? EOnDisk : EOnDiskOutOfDate);
+    return EUnknown;
   }
 
   void Storage::DownloadCountry(TIndex const & index)
@@ -159,14 +147,10 @@ namespace storage
     m_failedCountries.erase(index);
     // add it into the queue
     m_queue.push_back(index);
+
     // and start download if necessary
     if (m_queue.size() == 1)
     {
-      // reset total country's download progress
-      LocalAndRemoteSizeT const size = CountryByIndex(index).Size();
-      m_countryProgress.first = 0;
-      m_countryProgress.second = size.second;
-
       DownloadNextCountryFromQueue();
     }
     else
@@ -176,22 +160,6 @@ namespace storage
     }
   }
 
-  template <class TRemoveFn>
-  class DeactivateMap
-  {
-    string m_workingDir;
-    TRemoveFn & m_removeFn;
-  public:
-    DeactivateMap(TRemoveFn & removeFn) : m_removeFn(removeFn)
-    {
-      m_workingDir = GetPlatform().WritableDir();
-    }
-    void operator()(CountryFile const & file)
-    {
-      m_removeFn(file.GetFileWithExt());
-    }
-  };
-
   void Storage::NotifyStatusChanged(TIndex const & index) const
   {
     for (list<CountryObservers>::const_iterator it = m_observers.begin(); it != m_observers.end(); ++it)
@@ -200,85 +168,37 @@ namespace storage
 
   void Storage::DownloadNextCountryFromQueue()
   {
-    while (!m_queue.empty())
+    if (!m_queue.empty())
     {
-      TIndex index = m_queue.front();
-      FilesContainerT const & tiles = CountryByIndex(index).Files();
-      for (FilesContainerT::const_iterator it = tiles.begin(); it != tiles.end(); ++it)
-      {
-        if (it->GetFileSize() == 0)
-        {
-          // send Country name for statistics
-          string const postBody = it->m_fileName;
-          m_request.reset(HttpRequest::PostJson(GetPlatform().MetaServerUrl(),
-              postBody,
-              bind(&Storage::OnServerListDownloaded, this, _1)));
+      TIndex const index = m_queue.front();
+      Country const & country = CountryByIndex(index);
 
-          // new status for country, "Downloading"
-          NotifyStatusChanged(index);
-          return;
-        }
-      }
-
-      // continue with next country
-      m_queue.pop_front();
+      /// Reset progress before downloading.
+      /// @todo If we will have more than one file per country,
+      /// we should initialize progress before calling DownloadNextCountryFromQueue().
+      m_countryProgress.first = 0;
+      m_countryProgress.second = country.Size().second;
 
-      // reset total country's download progress
-      if (!m_queue.empty())
-      {
-        m_countryProgress.first = 0;
-        m_countryProgress.second = CountryByIndex(m_queue.front()).Size().second;
-      }
+      // send Country name for statistics
+      m_request.reset(HttpRequest::PostJson(GetPlatform().MetaServerUrl(),
+              country.GetFile().m_fileName,
+              bind(&Storage::OnServerListDownloaded, this, _1)));
 
-      // new status for country, "OnDisk"
+      // new status for country, "Downloading"
       NotifyStatusChanged(index);
     }
   }
 
-  class DeleteMap
-  {
-    string m_workingDir;
-  public:
-    DeleteMap()
-    {
-      m_workingDir = GetPlatform().WritableDir();
-    }
-
-    void operator()(CountryFile const & file)
-    {
-      FileWriter::DeleteFileX(m_workingDir + file.m_fileName + DOWNLOADING_FILE_EXTENSION);
-      FileWriter::DeleteFileX(m_workingDir + file.m_fileName + RESUME_FILE_EXTENSION);
-      FileWriter::DeleteFileX(m_workingDir + file.GetFileWithExt());
-    }
-  };
-
-  template <typename TRemoveFunc>
-  void DeactivateAndDeleteCountry(Country const & country, TRemoveFunc removeFunc)
-  {
-    // deactivate from multiindex
-    for_each(country.Files().begin(), country.Files().end(), DeactivateMap<TRemoveFunc>(removeFunc));
-    // delete from disk
-    for_each(country.Files().begin(), country.Files().end(), DeleteMap());
-  }
-
+  /*
   m2::RectD Storage::CountryBounds(TIndex const & index) const
   {
     Country const & country = CountryByIndex(index);
     return country.Bounds();
   }
+  */
 
-  void Storage::DeleteCountryFiles(TIndex const & index)
-  {
-    Country const & country = CountryByIndex(index);
-    for_each(country.Files().begin(), country.Files().end(), DeleteMap());
-  }
-
-  void Storage::DeleteCountry(TIndex const & index)
+  bool Storage::DeleteFromDownloader(TIndex const & index)
   {
-    Country const & country = CountryByIndex(index);
-
-    m2::RectD bounds;
-
     // check if we already downloading this country
     TQueue::iterator found = find(m_queue.begin(), m_queue.end(), index);
     if (found != m_queue.end())
@@ -289,12 +209,6 @@ namespace storage
         m_request.reset();
         // remove from the queue
         m_queue.erase(found);
-        // reset progress if the queue is not empty
-        if (!m_queue.empty())
-        {
-          m_countryProgress.first = 0;
-          m_countryProgress.second = CountryByIndex(m_queue.front()).Size().second;
-        }
         // start another download if the queue is not empty
         DownloadNextCountryFromQueue();
       }
@@ -303,18 +217,11 @@ namespace storage
         // remove from the queue
         m_queue.erase(found);
       }
-    }
-    else
-    {
-      // bounds are only updated if country was already activated before
-      bounds = country.Bounds();
-    }
 
-    DeactivateAndDeleteCountry(country, m_removeMap);
-    NotifyStatusChanged(index);
+      return true;
+    }
 
-    if (bounds != m2::RectD::GetEmptyRect())
-      m_updateRect(bounds);
+    return false;
   }
 
   void Storage::LoadCountriesFile(bool forceReload)
@@ -332,8 +239,8 @@ namespace storage
     }
   }
 
-  int Storage::Subscribe(TChangeCountryFunction change,
-                         TProgressFunction progress)
+  int Storage::Subscribe(TChangeCountryFunction const & change,
+                         TProgressFunction const & progress)
   {
     CountryObservers obs;
 
@@ -362,79 +269,30 @@ namespace storage
   {
     if (m_queue.empty())
     {
-      ASSERT ( false, ("Invalid url?", request.Data()) );
+      ASSERT ( false, ("queue can't be empty") );
       return;
     }
 
     TIndex const index = m_queue.front();
+    m_queue.pop_front();
+
     if (request.Status() == HttpRequest::EFailed)
     {
-      // remove failed country from the queue
-      m_queue.pop_front();
+      // add to failed countries set
       m_failedCountries.insert(index);
-
-      // notify GUI about failed country
-      NotifyStatusChanged(index);
     }
     else
     {
-      LocalAndRemoteSizeT const size = CountryByIndex(index).Size();
-      if (size.second != 0)
-        m_countryProgress.first = size.first;
-
-      // get file descriptor
-      string file = request.Data();
-      pl::GetNameFromURLRequest(file);
-
-      /// @todo By the way - this code os obsolete.
-      /// It doesn't supported properly in android now (because of Platform::RunOnGuiThread).
-      /*
-      Platform & pl = GetPlatform();
-      if (pl.IsFeatureSupported("search"))
-      {
-        // Generate search index if it's supported in this build
-        m_indexGeneration.insert(index);
-        pl.RunAsync(bind(&Storage::GenerateSearchIndex, this, index, file));
-      }
-      else
-      */
-      {
-        // Or simply activate downloaded map
-        UpdateAfterSearchIndex(index, file);
-      }
-    }
+      Country const & country = CountryByIndex(index);
 
-    m_request.reset();
-    DownloadNextCountryFromQueue();
-  }
-
-  /*
-  void Storage::GenerateSearchIndex(TIndex const & index, string const & fName)
-  {
-    if (indexer::BuildSearchIndexFromDatFile(fName))
-    {
-      GetPlatform().RunOnGuiThread(bind(&Storage::UpdateAfterSearchIndex, this, index, fName));
+      // notify framework that downloading is done
+      m_updateAfterDownload(country.GetFile().GetFileWithExt());
     }
-    else
-    {
-      LOG(LERROR, ("Can't build search index for", fName));
-    }
-  }
-  */
 
-  void Storage::UpdateAfterSearchIndex(TIndex const & index, string const & fName)
-  {
-    // remove from index set
-    //m_indexGeneration.erase(index);
     NotifyStatusChanged(index);
 
-    // activate downloaded map piece
-    m_addMap(fName);
-
-    // update rect from downloaded file
-    feature::DataHeader header;
-    LoadMapHeader(GetPlatform().GetReader(fName), header);
-    m_updateRect(header.GetBounds());
+    m_request.reset();
+    DownloadNextCountryFromQueue();
   }
 
   void Storage::ReportProgress(TIndex const & idx, pair<int64_t, int64_t> const & p)
@@ -447,7 +305,7 @@ namespace storage
   {
     if (m_queue.empty())
     {
-      ASSERT(false, ("queue can't be empty"));
+      ASSERT ( false, ("queue can't be empty") );
       return;
     }
 
@@ -465,12 +323,11 @@ namespace storage
   {
     if (m_queue.empty())
     {
-      ASSERT(false, ("this should never happen"));
+      ASSERT ( false, ("queue can't be empty") );
       return;
     }
 
-    // @TODO now supports only one file in the country
-    CountryFile const & file = CountryByIndex(m_queue.front()).Files().front();
+    CountryFile const & file = CountryByIndex(m_queue.front()).GetFile();
 
     vector<string> urls;
     GetServerListFromRequest(request, urls);
@@ -479,9 +336,8 @@ namespace storage
     for (size_t i = 0; i < urls.size(); ++i)
       urls[i] = GetFileDownloadUrl(urls[i], file.GetFileWithExt());
 
-    m_request.reset(HttpRequest::GetFile(urls,
-                                         GetPlatform().WritablePathForFile(file.GetFileWithExt()),
-                                         file.m_remoteSize,
+    string const fileName = GetPlatform().WritablePathForFile(file.GetFileWithExt() + READY_FILE_EXTENSION);
+    m_request.reset(HttpRequest::GetFile(urls, fileName, file.m_remoteSize,
                                          bind(&Storage::OnMapDownloadFinished, this, _1),
                                          bind(&Storage::OnMapDownloadProgress, this, _1)));
     ASSERT ( m_request, () );
diff --git a/storage/storage.hpp b/storage/storage.hpp
index ae0f8bc51a..3eabcfe2f7 100644
--- a/storage/storage.hpp
+++ b/storage/storage.hpp
@@ -12,6 +12,7 @@
 #include "../std/function.hpp"
 #include "../std/scoped_ptr.hpp"
 
+
 namespace storage
 {
   /// Used in GUI
@@ -23,7 +24,6 @@ namespace storage
     EDownloading,
     EInQueue,
     EUnknown,
-    EGeneratingIndex,
     EOnDiskOutOfDate
   };
 
@@ -81,9 +81,6 @@ namespace storage
     typedef set<TIndex> TCountriesSet;
     TCountriesSet m_failedCountries;
 
-    /// store countries set for which search index is generating
-    //TCountriesSet m_indexGeneration;
-
     /// used to correctly calculate total country download progress with more than 1 file
     /// <current, total>
     downloader::HttpRequest::ProgressT m_countryProgress;
@@ -108,24 +105,11 @@ namespace storage
 
     /// @name Communicate with Framework
     //@{
-    typedef vector<string> map_list_t;
-  public:
-    typedef function<void (string const &)> TAddMapFunction;
-    typedef function<void (string const &)> TRemoveMapFunction;
-    typedef function<void (m2::RectD const & r)> TUpdateRectFunction;
-    typedef function<void (map_list_t &)> TEnumMapsFunction;
-
-  private:
-    TAddMapFunction m_addMap;
-    TRemoveMapFunction m_removeMap;
-    TUpdateRectFunction m_updateRect;
+    typedef function<void (string const &)> TUpdateAfterDownload;
+    TUpdateAfterDownload m_updateAfterDownload;
     //@}
 
     void DownloadNextCountryFromQueue();
-    Country const & CountryByIndex(TIndex const & index) const;
-
-    //void GenerateSearchIndex(TIndex const & index, string const & fName);
-    void UpdateAfterSearchIndex(TIndex const & index, string const & fName);
 
     void LoadCountriesFile(bool forceReload);
 
@@ -134,9 +118,7 @@ namespace storage
   public:
     Storage();
 
-    void Init(TAddMapFunction addFunc,
-              TRemoveMapFunction removeFunc,
-              TUpdateRectFunction updateRectFunc);
+    void Init(TUpdateAfterDownload const & updateFn);
 
     /// @name Called from DownloadManager
     //@{
@@ -149,11 +131,12 @@ namespace storage
     //@{
 
     /// @return unique identifier that should be used with Unsubscribe function
-    int Subscribe(TChangeCountryFunction change,
-                  TProgressFunction progress);
+    int Subscribe(TChangeCountryFunction const & change,
+                  TProgressFunction const & progress);
     void Unsubscribe(int slotId);
     //@}
 
+    Country const & CountryByIndex(TIndex const & index) const;
     TIndex const FindIndexByName(string const & name) const;
 
     size_t CountriesCount(TIndex const & index) const;
@@ -161,11 +144,10 @@ namespace storage
     string const & CountryFlag(TIndex const & index) const;
     LocalAndRemoteSizeT CountrySizeInBytes(TIndex const & index) const;
     TStatus CountryStatus(TIndex const & index) const;
-    m2::RectD CountryBounds(TIndex const & index) const;
+    //m2::RectD CountryBounds(TIndex const & index) const;
 
     void DownloadCountry(TIndex const & index);
-    void DeleteCountry(TIndex const & index);
-    void DeleteCountryFiles(TIndex const & index);
+    bool DeleteFromDownloader(TIndex const & index);
 
     void CheckForUpdate();