Review fixes.

23 files changed, 99 insertions, 89 deletions

diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp
index 3187676797..6e9f751f5d 100644
--- a/android/jni/com/mapswithme/maps/Framework.cpp
+++ b/android/jni/com/mapswithme/maps/Framework.cpp
@@ -558,7 +558,7 @@ namespace android
 
   void Framework::GetMapsWithoutSearch(vector<string> & out) const
   {
-    ASSERT ( out.empty(), () );
+    ASSERT(out.empty(), ());
 
     ::Platform const & pl = GetPlatform();
 
@@ -569,20 +569,21 @@ namespace android
     {
       CountryFile const countryFile = localFile.GetCountryFile();
       // skip World and WorldCoast
-      if (countryFile.GetNameWithoutExt() != WORLD_FILE_NAME &&
-          countryFile.GetNameWithoutExt() != WORLD_COASTS_FILE_NAME)
+      if (countryFile.GetNameWithoutExt() == WORLD_FILE_NAME ||
+          countryFile.GetNameWithoutExt() == WORLD_COASTS_FILE_NAME)
       {
-        try
-        {
-          FilesContainerR cont(pl.GetCountryReader(localFile, TMapOptions::EMap));
-          if (!cont.IsExist(SEARCH_INDEX_FILE_TAG))
-            out.push_back(countryFile.GetNameWithoutExt());
-        }
-        catch (RootException const & ex)
-        {
-          // sdcard can contain dummy _*.mwm files. Supress this errors.
-          LOG(LWARNING, ("Bad mwm file:", countryFile.GetNameWithoutExt(), "Error:", ex.Msg()));
-        }
+        continue;
+      }
+      try
+      {
+        FilesContainerR cont(pl.GetCountryReader(localFile, TMapOptions::EMap));
+        if (!cont.IsExist(SEARCH_INDEX_FILE_TAG))
+          out.push_back(countryFile.GetNameWithoutExt());
+      }
+      catch (RootException const & ex)
+      {
+        // sdcard can contain dummy _*.mwm files. Suppress these errors.
+        LOG(LWARNING, ("Bad mwm file:", countryFile.GetNameWithoutExt(), "Error:", ex.Msg()));
       }
     }
   }
