[index] Fixed notifications logic, added UPDATE event.

indexer/index.cpp

@@ -75,23 +75,11 @@ unique_ptr<MwmSet::MwmValueBase> Index::CreateValue(MwmInfo & info) const
 
 pair<MwmSet::MwmId, MwmSet::RegResult> Index::RegisterMap(LocalCountryFile const & localFile)
 {
-  auto result = Register(localFile);
-  if (result.first.IsAlive() && result.second == MwmSet::RegResult::Success)
-    m_observers.ForEach(&Observer::OnMapRegistered, localFile);
-  return result;
+  return Register(localFile);
 }
 
 bool Index::DeregisterMap(CountryFile const & countryFile) { return Deregister(countryFile); }
 
-bool Index::AddObserver(Observer & observer) { return m_observers.Add(observer); }
-
-bool Index::RemoveObserver(Observer const & observer) { return m_observers.Remove(observer); }
-
-void Index::OnMwmDeregistered(LocalCountryFile const & localFile)
-{
-  m_observers.ForEach(&Observer::OnMapDeregistered, localFile);
-}
-
 //////////////////////////////////////////////////////////////////////////////////
 // Index::FeaturesLoaderGuard implementation
 //////////////////////////////////////////////////////////////////////////////////

indexer/index.hpp

@@ -14,7 +14,6 @@
 #include "defines.hpp"
 
 #include "base/macros.hpp"
-#include "base/observer_list.hpp"
 
 #include "std/algorithm.hpp"
 #include "std/limits.hpp"
@@ -52,28 +51,9 @@ protected:
   unique_ptr<MwmInfo> CreateInfo(platform::LocalCountryFile const & localFile) const override;
 
   unique_ptr<MwmValueBase> CreateValue(MwmInfo & info) const override;
-
-  void OnMwmDeregistered(platform::LocalCountryFile const & localFile) override;
   //@}
 
 public:
-  /// An Observer interface to MwmSet. Note that these functions can
-  /// be called from *ANY* thread because most signals are sent when
-  /// some thread releases its MwmHandle, so overrides must be as fast
-  /// as possible and non-blocking when it's possible.
-  class Observer
-  {
-  public:
-    virtual ~Observer() = default;
-
-    /// Called when a map is registered for a first time and can be
-    /// used.
-    virtual void OnMapRegistered(platform::LocalCountryFile const & localFile) {}
-
-    /// Called when a map is deregistered and can not be used.
-    virtual void OnMapDeregistered(platform::LocalCountryFile const & localFile) {}
-  };
-
   /// Registers a new map.
   pair<MwmId, RegResult> RegisterMap(platform::LocalCountryFile const & localFile);
 
@@ -84,10 +64,6 @@ public:
   ///         now, returns false.
   bool DeregisterMap(platform::CountryFile const & countryFile);
 
-  bool AddObserver(Observer & observer);
-
-  bool RemoveObserver(Observer const & observer);
-
 private:
 
   template <typename F> class ReadMWMFunctor
@@ -370,6 +346,4 @@ private:
       editor.ForEachFeatureInMwmRectAndScale(worldID[1], f, rect, scale);
     }
   }
-
-  my::ObserverList<Observer> m_observers;
 };

indexer/indexer_tests/index_test.cpp

@@ -2,6 +2,7 @@
 
 #include "indexer/data_header.hpp"
 #include "indexer/index.hpp"
+#include "indexer/mwm_set.hpp"
 
 #include "coding/file_name_utils.hpp"
 #include "coding/internal/file_data.hpp"
