diff --git a/indexer/index.cpp b/indexer/index.cpp index 3110b8a08a..0f0505d394 100644 --- a/indexer/index.cpp +++ b/indexer/index.cpp @@ -75,23 +75,11 @@ unique_ptr Index::CreateValue(MwmInfo & info) const pair 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 ////////////////////////////////////////////////////////////////////////////////// diff --git a/indexer/index.hpp b/indexer/index.hpp index 8694055151..39a38c15d5 100644 --- a/indexer/index.hpp +++ b/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 CreateInfo(platform::LocalCountryFile const & localFile) const override; unique_ptr 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 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 class ReadMWMFunctor @@ -370,6 +346,4 @@ private: editor.ForEachFeatureInMwmRectAndScale(worldID[1], f, rect, scale); } } - - my::ObserverList m_observers; }; diff --git a/indexer/indexer_tests/index_test.cpp b/indexer/indexer_tests/index_test.cpp index 1b30199d23..292857c967 100644 --- a/indexer/indexer_tests/index_test.cpp +++ b/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 + void AddEvent(vector & events, Args... args) + { + events.emplace_back(forward(args)...); + } -private: - size_t m_numRegisteredCalls; - size_t m_numDeregisteredCalls; - - vector m_expectedRegisteredMaps; - vector m_expectedDeregisteredMaps; + Index m_index; + vector m_expected; + vector 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); } diff --git a/indexer/mwm_set.cpp b/indexer/mwm_set.cpp index 97592cabdd..83ca9e107b 100644 --- a/indexer/mwm_set.cpp +++ b/indexer/mwm_set.cpp @@ -85,38 +85,57 @@ MwmSet::MwmId MwmSet::GetMwmIdByCountryFileImpl(CountryFile const & countryFile) pair MwmSet::Register(LocalCountryFile const & localFile) { - lock_guard lock(m_lock); + pair 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 info = id.GetInfo(); - shared_ptr 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::RegisterImpl(LocalCountryFile const & localFile) +pair MwmSet::RegisterImpl(LocalCountryFile const & localFile, + EventList & events) { // This function can throw an exception for a bad mwm file. shared_ptr info(CreateInfo(localFile)); @@ -124,13 +143,13 @@ pair 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 const & info = id.GetInfo(); if (info->m_numRefs == 0) { - info->SetStatus(MwmInfo::STATUS_DEREGISTERED); + SetStatus(*info, MwmInfo::STATUS_DEREGISTERED, events); vector> & 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 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> & info) const } } -unique_ptr MwmSet::LockValue(MwmId const & id) +void MwmSet::SetStatus(MwmInfo & info, MwmInfo::Status status, EventList & events) { - lock_guard 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::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::LockValue(MwmId const & id) +{ + unique_ptr result; + WithEventLog([&](EventList & events) + { + result = LockValueImpl(id, events); + }); + return result; +} + +unique_ptr MwmSet::LockValueImpl(MwmId const & id, EventList & events) { if (!id.IsAlive()) return nullptr; @@ -224,18 +287,20 @@ unique_ptr 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 && p) { - lock_guard lock(m_lock); - UnlockValueImpl(id, move(p)); + WithEventLog([&](EventList & events) + { + UnlockValueImpl(id, move(p), events); + }); } -void MwmSet::UnlockValueImpl(MwmId const & id, unique_ptr && p) +void MwmSet::UnlockValueImpl(MwmId const & id, unique_ptr && p, EventList & events) { ASSERT(id.IsAlive() && p, (id)); if (!id.IsAlive() || !p) @@ -245,7 +310,7 @@ void MwmSet::UnlockValueImpl(MwmId const & id, unique_ptr && 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 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 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 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(); +} diff --git a/indexer/mwm_set.hpp b/indexer/mwm_set.hpp index 5ae2c17e6f..32f19cb6cf 100644 --- a/indexer/mwm_set.hpp +++ b/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 m_status; ///< Current country status. @@ -92,6 +98,7 @@ public: { return (m_info && m_info->GetStatus() != MwmInfo::STATUS_DEREGISTERED); } + shared_ptr & GetInfo() { return m_info; } shared_ptr 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 const & Get() const { return m_events; } + + private: + vector 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 RegisterImpl(platform::LocalCountryFile const & localFile); + pair RegisterImpl(platform::LocalCountryFile const & localFile, EventList & events); public: pair 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>> 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 + void WithEventLog(TFn && fn) + { + EventList events; + { + lock_guard 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 LockValue(MwmId const & id); - unique_ptr LockValueImpl(MwmId const & id); + unique_ptr LockValueImpl(MwmId const & id, EventList & events); void UnlockValue(MwmId const & id, unique_ptr && p); - void UnlockValueImpl(MwmId const & id, unique_ptr && p); + void UnlockValueImpl(MwmId const & id, unique_ptr && 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>> m_info; mutable mutex m_lock; + +private: + my::ObserverList m_observers; }; string DebugPrint(MwmSet::RegResult result); +string DebugPrint(MwmSet::Event::Type type); +string DebugPrint(MwmSet::Event const & event); diff --git a/map/feature_vec_model.cpp b/map/feature_vec_model.cpp index a54308bdbe..164e5249de 100644 --- a/map/feature_vec_model.cpp +++ b/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) diff --git a/map/feature_vec_model.hpp b/map/feature_vec_model.hpp index 75d7dca4fc..c4e5970467 100644 --- a/map/feature_vec_model.hpp +++ b/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;