diff --git a/generator/routing_generator.cpp b/generator/routing_generator.cpp
index 169e09f2ad..2f37a187a3 100644
--- a/generator/routing_generator.cpp
+++ b/generator/routing_generator.cpp
@@ -227,7 +227,10 @@ void BuildRoutingIndex(string const & baseDir, string const & countryName, strin
   classificator::Load();
 
   CountryFile countryFile(countryName);
+
+  // Correct mwm version doesn't matter here - we just need access to mwm files via Index.
   LocalCountryFile localFile(baseDir, countryFile, 0 /* version */);
+  localFile.SyncWithDisk();
   Index index;
   pair<MwmSet::MwmLock, bool> const p = index.Register(localFile);
   if (!p.second)
diff --git a/indexer/index.hpp b/indexer/index.hpp
index ba5e031cf4..20d9d75c5d 100644
--- a/indexer/index.hpp
+++ b/indexer/index.hpp
@@ -66,9 +66,9 @@ public:
   /// Registers a new map.
   ///
   /// \return A pair of an MwmLock and a flag. There are three cases:
-  ///         * the map is newer than the newset registered - returns
+  ///         * the map is newer than the newest registered - returns
   ///           active lock and set flag
-  ///         * the map is older than the newset registered - returns inactive lock and
+  ///         * the map is older than the newest registered - returns inactive lock and
   ///           unset flag.
   ///         * the version of the map equals to the version of the newest registered -
   ///           returns active lock and unset flag.
diff --git a/indexer/indexer_tests/mwm_set_test.cpp b/indexer/indexer_tests/mwm_set_test.cpp
index cc62d5d44e..30b9adebe5 100644
--- a/indexer/indexer_tests/mwm_set_test.cpp
+++ b/indexer/indexer_tests/mwm_set_test.cpp
@@ -10,6 +10,8 @@
 using platform::CountryFile;
 using platform::LocalCountryFile;
 
+using TMwmsInfo = unordered_map<string, shared_ptr<MwmInfo>>;
+
 namespace
 {
 class MwmValue : public MwmSet::MwmValueBase
@@ -38,7 +40,7 @@ public:
   ~TestMwmSet() { Cleanup(); }
 };
 
-void GetMwmsInfo(MwmSet const & mwmSet, unordered_map<string, shared_ptr<MwmInfo>> & mwmsInfo)
+void GetMwmsInfo(MwmSet const & mwmSet, TMwmsInfo & mwmsInfo)
 {
   vector<shared_ptr<MwmInfo>> mwmsInfoList;
   mwmSet.GetMwmsInfo(mwmsInfoList);
@@ -48,8 +50,7 @@ void GetMwmsInfo(MwmSet const & mwmSet, unordered_map<string, shared_ptr<MwmInfo
     mwmsInfo[info->GetCountryName()] = info;
 }
 
-void TestFilesPresence(unordered_map<string, shared_ptr<MwmInfo>> const & mwmsInfo,
-                       initializer_list<string> const & expectedNames)
+void TestFilesPresence(TMwmsInfo const & mwmsInfo, initializer_list<string> const & expectedNames)
 {
   TEST_EQUAL(expectedNames.size(), mwmsInfo.size(), ());
   for (string const & countryFileName : expectedNames)
@@ -61,7 +62,7 @@ void TestFilesPresence(unordered_map<string, shared_ptr<MwmInfo>> const & mwmsIn
 UNIT_TEST(MwmSetSmokeTest)
 {
   TestMwmSet mwmSet;
-  unordered_map<string, shared_ptr<MwmInfo>> mwmsInfo;
+  TMwmsInfo mwmsInfo;
 
   UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("0")));
   UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("1")));
diff --git a/indexer/mwm_set.cpp b/indexer/mwm_set.cpp
index a360a4181b..d6961ebdd0 100644
--- a/indexer/mwm_set.cpp
+++ b/indexer/mwm_set.cpp
@@ -95,8 +95,6 @@ pair<MwmSet::MwmLock, bool> MwmSet::Register(LocalCountryFile const & localFile)
   lock_guard<mutex> lock(m_lock);
 
   CountryFile const & countryFile = localFile.GetCountryFile();
-  string const name = countryFile.GetNameWithoutExt();
-
   MwmId const id = GetMwmIdByCountryFileImpl(countryFile);
   if (!id.IsAlive())
     return RegisterImpl(localFile);
@@ -110,6 +108,7 @@ pair<MwmSet::MwmLock, bool> MwmSet::Register(LocalCountryFile const & localFile)
     return RegisterImpl(localFile);
   }
 