@@ -23,133 +24,161 @@ using platform::LocalCountryFile;
 
 namespace
 {
-class Observer : public Index::Observer
+class IndexTest : public MwmSet::Observer
 {
 public:
-  Observer() : m_numRegisteredCalls(0), m_numDeregisteredCalls(0) {}
+  IndexTest() { TEST(m_index.AddObserver(*this), ()); }
 
-  ~Observer() { CheckExpectations(); }
+  ~IndexTest() override { TEST(m_index.RemoveObserver(*this), ()); }
 
-  void ExpectRegisteredMap(platform::LocalCountryFile const & localFile)
+  void ExpectRegistered(platform::LocalCountryFile const & localFile)
   {
-    m_expectedRegisteredMaps.push_back(localFile);
+    AddEvent(m_expected, MwmSet::Event::TYPE_REGISTERED, localFile);
   }
 
-  void ExpectDeregisteredMap(platform::LocalCountryFile const & localFile)
+  void ExpectUpdated(platform::LocalCountryFile const & newFile,
+                     platform::LocalCountryFile const & oldFile)
   {
-    m_expectedDeregisteredMaps.push_back(localFile);
+    AddEvent(m_expected, MwmSet::Event::TYPE_UPDATED, newFile, oldFile);
   }
 
-  void CheckExpectations()
+  void ExpectDeregistered(platform::LocalCountryFile const & localFile)
   {
-    CHECK_EQUAL(m_numRegisteredCalls, m_expectedRegisteredMaps.size(), ());
-    CHECK_EQUAL(m_numDeregisteredCalls, m_expectedDeregisteredMaps.size(), ());
+    AddEvent(m_expected, MwmSet::Event::TYPE_DEREGISTERED, localFile);
   }
 
-  // Index::Observer overrides:
+  // Checks expectations and clears them.
+  bool Check()
+  {
+    bool ok = true;
+
+    size_t i = 0;
+    for (; i < m_actual.size() && m_expected.size(); ++i)
+    {
+      if (m_actual[i] != m_expected[i])
+      {
+        LOG(LINFO, ("Check failed: expected:", m_expected[i], "actual:", m_actual[i]));
+        ok = false;
+      }
+    }
+    for (; i < m_actual.size(); ++i)
+    {
+      LOG(LINFO, ("Unexpected event:", m_actual[i]));
+      ok = false;
+    }
+    for (; i < m_expected.size(); ++i)
+    {
+      LOG(LINFO, ("Unmet expectation:", m_expected[i]));
+      ok = false;
+    }
+
+    m_actual.clear();
+    m_expected.clear();
+    return ok;
+  }
+
+  // MwmSet::Observer overrides:
   void OnMapRegistered(platform::LocalCountryFile const & localFile) override
   {
-    CHECK_LESS(m_numRegisteredCalls, m_expectedRegisteredMaps.size(),
-               ("Unexpected OnMapRegistered() call (", m_numRegisteredCalls, "): ", localFile));
-    CHECK_EQUAL(m_expectedRegisteredMaps[m_numRegisteredCalls], localFile, (m_numRegisteredCalls));
-    ++m_numRegisteredCalls;
+    AddEvent(m_actual, MwmSet::Event::TYPE_REGISTERED, localFile);
+  }
+
+  void OnMapUpdated(platform::LocalCountryFile const & newFile,
+                    platform::LocalCountryFile const & oldFile) override
+  {
+    AddEvent(m_actual, MwmSet::Event::TYPE_UPDATED, newFile, oldFile);
   }
 
   void OnMapDeregistered(platform::LocalCountryFile const & localFile) override
   {
-    CHECK_LESS(m_numDeregisteredCalls, m_expectedDeregisteredMaps.size(),
-               ("Unexpected OnMapDeregistered() call (", m_numDeregisteredCalls, "): ", localFile));
-    CHECK_EQUAL(m_expectedDeregisteredMaps[m_numDeregisteredCalls], localFile,
-                (m_numDeregisteredCalls));
-    ++m_numDeregisteredCalls;
+    AddEvent(m_actual, MwmSet::Event::TYPE_DEREGISTERED, localFile);
   }
 
-  inline size_t MapRegisteredCalls() const { return m_numRegisteredCalls; }
-  inline size_t MapDeregisteredCalls() const { return m_numDeregisteredCalls; }
+protected:
+  template <typename... Args>
+  void AddEvent(vector<MwmSet::Event> & events, Args... args)
+  {
+    events.emplace_back(forward<Args>(args)...);
+  }
 
-private:
-  size_t m_numRegisteredCalls;
-  size_t m_numDeregisteredCalls;
-
-  vector<LocalCountryFile> m_expectedRegisteredMaps;
-  vector<LocalCountryFile> m_expectedDeregisteredMaps;
+  Index m_index;
+  vector<MwmSet::Event> m_expected;
+  vector<MwmSet::Event> m_actual;
 };
 }  // namespace
 
-UNIT_TEST(Index_Parse)
+UNIT_CLASS_TEST(IndexTest, Parse)
 {
-  Index index;
-  UNUSED_VALUE(index.RegisterMap(platform::LocalCountryFile::MakeForTesting("minsk-pass")));
+  UNUSED_VALUE(m_index.RegisterMap(platform::LocalCountryFile::MakeForTesting("minsk-pass")));
 
   // Make sure that index is actually parsed.
   NoopFunctor fn;
-  index.ForEachInScale(fn, 15);
+  m_index.ForEachInScale(fn, 15);
 }
 
