From 83bae35a7ecfb68624ceff144dcd99f149e1ac54 Mon Sep 17 00:00:00 2001
From: Arsentiy Milchakov <milcars@mapswithme.com>
Date: Mon, 30 Sep 2019 18:23:02 +0300
Subject: [PATCH] [storage] review fixes

---
 storage/diff_scheme/diff_manager.cpp       |  8 +++-----
 storage/diff_scheme/diff_manager.hpp       |  5 +++--
 storage/diff_scheme/diff_scheme_loader.cpp | 13 +++++++------
 storage/storage.cpp                        | 16 +++++++---------
 4 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/storage/diff_scheme/diff_manager.cpp b/storage/diff_scheme/diff_manager.cpp
index 2fb09fb8b5..e5f3c1e9b1 100644
--- a/storage/diff_scheme/diff_manager.cpp
+++ b/storage/diff_scheme/diff_manager.cpp
@@ -39,12 +39,13 @@ void Manager::Load(NameDiffInfoMap && info)
   }
 }
 
+// static
 void Manager::ApplyDiff(ApplyDiffParams && p, base::Cancellable const & cancellable,
                         Manager::OnDiffApplicationFinished const & task)
 {
   using namespace generator::mwm_diff;
 
-  GetPlatform().RunTask(Platform::Thread::File, [this, p = std::move(p), &cancellable, task] {
+  GetPlatform().RunTask(Platform::Thread::File, [p = std::move(p), &cancellable, task] {
     CHECK(p.m_diffFile, ());
     CHECK(p.m_oldMwmFile, ());
 
@@ -101,11 +102,8 @@ void Manager::ApplyDiff(ApplyDiffParams && p, base::Cancellable const & cancella
       break;
     }
 
-    GetPlatform().RunTask(Platform::Thread::Gui, [this, task, result]()
+    GetPlatform().RunTask(Platform::Thread::Gui, [task, result]()
     {
-      if (result == DiffApplicationResult::Failed)
-        m_status = Status::NotAvailable;
-
       task(result);
     });
   });
diff --git a/storage/diff_scheme/diff_manager.hpp b/storage/diff_scheme/diff_manager.hpp
index 3bafbb0729..a6c5f1daab 100644
--- a/storage/diff_scheme/diff_manager.hpp
+++ b/storage/diff_scheme/diff_manager.hpp
@@ -59,8 +59,9 @@ public:
   Status GetStatus() const;
 
   void Load(NameDiffInfoMap && info);
-  void ApplyDiff(ApplyDiffParams && p, base::Cancellable const & cancellable,
-                 OnDiffApplicationFinished const & task);
+  static void ApplyDiff(ApplyDiffParams && p, base::Cancellable const & cancellable,
+                        OnDiffApplicationFinished const & task);
+
 private:
   template <typename Fn>
   bool WithNotAppliedDiff(storage::CountryId const & countryId, Fn && fn) const
diff --git a/storage/diff_scheme/diff_scheme_loader.cpp b/storage/diff_scheme/diff_scheme_loader.cpp
index 75e6d8f551..3b3d47c6d8 100644
--- a/storage/diff_scheme/diff_scheme_loader.cpp
+++ b/storage/diff_scheme/diff_scheme_loader.cpp
@@ -138,14 +138,15 @@ namespace storage
 {
 namespace diffs
 {
-//static
+// static
 void Loader::Load(LocalMapsInfo && info, DiffsReceivedCallback && callback)
 {
-    GetPlatform().RunTask(Platform::Thread::Network, [info = move(info), callback = move(callback)]() {
-      GetPlatform().RunTask(Platform::Thread::Gui,
-                            [result = ::Load(info), callback = move(callback)]() mutable {
-                              callback(move(result));
-                            });
+  GetPlatform().RunTask(Platform::Thread::Network, [info = move(info), callback = move(callback)]() {
+    auto result = ::Load(info);
+    GetPlatform().RunTask(Platform::Thread::Gui,
+                          [result = move(result), callback = move(callback)]() mutable {
+                            callback(move(result));
+                          });
   });
 }
 }  // namespace diffs
diff --git a/storage/storage.cpp b/storage/storage.cpp
index 1131677330..6b0abaf4f8 100644
--- a/storage/storage.cpp
+++ b/storage/storage.cpp
@@ -330,8 +330,8 @@ void Storage::RegisterAllLocalMaps(bool enableDiffs)
   if (enableDiffs)
     LoadDiffScheme();
   // Note: call order is important, diffs loading must be called first.
-  // Because of diffs downloading and servers list downloading
-  // are working on network thread, consequtive executing is guaranteed.
+  // Since diffs downloading and servers list downloading
+  // are working on network thread, consecutive executing is guaranteed.
   RestoreDownloadQueue();
 }
 
@@ -483,11 +483,11 @@ Status Storage::CountryStatusEx(CountryId const & countryId) const
   if (status != Status::EUnknown)
     return status;
 
-  LocalFilePtr localFile = GetLatestLocalFile(countryId);
+  auto localFile = GetLatestLocalFile(countryId);
   if (!localFile || !localFile->OnDisk(MapOptions::Map))
     return Status::ENotDownloaded;
 
-  CountryFile const & countryFile = GetCountryFile(countryId);
+  auto const & countryFile = GetCountryFile(countryId);
   if (GetRemoteSize(countryFile, MapOptions::Map, GetCurrentDataVersion()) == 0)
     return Status::EUnknown;
 
@@ -1561,7 +1561,7 @@ void Storage::ApplyDiff(CountryId const & countryId, function<void(bool isSucces
 
   LocalFilePtr & diffFile = params.m_diffFile;
 
-  m_diffManager.ApplyDiff(
+  diffs::Manager::ApplyDiff(
       move(params), m_diffsCancellable,
       [this, fn, countryId, diffFile](DiffApplicationResult result) {
         CHECK_THREAD_CHECKER(m_threadChecker, ());
@@ -1614,7 +1614,6 @@ bool Storage::IsPossibleToAutoupdate() const
   if (m_diffManager.GetStatus() != diffs::Status::Available)
     return false;
 
-  bool isPossibleToAutoupdate = true;
   auto const currentVersion = GetCurrentDataVersion();
   CountriesVec localMaps;
   GetLocalRealMaps(localMaps);
@@ -1625,12 +1624,11 @@ bool Storage::IsPossibleToAutoupdate() const
     if (mapVersion != currentVersion && mapVersion > 0 &&
         !m_diffManager.HasDiffFor(localFile->GetCountryName()))
     {
-      isPossibleToAutoupdate = false;
-      break;
+      return false;
     }
   }
 
-  return isPossibleToAutoupdate;
+  return true;
 }
 
 void Storage::SetStartDownloadingCallback(StartDownloadingCallback const & cb)