+  string const name = countryFile.GetNameWithoutExt();
   // Update the status of the mwm with the same version.
   if (info->GetVersion() == localFile.GetVersion())
   {
@@ -144,12 +143,11 @@ bool MwmSet::DeregisterImpl(MwmId const & id)
   if (!id.IsAlive())
     return false;
   shared_ptr<MwmInfo> const & info = id.GetInfo();
-  string const name = info->GetCountryName();
 
   if (info->m_lockCount == 0)
   {
     info->SetStatus(MwmInfo::STATUS_DEREGISTERED);
-    vector<shared_ptr<MwmInfo>> & infos = m_info[name];
+    vector<shared_ptr<MwmInfo>> & infos = m_info[info->GetCountryName()];
     infos.erase(remove(infos.begin(), infos.end(), info), infos.end());
     OnMwmDeregistered(info->GetLocalFile());
     return true;
@@ -253,10 +251,10 @@ void MwmSet::UnlockValueImpl(MwmId const & id, TMwmValueBasePtr p)
     return;
 
   shared_ptr<MwmInfo> const & info = id.GetInfo();
-  CHECK_GREATER(info->m_lockCount, 0, ());
+  ASSERT_GREATER(info->m_lockCount, 0, ());
   --info->m_lockCount;
   if (info->m_lockCount == 0 && info->GetStatus() == MwmInfo::STATUS_MARKED_TO_DEREGISTER)
-    CHECK(DeregisterImpl(id), ());
+    VERIFY(DeregisterImpl(id), ());
 
   if (info->IsUpToDate())
   {
diff --git a/indexer/mwm_set.hpp b/indexer/mwm_set.hpp
index 9365e2b81e..dfc225f4c4 100644
--- a/indexer/mwm_set.hpp
+++ b/indexer/mwm_set.hpp
@@ -35,8 +35,8 @@ public:
   enum Status
   {
     STATUS_REGISTERED,            ///< Mwm is registered and up to date.
-    STATUS_MARKED_TO_DEREGISTER,  ///< Mwm is marked to be deregistered as soon as possible
-    STATUS_DEREGISTERED,          ///< Mwm is deregistered
+    STATUS_MARKED_TO_DEREGISTER,  ///< Mwm is marked to be deregistered as soon as possible.
+    STATUS_DEREGISTERED,          ///< Mwm is deregistered.
   };
 
   MwmInfo();
@@ -156,9 +156,9 @@ public:
   /// Registers a new map.
   ///
   /// \return A pair of an MwmLock and a flag. There are three cases:
-  ///         * the map is newer than the newset registered - returns
-  ///           active lock and set flag
-  ///         * the map is older than the newset registered - returns inactive lock and
+  ///         * the map is newer than the newest registered - returns
+  ///           active lock and set flag.
+  ///         * the map is older than the newest registered - returns inactive lock and
   ///           unset flag.
   ///         * the version of the map equals to the version of the newest registered -
   ///           returns active lock and unset flag.
diff --git a/integration_tests/osrm_test_tools.cpp b/integration_tests/osrm_test_tools.cpp
index 72acd2e3d6..4bf6cd94b8 100644
--- a/integration_tests/osrm_test_tools.cpp
+++ b/integration_tests/osrm_test_tools.cpp
@@ -37,8 +37,7 @@ namespace
 
 namespace integration
 {
-shared_ptr<model::FeaturesFetcher> CreateFeaturesFetcher(
-    vector<LocalCountryFile> const & localFiles)
+  shared_ptr<model::FeaturesFetcher> CreateFeaturesFetcher(vector<LocalCountryFile> const & localFiles)
   {
     size_t const maxOpenFileNumber = 1024;
     ChangeMaxNumberOfOpenFiles(maxOpenFileNumber);
@@ -47,11 +46,10 @@ shared_ptr<model::FeaturesFetcher> CreateFeaturesFetcher(
 
     for (LocalCountryFile const & localFile : localFiles)
     {
-      LOG(LINFO, ("Registering mwm:", localFile.GetPath(TMapOptions::EMap)));
       pair<MwmSet::MwmLock, bool> result = featuresFetcher->RegisterMap(localFile);
       if (!result.second)
       {
-        ASSERT(false, ());
+        ASSERT(false, ("Can't register", localFile));
         return nullptr;
       }
     }
diff --git a/map/active_maps_layout.hpp b/map/active_maps_layout.hpp
index e4e7cbb87d..331d17296b 100644
--- a/map/active_maps_layout.hpp
+++ b/map/active_maps_layout.hpp
@@ -3,9 +3,8 @@
 #include "storage/index.hpp"
 #include "storage/storage_defines.hpp"
 
-#include "platform/country_file.hpp"
-
 #include "platform/country_defines.hpp"
+#include "platform/country_file.hpp"
 
 #include "base/buffer_vector.hpp"
 
diff --git a/map/benchmark_engine.cpp b/map/benchmark_engine.cpp
index 3e2a6bcffe..208cd1bb94 100644
--- a/map/benchmark_engine.cpp
+++ b/map/benchmark_engine.cpp
@@ -133,7 +133,6 @@ void BenchmarkEngine::PrepareMaps()
   // add only maps needed for benchmarks
   MapsCollector collector;
   ForEachBenchmarkRecord(collector);
-  string const directory = GetPlatform().WritableDir();
   for (string const & map : collector.m_maps)
   {
     LOG(LINFO, ("Looking for:", map));
diff --git a/map/feature_vec_model.cpp b/map/feature_vec_model.cpp
index e1ba82c165..ca5f1f4ba0 100644
--- a/map/feature_vec_model.cpp
+++ b/map/feature_vec_model.cpp
@@ -18,9 +18,15 @@ using platform::LocalCountryFile;
 
 namespace model
 {
-FeaturesFetcher::FeaturesFetcher() { m_multiIndex.AddObserver(*this); }
+FeaturesFetcher::FeaturesFetcher()
+{
+  m_multiIndex.AddObserver(*this);
+}
 
-FeaturesFetcher::~FeaturesFetcher() { m_multiIndex.RemoveObserver(*this); }
+FeaturesFetcher::~FeaturesFetcher()
+{
+  m_multiIndex.RemoveObserver(*this);
+}
 
 // While reading any files (classificator or mwm), there are 2 types of possible exceptions:
 // Reader::Exception, FileAbsentException.
diff --git a/map/feature_vec_model.hpp b/map/feature_vec_model.hpp
index 39ddabd690..9698d5a745 100644
--- a/map/feature_vec_model.hpp
+++ b/map/feature_vec_model.hpp
@@ -48,9 +48,9 @@ class FeaturesFetcher : public Index::Observer
     /// Registers a new map.
     ///
     /// \return A pair of an MwmLock and a flag. There are three cases:
-    ///         * the map is newer than the newset registered - returns
+    ///         * the map is newer than the newest registered - returns
     ///           active lock and set flag
-    ///         * the map is older than the newset registered - returns inactive lock and
+    ///         * the map is older than the newest registered - returns inactive lock and
     ///           unset flag.
     ///         * the version of the map equals to the version of the newest registered -
     ///           returns active lock and unset flag.
diff --git a/map/framework.cpp b/map/framework.cpp
index 9bbc86ad94..e4083f8004 100644
--- a/map/framework.cpp
+++ b/map/framework.cpp
@@ -372,9 +372,11 @@ void Framework::DeleteCountry(storage::TIndex const & index, TMapOptions opt)
       {
         InvalidateRect(GetCountryBounds(countryFile.GetNameWithoutExt()), true /* doForceUpdate */);
       }
-    }
+      // TODO (@ldragunov, @gorshenin): rewrite routing session to use MwmLocks. Thus,
+      // it won' be needed to reset it after maps update.
       m_routingSession.Reset();
       return;
+    }
     case TMapOptions::ECarRouting:
       m_routingSession.Reset();
       m_storage.DeleteCountry(index, opt);
@@ -421,6 +423,8 @@ void Framework::ShowCountry(TIndex const & index)
 
 void Framework::UpdateAfterDownload(LocalCountryFile const & localFile)
 {
+  // TODO (@ldragunov, @gorshenin): rewrite routing session to use MwmLocks. Thus,
+  // it won' be needed to reset it after maps update.
   m_routingSession.Reset();
 
   if (!HasOptions(localFile.GetFiles(), TMapOptions::EMap))
@@ -455,7 +459,7 @@ void Framework::RegisterAllMaps()
   for (CountryFile const & countryFile : maps)
   {
     shared_ptr<LocalCountryFile> localFile = m_storage.GetLatestLocalFile(countryFile);
-    if (!localFile.get())
+    if (!localFile)
       continue;
     pair<MwmSet::MwmLock, bool> const p = RegisterMap(*localFile);
     if (!p.second)
diff --git a/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp b/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp
index 572388e540..fe9e956d73 100644
--- a/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp
+++ b/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp
@@ -71,7 +71,7 @@ m2::PointD GetPointOnEdge(routing::Edge & e, double posAlong)
 
 void GetNearestPedestrianEdges(Index & index, m2::PointD const & pt, vector<pair<routing::Edge, m2::PointD>> & edges)
 {
-  MwmSet::MwmId const id = index.GetMwmIdByFileName(MAP_FILE);
+  MwmSet::MwmId const id = index.GetMwmIdByCountryFile(CountryFile(MAP_NAME));
   TEST(id.IsAlive(), ());
 
   routing::PedestrianModel const vehicleModel;
@@ -109,7 +109,7 @@ void TestRouter(routing::IRouter & router, m2::PointD const & startPos, m2::Poin
 
 void TestRouters(Index const & index, m2::PointD const & startPos, m2::PointD const & finalPos)
 {
-  auto const countryFileFn = [](m2::PointD const & /* point */){ return MAP_FILE; };
+  auto const countryFileFn = [](m2::PointD const & /* point */) { return MAP_NAME; };
 
   // find route by A*-bidirectional algorithm
   routing::Route routeFoundByAstarBidirectional("");
@@ -129,10 +129,10 @@ void TestTwoPointsOnFeature(m2::PointD const & startPos, m2::PointD const & fina
 {
   classificator::Load();
 
-  Index index;
-
   CountryFile countryFile(MAP_NAME);
   LocalCountryFile localFile = LocalCountryFile::MakeForTesting(MAP_NAME);
+
+  Index index;
   UNUSED_VALUE(index.RegisterMap(localFile));
   TEST(index.IsLoaded(countryFile), ());
   MwmSet::MwmId const id = index.GetMwmIdByCountryFile(countryFile);
@@ -146,8 +146,10 @@ void TestTwoPointsOnFeature(m2::PointD const & startPos, m2::PointD const & fina
   GetNearestPedestrianEdges(index, finalPos, finalEdges);
   TEST(!finalEdges.empty(), ());
 
-  m2::PointD const startPosOnFeature = GetPointOnEdge(startEdges.front().first, 0.0 /* the start point of the feature */ );
-  m2::PointD const finalPosOnFeature = GetPointOnEdge(finalEdges.front().first, 1.0 /* the end point of the feature */ );
+  m2::PointD const startPosOnFeature =
+      GetPointOnEdge(startEdges.front().first, 0.0 /* the start point of the feature */);
+  m2::PointD const finalPosOnFeature =
+      GetPointOnEdge(finalEdges.front().first, 1.0 /* the end point of the feature */);
 
   TestRouters(index, startPosOnFeature, finalPosOnFeature);
 }
diff --git a/platform/local_country_file.cpp b/platform/local_country_file.cpp
index 4cf20444ce..2ccf776392 100644
--- a/platform/local_country_file.cpp
+++ b/platform/local_country_file.cpp
@@ -49,10 +49,7 @@ void LocalCountryFile::DeleteFromDisk(TMapOptions files) const
     if (OnDisk(file) && HasOptions(files, file))
     {
       if (!my::DeleteFileX(GetPath(file)))
-      {
         LOG(LERROR, (file, "from", *this, "wasn't deleted from disk."));
-        ASSERT(false, ());
-      }
     }
   }
 }
diff --git a/platform/local_country_file_utils.cpp b/platform/local_country_file_utils.cpp
index 4c2ca4993a..b5ded8a11c 100644
--- a/platform/local_country_file_utils.cpp
+++ b/platform/local_country_file_utils.cpp
@@ -189,6 +189,7 @@ shared_ptr<LocalCountryFile> PreparePlaceForCountryFiles(CountryFile const & cou
       return make_shared<LocalCountryFile>(directory, countryFile, version);
     }
     default:
+      LOG(LERROR, ("Can't prepare place for", countryFile, "(", version, ") :", ret));
       return shared_ptr<LocalCountryFile>();
   }
 }
diff --git a/platform/platform.cpp b/platform/platform.cpp
index d7b04686cf..bfffb65a7c 100644
--- a/platform/platform.cpp
+++ b/platform/platform.cpp
@@ -153,6 +153,5 @@ string Platform::GetIndexFileName(string const & mwmName, string const & extensi
 ModelReader * Platform::GetCountryReader(platform::LocalCountryFile const & file,
                                          TMapOptions options) const
 {
-  LOG(LINFO, ("Getting country reader for:", file, options));
   return GetReader(file.GetPath(options), "f");
 }
diff --git a/platform/platform_tests/local_country_file_tests.cpp b/platform/platform_tests/local_country_file_tests.cpp
index 12aace8803..46981142f6 100644
--- a/platform/platform_tests/local_country_file_tests.cpp
+++ b/platform/platform_tests/local_country_file_tests.cpp
@@ -60,19 +60,20 @@ public:
     if (m_reset)
       return;
 
-    Platform::EError ret = Platform::RmDir(GetFullPath());
+    string const fullPath = GetFullPath();
+    Platform::EError ret = Platform::RmDir(fullPath);
     switch (ret)
     {
       case Platform::ERR_OK:
         break;
       case Platform::ERR_FILE_DOES_NOT_EXIST:
-        LOG(LWARNING, (GetFullPath(), "was deleted before destruction of ScopedTestDir."));
+        LOG(LWARNING, (fullPath, "was deleted before destruction of ScopedTestDir."));
         break;
       case Platform::ERR_DIRECTORY_NOT_EMPTY:
-        LOG(LWARNING, ("There are files in", GetFullPath()));
+        LOG(LWARNING, ("There are files in", fullPath));
         break;
       default:
-        LOG(LWARNING, ("Platform::RmDir() error for", GetFullPath(), ":", ret));
+        LOG(LWARNING, ("Platform::RmDir() error for", fullPath, ":", ret));
         break;
     }
   }
diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp
index 4f1f9dea72..e52a2bc6c8 100644
--- a/routing/osrm_router.cpp
+++ b/routing/osrm_router.cpp
@@ -352,7 +352,7 @@ public:
 
 OsrmRouter::OsrmRouter(Index const * index, TCountryFileFn const & countryFileFn,
                        TCountryLocalFileFn const & countryLocalFileFn,
-                       RoutingVisualizerFn routingVisualization)
+                       TRoutingVisualizerFn routingVisualization)
     : m_pIndex(index),
       m_indexManager(countryFileFn, countryLocalFileFn, index),
       m_routingVisualization(routingVisualization)
diff --git a/routing/osrm_router.hpp b/routing/osrm_router.hpp
index e10a4a756b..813fffd1cd 100644
--- a/routing/osrm_router.hpp
+++ b/routing/osrm_router.hpp
@@ -38,7 +38,7 @@ public:
 
   OsrmRouter(Index const * index, TCountryFileFn const & countryFileFn,
              TCountryLocalFileFn const & countryLocalFileFn,
-             RoutingVisualizerFn routingVisualization = nullptr);
+             TRoutingVisualizerFn routingVisualization = nullptr);
 
   virtual string GetName() const;
 
diff --git a/routing/routing_mapping.cpp b/routing/routing_mapping.cpp
index c760cf1921..a7c6f3bf97 100644
--- a/routing/routing_mapping.cpp
+++ b/routing/routing_mapping.cpp
@@ -143,7 +143,7 @@ TRoutingMappingPtr RoutingIndexManager::GetMappingByName(string const & mapName)
 {
   shared_ptr<platform::LocalCountryFile> localFile = m_countryLocalFileFn(mapName);
   // Return invalid mapping when file does not exist.
-  if (!localFile.get())
+  if (!localFile)
     return RoutingMapping::MakeInvalid(platform::CountryFile(mapName));
 
   // Check if we have already loaded this file.
diff --git a/storage/storage.cpp b/storage/storage.cpp
index 8994f873cd..b9f7e75f58 100644
--- a/storage/storage.cpp
+++ b/storage/storage.cpp
@@ -169,16 +169,17 @@ CountriesContainerT const & NodeFromIndex(CountriesContainerT const & root, TInd
   // complex logic to avoid [] out_of_bounds exceptions
   if (index.m_group == TIndex::INVALID || index.m_group >= static_cast<int>(root.SiblingsCount()))
     return root;
-  else
+  if (index.m_country == TIndex::INVALID ||
+      index.m_country >= static_cast<int>(root[index.m_group].SiblingsCount()))
+  {
+    return root[index.m_group];
+  }
+  if (index.m_region == TIndex::INVALID ||
+      index.m_region >= static_cast<int>(root[index.m_group][index.m_country].SiblingsCount()))
   {
-    if (index.m_country == TIndex::INVALID ||
-        index.m_country >= static_cast<int>(root[index.m_group].SiblingsCount()))
-      return root[index.m_group];
-    if (index.m_region == TIndex::INVALID ||
-        index.m_region >= static_cast<int>(root[index.m_group][index.m_country].SiblingsCount()))
-      return root[index.m_group][index.m_country];
-    return root[index.m_group][index.m_country][index.m_region];
+    return root[index.m_group][index.m_country];
   }
+  return root[index.m_group][index.m_country][index.m_region];
 }
 
 Country const & Storage::CountryByIndex(TIndex const & index) const
@@ -265,7 +266,7 @@ shared_ptr<LocalCountryFile> Storage::GetLatestLocalFile(TIndex const & index) c
 
 TStatus Storage::CountryStatus(TIndex const & index) const
 {
-  // first, check if we already downloading this country or have in in the queue
+  // Check if we already downloading this country or have it in the queue
   if (IsCountryInQueue(index))
   {
     if (IsCountryFirstInQueue(index))
@@ -274,7 +275,7 @@ TStatus Storage::CountryStatus(TIndex const & index) const
       return TStatus::EInQueue;
   }
 
-  // second, check if this country has failed while downloading
+  // Check if this country has failed while downloading.
   if (m_failedCountries.count(index) > 0)
     return TStatus::EDownloadFailed;
 
@@ -770,7 +771,7 @@ shared_ptr<LocalCountryFile> Storage::GetLocalFile(TIndex const & index, int64_t
 
 void Storage::RegisterCountryFiles(shared_ptr<LocalCountryFile> localFile)
 {
-  ASSERT(localFile.get(), ());
+  CHECK(localFile.get(), ());
   localFile->SyncWithDisk();
 
   TIndex const index = FindIndexByFile(localFile->GetCountryFile().GetNameWithoutExt());
@@ -784,7 +785,7 @@ void Storage::RegisterCountryFiles(shared_ptr<LocalCountryFile> localFile)
 void Storage::RegisterCountryFiles(TIndex const & index, string const & directory, int64_t version)
 {
   shared_ptr<LocalCountryFile> localFile = GetLocalFile(index, version);
-  if (localFile.get() != nullptr)
+  if (localFile)
     return;
 
   CountryFile const & countryFile = GetCountryFile(index);
@@ -848,7 +849,7 @@ bool Storage::DeleteCountryFilesFromDownloader(TIndex const & index, TMapOptions
 
 void Storage::KickDownloaderAfterDeletionOfCountryFiles(TIndex const & index)
 {
-  // Do nothing when there're no counties to download or when downloader is busy.
+  // Do nothing when there're no countries to download or when downloader is busy.
   if (m_queue.empty() || !m_downloader->IsIdle())
     return;
   if (IsCountryFirstInQueue(index))
diff --git a/storage/storage.hpp b/storage/storage.hpp
index c080986061..0e3b1f1ccb 100644
--- a/storage/storage.hpp
+++ b/storage/storage.hpp
@@ -38,7 +38,7 @@ class Storage
   /// RunOnUIThread.  If not, at least use a syncronization object.
   TQueue m_queue;
 
-  /// stores countries which download has failed recently
+  /// stores countries whose download has failed recently
   typedef set<TIndex> TCountriesSet;
   TCountriesSet m_failedCountries;
 
diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp
index 55eefbf7d9..9bc206024c 100644
--- a/storage/storage_tests/storage_tests.cpp
+++ b/storage/storage_tests/storage_tests.cpp
@@ -48,22 +48,22 @@ public:
     m_slot = m_storage.Subscribe(
         bind(&CountryDownloaderChecker::OnCountryStatusChanged, this, _1),
         bind(&CountryDownloaderChecker::OnCountryDownloadingProgress, this, _1, _2));
-    CHECK(m_index.IsValid(), (m_countryFile));
-    CHECK(!m_transitionList.empty(), (m_countryFile));
+    TEST(m_index.IsValid(), (m_countryFile));
+    TEST(!m_transitionList.empty(), (m_countryFile));
   }
 
   void StartDownload()
   {
-    CHECK_EQUAL(0, m_currStatus, (m_countryFile));
-    CHECK_LESS(m_currStatus, m_transitionList.size(), (m_countryFile));
-    CHECK_EQUAL(m_transitionList[m_currStatus], m_storage.CountryStatusEx(m_index),
+    TEST_EQUAL(0, m_currStatus, (m_countryFile));
+    TEST_LESS(m_currStatus, m_transitionList.size(), (m_countryFile));
+    TEST_EQUAL(m_transitionList[m_currStatus], m_storage.CountryStatusEx(m_index),
                 (m_countryFile));
     m_storage.DownloadCountry(m_index, m_files);
   }
 
   virtual ~CountryDownloaderChecker()
   {
-    CHECK_EQUAL(m_currStatus + 1, m_transitionList.size(), (m_countryFile));
+    TEST_EQUAL(m_currStatus + 1, m_transitionList.size(), (m_countryFile));
     m_storage.Unsubscribe(m_slot);
   }
 
@@ -76,8 +76,8 @@ private:
     TStatus const nextStatus = m_storage.CountryStatusEx(m_index);
     LOG(LINFO, (m_countryFile, "status transition: from", m_transitionList[m_currStatus], "to",
                 nextStatus));
-    CHECK_LESS(m_currStatus + 1, m_transitionList.size(), (m_countryFile));
-    CHECK_EQUAL(nextStatus, m_transitionList[m_currStatus + 1], (m_countryFile));
+    TEST_LESS(m_currStatus + 1, m_transitionList.size(), (m_countryFile));
+    TEST_EQUAL(nextStatus, m_transitionList[m_currStatus + 1], (m_countryFile));
     ++m_currStatus;
     if (m_transitionList[m_currStatus] == TStatus::EDownloading)
     {
@@ -93,12 +93,12 @@ private:
 
     LOG(LINFO, (m_countryFile, "downloading progress:", progress));
 
-    CHECK_GREATER(progress.first, m_bytesDownloaded, (m_countryFile));
+    TEST_GREATER(progress.first, m_bytesDownloaded, (m_countryFile));
     m_bytesDownloaded = progress.first;
-    CHECK_LESS_OR_EQUAL(m_bytesDownloaded, m_totalBytesToDownload, (m_countryFile));
+    TEST_LESS_OR_EQUAL(m_bytesDownloaded, m_totalBytesToDownload, (m_countryFile));
 
     LocalAndRemoteSizeT localAndRemoteSize = m_storage.CountrySizeInBytes(m_index, m_files);
-    CHECK_EQUAL(m_totalBytesToDownload, localAndRemoteSize.second, (m_countryFile));
+    TEST_EQUAL(m_totalBytesToDownload, localAndRemoteSize.second, (m_countryFile));
   }
 
   Storage & m_storage;
@@ -167,15 +167,15 @@ shared_ptr<LocalCountryFile> CreateDummyMapFile(CountryFile const & countryFile,
 {
   shared_ptr<LocalCountryFile> localFile =
       platform::PreparePlaceForCountryFiles(countryFile, version);
-  CHECK(localFile.get(), ("Can't prepare place for", countryFile, "(version ", version, ")"));
+  TEST(localFile.get(), ("Can't prepare place for", countryFile, "(version ", version, ")"));
   {
     string const zeroes(size, '\0');
     FileWriter writer(localFile->GetPath(TMapOptions::EMap));
     writer.Write(zeroes.data(), zeroes.size());
   }
   localFile->SyncWithDisk();
-  CHECK_EQUAL(TMapOptions::EMap, localFile->GetFiles(), ());
-  CHECK_EQUAL(size, localFile->GetSize(TMapOptions::EMap), ());
+  TEST_EQUAL(TMapOptions::EMap, localFile->GetFiles(), ());
+  TEST_EQUAL(size, localFile->GetSize(TMapOptions::EMap), ());
   return localFile;
 }