-UNIT_TEST(Index_MwmStatusNotifications)
+UNIT_CLASS_TEST(IndexTest, StatusNotifications)
 {
-  Platform & platform = GetPlatform();
-  string const mapsDir = platform.WritableDir();
-  CountryFile const countryFile("minsk-pass");
+  string const mapsDir = GetPlatform().WritableDir();
+  CountryFile const country("minsk-pass");
 
   // These two classes point to the same file, but will be considered
   // by Index as distinct files because versions are artificially set
   // to different numbers.
-  LocalCountryFile const localFileV1(mapsDir, countryFile, 1 /* version */);
-  LocalCountryFile const localFileV2(mapsDir, countryFile, 2 /* version */);
+  LocalCountryFile const file1(mapsDir, country, 1 /* version */);
+  LocalCountryFile const file2(mapsDir, country, 2 /* version */);
 
-  Index index;
-  Observer observer;
-  index.AddObserver(observer);
-
-  TEST_EQUAL(0, observer.MapRegisteredCalls(), ());
-
-  MwmSet::MwmId localFileV1Id;
+  MwmSet::MwmId id1;
 
   // Checks that observers are triggered after map registration.
   {
-    observer.ExpectRegisteredMap(localFileV1);
-    auto p = index.RegisterMap(localFileV1);
-    TEST(p.first.IsAlive(), ());
-    TEST_EQUAL(MwmSet::RegResult::Success, p.second, ());
-    observer.CheckExpectations();
-    localFileV1Id = p.first;
+    auto result = m_index.RegisterMap(file1);
+    TEST_EQUAL(MwmSet::RegResult::Success, result.second, ());
+
+    id1 = result.first;
+    TEST(id1.IsAlive(), ());
+
+    ExpectRegistered(file1);
+    TEST(Check(), ());
   }
 
   // Checks that map can't registered twice.
   {
-    auto p = index.RegisterMap(localFileV1);
-    TEST(p.first.IsAlive(), ());
-    TEST_EQUAL(MwmSet::RegResult::VersionAlreadyExists, p.second, ());
-    observer.CheckExpectations();
-    TEST_EQUAL(localFileV1Id, p.first, ());
+    auto result = m_index.RegisterMap(file1);
+    TEST_EQUAL(MwmSet::RegResult::VersionAlreadyExists, result.second, ());
+
+    TEST(result.first.IsAlive(), ());
+    TEST_EQUAL(id1, result.first, ());
+
+    TEST(Check(), ());
   }
 
   // Checks that observers are notified when map is updated.
-  MwmSet::MwmId localFileV2Id;
+  MwmSet::MwmId id2;
   {
-    observer.ExpectRegisteredMap(localFileV2);
-    observer.ExpectDeregisteredMap(localFileV1);
-    auto p = index.RegisterMap(localFileV2);
-    TEST(p.first.IsAlive(), ());
-    TEST_EQUAL(MwmSet::RegResult::Success, p.second, ());
-    observer.CheckExpectations();
-    localFileV2Id = p.first;
-    TEST_NOT_EQUAL(localFileV1Id, localFileV2Id, ());
+    auto result = m_index.RegisterMap(file2);
+    TEST_EQUAL(MwmSet::RegResult::Success, result.second, ());
+
+    id2 = result.first;
+    TEST(id2.IsAlive(), ());
+    TEST_NOT_EQUAL(id1, id2, ());
+
+    ExpectUpdated(file2, file1);
+    TEST(Check(), ());
   }
 
   // Tries to deregister a map in presence of an active lock. Map
   // should be marked "to be removed" but can't be deregistered. After
   // leaving the inner block the map should be deregistered.
   {
-    MwmSet::MwmHandle const handle = index.GetMwmHandleByCountryFile(countryFile);
-    TEST(handle.IsAlive(), ());
+    {
+      MwmSet::MwmHandle const handle = m_index.GetMwmHandleByCountryFile(country);
+      TEST(handle.IsAlive(), ());
 
-    TEST(!index.DeregisterMap(countryFile), ());
-    observer.CheckExpectations();
+      TEST(!m_index.DeregisterMap(country), ());
+      TEST(Check(), ());
+    }
 
-    observer.ExpectDeregisteredMap(localFileV2);
+    ExpectDeregistered(file2);
+    TEST(Check(), ());
   }
-  observer.CheckExpectations();
-  index.RemoveObserver(observer);
 }

indexer/mwm_set.cpp

@@ -85,38 +85,57 @@ MwmSet::MwmId MwmSet::GetMwmIdByCountryFileImpl(CountryFile const & countryFile)
 
 pair<MwmSet::MwmId, MwmSet::RegResult> MwmSet::Register(LocalCountryFile const & localFile)
 {
-  lock_guard<mutex> lock(m_lock);
+  pair<MwmSet::MwmId, MwmSet::RegResult> result;
+  WithEventLog([&](EventList & events)
+               {
+                 CountryFile const & countryFile = localFile.GetCountryFile();
+                 MwmId const id = GetMwmIdByCountryFileImpl(countryFile);
+                 if (!id.IsAlive())
+                 {
+                   result = RegisterImpl(localFile, events);
+                   return;
+                 }
 
-  CountryFile const & countryFile = localFile.GetCountryFile();
-  MwmId const id = GetMwmIdByCountryFileImpl(countryFile);
-  if (!id.IsAlive())
-    return RegisterImpl(localFile);
+                 shared_ptr<MwmInfo> info = id.GetInfo();
 
-  shared_ptr<MwmInfo> info = id.GetInfo();
+                 // Deregister old mwm for the country.
+                 if (info->GetVersion() < localFile.GetVersion())
+                 {
+                   EventList subEvents;
+                   DeregisterImpl(id, subEvents);
+                   result = RegisterImpl(localFile, subEvents);
 
-  // Deregister old mwm for the country.
-  if (info->GetVersion() < localFile.GetVersion())
-  {
-    DeregisterImpl(id);
-    return RegisterImpl(localFile);
-  }
+                   // In the case of success all sub-events are
+                   // replaced with a single UPDATE event. Otherwise,
+                   // sub-events are reported as is.
+                   if (result.second == MwmSet::RegResult::Success)
+                     events.Add(Event(Event::TYPE_UPDATED, localFile, info->GetLocalFile()));
+                   else
+                     events.Append(subEvents);
+                   return;
+                 }
 
-  string const name = countryFile.GetNameWithoutExt();
-  // Update the status of the mwm with the same version.
-  if (info->GetVersion() == localFile.GetVersion())
-  {
-    LOG(LINFO, ("Updating already registered mwm:", name));
-    info->SetStatus(MwmInfo::STATUS_REGISTERED);
-    info->m_file = localFile;
-    return make_pair(id, RegResult::VersionAlreadyExists);
-  }
+                 string const name = countryFile.GetNameWithoutExt();
+                 // Update the status of the mwm with the same version.
+                 if (info->GetVersion() == localFile.GetVersion())
+                 {
+                   LOG(LINFO, ("Updating already registered mwm:", name));
+                   SetStatus(*info, MwmInfo::STATUS_REGISTERED, events);
+                   info->m_file = localFile;
+                   result = make_pair(id, RegResult::VersionAlreadyExists);
+                   return;
+                 }
 
-  LOG(LWARNING, ("Trying to add too old (", localFile.GetVersion(), ") mwm (", name,
-                 "), current version:", info->GetVersion()));
-  return make_pair(MwmId(), RegResult::VersionTooOld);
+                 LOG(LWARNING, ("Trying to add too old (", localFile.GetVersion(), ") mwm (",
+                                name, "), current version:", info->GetVersion()));
+                 result = make_pair(MwmId(), RegResult::VersionTooOld);
+                 return;
+               });
+  return result;
 }
 
-pair<MwmSet::MwmId, MwmSet::RegResult> MwmSet::RegisterImpl(LocalCountryFile const & localFile)
+pair<MwmSet::MwmId, MwmSet::RegResult> MwmSet::RegisterImpl(LocalCountryFile const & localFile,
+                                                            EventList & events)
 {
   // This function can throw an exception for a bad mwm file.
   shared_ptr<MwmInfo> info(CreateInfo(localFile));
@@ -124,13 +143,13 @@ pair<MwmSet::MwmId, MwmSet::RegResult> MwmSet::RegisterImpl(LocalCountryFile con
     return make_pair(MwmId(), RegResult::UnsupportedFileFormat);
 
   info->m_file = localFile;
-  info->SetStatus(MwmInfo::STATUS_REGISTERED);
+  SetStatus(*info, MwmInfo::STATUS_REGISTERED, events);
   m_info[localFile.GetCountryName()].push_back(info);
 
   return make_pair(MwmId(info), RegResult::Success);
 }
 
-bool MwmSet::DeregisterImpl(MwmId const & id)
+bool MwmSet::DeregisterImpl(MwmId const & id, EventList & events)
 {
   if (!id.IsAlive())
     return false;
@@ -138,29 +157,32 @@ bool MwmSet::DeregisterImpl(MwmId const & id)
   shared_ptr<MwmInfo> const & info = id.GetInfo();
   if (info->m_numRefs == 0)
   {
-    info->SetStatus(MwmInfo::STATUS_DEREGISTERED);
+    SetStatus(*info, MwmInfo::STATUS_DEREGISTERED, events);
     vector<shared_ptr<MwmInfo>> & infos = m_info[info->GetCountryName()];
     infos.erase(remove(infos.begin(), infos.end(), info), infos.end());
-    OnMwmDeregistered(info->GetLocalFile());
     return true;
   }
 
-  info->SetStatus(MwmInfo::STATUS_MARKED_TO_DEREGISTER);
+  SetStatus(*info, MwmInfo::STATUS_MARKED_TO_DEREGISTER, events);
   return false;
 }
 
 bool MwmSet::Deregister(CountryFile const & countryFile)
 {
-  lock_guard<mutex> lock(m_lock);
-  return DeregisterImpl(countryFile);
+  bool deregistered = false;
+  WithEventLog([&](EventList & events)
+               {
+                 deregistered = DeregisterImpl(countryFile, events);
+               });
+  return deregistered;
 }
 
-bool MwmSet::DeregisterImpl(CountryFile const & countryFile)
+bool MwmSet::DeregisterImpl(CountryFile const & countryFile, EventList & events)
 {
   MwmId const id = GetMwmIdByCountryFileImpl(countryFile);
   if (!id.IsAlive())
     return false;
-  bool const deregistered = DeregisterImpl(id);
+  bool const deregistered = DeregisterImpl(id, events);
   ClearCache(id);
   return deregistered;
 }
@@ -185,13 +207,54 @@ void MwmSet::GetMwmsInfo(vector<shared_ptr<MwmInfo>> & info) const
   }
 }
 
-unique_ptr<MwmSet::MwmValueBase> MwmSet::LockValue(MwmId const & id)
+void MwmSet::SetStatus(MwmInfo & info, MwmInfo::Status status, EventList & events)
 {
-  lock_guard<mutex> lock(m_lock);
-  return LockValueImpl(id);
+  MwmInfo::Status oldStatus = info.SetStatus(status);
+  if (oldStatus == status)
+    return;
+
+  switch (status)
+  {
+  case MwmInfo::STATUS_REGISTERED:
+    events.Add(Event(Event::TYPE_REGISTERED, info.GetLocalFile()));
+    break;
+  case MwmInfo::STATUS_MARKED_TO_DEREGISTER: break;
+  case MwmInfo::STATUS_DEREGISTERED:
+    events.Add(Event(Event::TYPE_DEREGISTERED, info.GetLocalFile()));
+    break;
+  }
 }
 
-unique_ptr<MwmSet::MwmValueBase> MwmSet::LockValueImpl(MwmId const & id)
+void MwmSet::ProcessEventList(EventList & events)
+{
+  for (auto const & event : events.Get())
+  {
+    switch (event.m_type)
+    {
+    case Event::TYPE_REGISTERED:
+      m_observers.ForEach(&Observer::OnMapRegistered, event.m_file);
+      break;
+    case Event::TYPE_UPDATED:
+      m_observers.ForEach(&Observer::OnMapUpdated, event.m_file, event.m_oldFile);
+      break;
+    case Event::TYPE_DEREGISTERED:
+      m_observers.ForEach(&Observer::OnMapDeregistered, event.m_file);
+      break;
+    }
+  }
+}
+
+unique_ptr<MwmSet::MwmValueBase> MwmSet::LockValue(MwmId const & id)
+{
+  unique_ptr<MwmSet::MwmValueBase> result;
+  WithEventLog([&](EventList & events)
+               {
+                 result = LockValueImpl(id, events);
+               });
+  return result;
+}
+
+unique_ptr<MwmSet::MwmValueBase> MwmSet::LockValueImpl(MwmId const & id, EventList & events)
 {
   if (!id.IsAlive())
     return nullptr;
@@ -224,18 +287,20 @@ unique_ptr<MwmSet::MwmValueBase> MwmSet::LockValueImpl(MwmId const & id)
     LOG(LERROR, ("Can't create MWMValue for", info->GetCountryName(), "Reason", ex.what()));
 
     --info->m_numRefs;
-    DeregisterImpl(id);
+    DeregisterImpl(id, events);
     return nullptr;
   }
 }
 
 void MwmSet::UnlockValue(MwmId const & id, unique_ptr<MwmValueBase> && p)
 {
-  lock_guard<mutex> lock(m_lock);
-  UnlockValueImpl(id, move(p));
+  WithEventLog([&](EventList & events)
+               {
+                 UnlockValueImpl(id, move(p), events);
+               });
 }
 
-void MwmSet::UnlockValueImpl(MwmId const & id, unique_ptr<MwmValueBase> && p)
+void MwmSet::UnlockValueImpl(MwmId const & id, unique_ptr<MwmValueBase> && p, EventList & events)
 {
   ASSERT(id.IsAlive() && p, (id));
   if (!id.IsAlive() || !p)
@@ -245,7 +310,7 @@ void MwmSet::UnlockValueImpl(MwmId const & id, unique_ptr<MwmValueBase> && p)
   ASSERT_GREATER(info->m_numRefs, 0, ());
   --info->m_numRefs;
   if (info->m_numRefs == 0 && info->GetStatus() == MwmInfo::STATUS_MARKED_TO_DEREGISTER)
-    VERIFY(DeregisterImpl(id), ());
+    VERIFY(DeregisterImpl(id, events), ());
 
   if (info->IsUpToDate())
   {
@@ -282,21 +347,29 @@ MwmSet::MwmId MwmSet::GetMwmIdByCountryFile(CountryFile const & countryFile) con
 
 MwmSet::MwmHandle MwmSet::GetMwmHandleByCountryFile(CountryFile const & countryFile)
 {
-  lock_guard<mutex> lock(m_lock);
-  return GetMwmHandleByIdImpl(GetMwmIdByCountryFileImpl(countryFile));
+  MwmSet::MwmHandle handle;
+  WithEventLog([&](EventList & events)
+               {
+                 handle = GetMwmHandleByIdImpl(GetMwmIdByCountryFileImpl(countryFile), events);
+               });
+  return handle;
 }
 
 MwmSet::MwmHandle MwmSet::GetMwmHandleById(MwmId const & id)
 {
-  lock_guard<mutex> lock(m_lock);
-  return GetMwmHandleByIdImpl(id);
+  MwmSet::MwmHandle handle;
+  WithEventLog([&](EventList & events)
+               {
+                 handle = GetMwmHandleByIdImpl(id, events);
+               });
+  return handle;
 }
 
-MwmSet::MwmHandle MwmSet::GetMwmHandleByIdImpl(MwmId const & id)
+MwmSet::MwmHandle MwmSet::GetMwmHandleByIdImpl(MwmId const & id, EventList & events)
 {
   unique_ptr<MwmValueBase> value;
   if (id.IsAlive())
-    value = LockValueImpl(id);
+    value = LockValueImpl(id, events);
   return MwmHandle(*this, id, move(value));
 }
 
@@ -330,3 +403,24 @@ string DebugPrint(MwmSet::RegResult result)
       return "UnsupportedFileFormat";
   }
 }
+
+string DebugPrint(MwmSet::Event::Type type)
+{
+  switch (type)
+  {
+    case MwmSet::Event::TYPE_REGISTERED: return "Registered";
+    case MwmSet::Event::TYPE_UPDATED: return "Updated";
+    case MwmSet::Event::TYPE_DEREGISTERED: return "Deregistered";
+  }
+  return "Undefined";
+}
+
+string DebugPrint(MwmSet::Event const & event)
+{
+  ostringstream os;
+  os << "MwmSet::Event [" << DebugPrint(event.m_type) << ", " << DebugPrint(event.m_file);
+  if (event.m_type == MwmSet::Event::TYPE_UPDATED)
+    os << ", " << DebugPrint(event.m_oldFile);
+  os << "]";
+  return os.str();
+}

indexer/mwm_set.hpp

@@ -18,6 +18,7 @@
 #include "std/utility.hpp"
 #include "std/vector.hpp"
 
+#include "base/observer_list.hpp"
 
 /// Information about stored mwm.
 class MwmInfo
@@ -69,7 +70,12 @@ public:
   uint8_t GetNumRefs() const { return m_numRefs; }
 
 private:
-  inline void SetStatus(Status status) { m_status = status; }
+  inline Status SetStatus(Status status)
+  {
+    Status result = m_status;
+    m_status = status;
+    return result;
+  }
 
   platform::LocalCountryFile m_file;  ///< Path to the mwm file.
   atomic<Status> m_status;            ///< Current country status.
@@ -92,6 +98,7 @@ public:
     {
       return (m_info && m_info->GetStatus() != MwmInfo::STATUS_DEREGISTERED);
     }
+    shared_ptr<MwmInfo> & GetInfo() { return m_info; }
     shared_ptr<MwmInfo> const & GetInfo() const { return m_info; }
 
     inline bool operator==(MwmId const & rhs) const { return GetInfo() == rhs.GetInfo(); }
@@ -153,6 +160,58 @@ public:
     DISALLOW_COPY(MwmHandle);
   };
 
+  struct Event
+  {
+    enum Type
+    {
+      TYPE_REGISTERED,
+      TYPE_DEREGISTERED,
+      TYPE_UPDATED,
+    };
+
+    Event() = default;
+    Event(Type type, platform::LocalCountryFile const & file)
+      : m_type(type), m_file(file)
+    {
+    }
+    Event(Type type, platform::LocalCountryFile const & newFile,
+          platform::LocalCountryFile const & oldFile)
+      : m_type(type), m_file(newFile), m_oldFile(oldFile)
+    {
+    }
+
+    inline bool operator==(Event const & rhs) const
+    {
+      return m_type == rhs.m_type && m_file == rhs.m_file && m_oldFile == rhs.m_oldFile;
+    }
+
+    inline bool operator!=(Event const & rhs) const { return !(*this == rhs); }
+
+    Type m_type;
+    platform::LocalCountryFile m_file;
+    platform::LocalCountryFile m_oldFile;
+  };
+
+  class EventList
+  {
+  public:
+    EventList() = default;
+
+    inline void Add(Event const & event) { m_events.push_back(event); }
+
+    inline void Append(EventList const & events)
+    {
+      m_events.insert(m_events.end(), events.m_events.begin(), events.m_events.end());
+    }
+
+    vector<Event> const & Get() const { return m_events; }
+
+  private:
+    vector<Event> m_events;
+
+    DISALLOW_COPY_AND_MOVE(EventList);
+  };
+
   enum class RegResult
   {
     Success,
@@ -162,6 +221,26 @@ public:
     BadFile
   };
 
+  // An Observer interface to MwmSet. Note that these functions can
+  // be called from *ANY* thread because most signals are sent when
+  // some thread releases its MwmHandle, so overrides must be as fast
+  // as possible and non-blocking when it's possible.
+  class Observer
+  {
+  public:
+    virtual ~Observer() = default;
+
+    // Called when a map is registered for a first time and can be
+    // used.
+    virtual void OnMapRegistered(platform::LocalCountryFile const & localFile) {}
+
+    // Called when a map is updated to a newer version.
+    virtual void OnMapUpdated(platform::LocalCountryFile const & newFile, platform::LocalCountryFile const & oldFile) {}
+
+    // Called when a map is deregistered and can not be used.
+    virtual void OnMapDeregistered(platform::LocalCountryFile const & localFile) {}
+  };
+
   /// Registers a new map.
   ///
   /// \return An active mwm handle when an mwm file with this version
@@ -170,7 +249,7 @@ public:
   /// are older than the localFile (in this case mwm handle will point
   /// to just-registered file).
 protected:
-  pair<MwmId, RegResult> RegisterImpl(platform::LocalCountryFile const & localFile);
+  pair<MwmId, RegResult> RegisterImpl(platform::LocalCountryFile const & localFile, EventList & events);
 
 public:
   pair<MwmId, RegResult> Register(platform::LocalCountryFile const & localFile);
@@ -185,14 +264,18 @@ protected:
   /// \return True if the map was successfully deregistered. If map is locked
   ///         now, returns false.
   //@{
-  bool DeregisterImpl(MwmId const & id);
-  bool DeregisterImpl(platform::CountryFile const & countryFile);
+  bool DeregisterImpl(MwmId const & id, EventList & events);
+  bool DeregisterImpl(platform::CountryFile const & countryFile, EventList & events);
   //@}
 
 public:
   bool Deregister(platform::CountryFile const & countryFile);
   //@}
 
+  inline bool AddObserver(Observer & observer) { return m_observers.Add(observer); }
+
+  inline bool RemoveObserver(Observer const & observer) { return m_observers.Remove(observer); }
+
   /// Returns true when country is registered and can be used.
   bool IsLoaded(platform::CountryFile const & countryFile) const;
 
@@ -226,13 +309,37 @@ protected:
 private:
   typedef deque<pair<MwmId, unique_ptr<MwmValueBase>>> CacheType;
 
+  // This is the only valid way to take |m_lock| and use *Impl()
+  // functions. The reason is that event processing requires
+  // triggering of observers, but it's generally unsafe to call
+  // user-provided functions while |m_lock| is taken, as it may lead
+  // to deadlocks or locks without any time guarantees. Instead, a
+  // list of Events is used to collect all events and then send them
+  // to observers without |m_lock| protection.
+  template <typename TFn>
+  void WithEventLog(TFn && fn)
+  {
+    EventList events;
+    {
+      lock_guard<mutex> lock(m_lock);
+      fn(events);
+    }
+    ProcessEventList(events);
+  }
+
+  // Sets |status| in |info|, adds corresponding event to |event|.
+  void SetStatus(MwmInfo & info, MwmInfo::Status status, EventList & events);
+
+  // Triggers observers on each event in |events|.
+  void ProcessEventList(EventList & events);
+
   /// @precondition This function is always called under mutex m_lock.
-  MwmHandle GetMwmHandleByIdImpl(MwmId const & id);
+  MwmHandle GetMwmHandleByIdImpl(MwmId const & id, EventList & events);
 
   unique_ptr<MwmValueBase> LockValue(MwmId const & id);
-  unique_ptr<MwmValueBase> LockValueImpl(MwmId const & id);
+  unique_ptr<MwmValueBase> LockValueImpl(MwmId const & id, EventList & events);
   void UnlockValue(MwmId const & id, unique_ptr<MwmValueBase> && p);
-  void UnlockValueImpl(MwmId const & id, unique_ptr<MwmValueBase> && p);
+  void UnlockValueImpl(MwmId const & id, unique_ptr<MwmValueBase> && p, EventList & events);
 
   /// Do the cleaning for [beg, end) without acquiring the mutex.
   /// @precondition This function is always called under mutex m_lock.
@@ -249,19 +356,14 @@ protected:
   /// @precondition This function is always called under mutex m_lock.
   MwmId GetMwmIdByCountryFileImpl(platform::CountryFile const & countryFile) const;
 
-  /// @precondition This function is always called under mutex m_lock.
-  WARN_UNUSED_RESULT inline MwmHandle GetLock(MwmId const & id)
-  {
-    return MwmHandle(*this, id, LockValueImpl(id));
-  }
-
-  // This method is called under m_lock when mwm is removed from a
-  // registry.
-  virtual void OnMwmDeregistered(platform::LocalCountryFile const & localFile) {}
-
   map<string, vector<shared_ptr<MwmInfo>>> m_info;
 
   mutable mutex m_lock;
+
+private:
+  my::ObserverList<Observer> m_observers;
 };
 
 string DebugPrint(MwmSet::RegResult result);
+string DebugPrint(MwmSet::Event::Type type);
+string DebugPrint(MwmSet::Event const & event);

map/feature_vec_model.cpp

@@ -83,6 +83,13 @@ void FeaturesFetcher::ClearCaches()
   m_multiIndex.ClearCache();
 }
 
+void FeaturesFetcher::OnMapUpdated(platform::LocalCountryFile const & newFile,
+                      platform::LocalCountryFile const & oldFile)
+{
+  if (m_onMapDeregistered)
+    m_onMapDeregistered(oldFile);
+}
+
 void FeaturesFetcher::OnMapDeregistered(platform::LocalCountryFile const & localFile)
 {
   if (m_onMapDeregistered)

map/feature_vec_model.hpp

@@ -2,6 +2,7 @@
 
 #include "indexer/data_header.hpp"
 #include "indexer/index.hpp"
+#include "indexer/mwm_set.hpp"
 
 #include "geometry/rect2d.hpp"
 #include "geometry/point2d.hpp"
@@ -15,7 +16,7 @@ namespace model
 {
 //#define USE_BUFFER_READER
 
-class FeaturesFetcher : public Index::Observer
+class FeaturesFetcher : public MwmSet::Observer
   {
   public:
 #ifdef USE_BUFFER_READER
@@ -61,7 +62,9 @@ class FeaturesFetcher : public Index::Observer
       return m_multiIndex.IsLoaded(platform::CountryFile(countryFileName));
     }
 
-    // Index::Observer overrides:
+    // MwmSet::Observer overrides:
+    void OnMapUpdated(platform::LocalCountryFile const & newFile,
+                      platform::LocalCountryFile const & oldFile) override;
     void OnMapDeregistered(platform::LocalCountryFile const & localFile) override;
 
     //bool IsLoaded(m2::PointD const & pt) const;