[core] harden OfflineDatabase

mapbox · Aug 6, 2018 · 03a4dc2 · 03a4dc2
1 parent 99a365d
commit 03a4dc2
Show file tree

Hide file tree

Showing 16 changed files with 1,157 additions and 353 deletions.
diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake
@@ -95,6 +95,8 @@ set(MBGL_TEST_FILES
     test/src/mbgl/test/fixture_log_observer.hpp
     test/src/mbgl/test/getrss.cpp
     test/src/mbgl/test/getrss.hpp
+    test/src/mbgl/test/sqlite3_test_fs.cpp
+    test/src/mbgl/test/sqlite3_test_fs.hpp
     test/src/mbgl/test/stub_file_source.cpp
     test/src/mbgl/test/stub_file_source.hpp
     test/src/mbgl/test/stub_geometry_tile_feature.hpp

diff --git a/include/mbgl/storage/offline.hpp b/include/mbgl/storage/offline.hpp
@@ -187,11 +187,11 @@ class OfflineRegion {
 public:
     // Move-only; not publicly constructible.
     OfflineRegion(OfflineRegion&&);
-    OfflineRegion& operator=(OfflineRegion&&);
     ~OfflineRegion();
 
     OfflineRegion() = delete;
     OfflineRegion(const OfflineRegion&) = delete;
+    OfflineRegion& operator=(OfflineRegion&&) = delete;
     OfflineRegion& operator=(const OfflineRegion&) = delete;
 
     int64_t getID() const;

diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp
@@ -74,9 +74,9 @@ class DefaultFileSource::Impl {
     }
 
     void getRegionStatus(int64_t regionID, std::function<void (std::exception_ptr, optional<OfflineRegionStatus>)> callback) {
-        try {
-            callback({}, getDownload(regionID).getStatus());
-        } catch (...) {
+        if (auto download = getDownload(regionID)) {
+            callback({}, download->getStatus());
+        } else {
             callback(std::current_exception(), {});
         }
     }
@@ -92,11 +92,15 @@ class DefaultFileSource::Impl {
     }
 
     void setRegionObserver(int64_t regionID, std::unique_ptr<OfflineRegionObserver> observer) {
-        getDownload(regionID).setObserver(std::move(observer));
+        if (auto download = getDownload(regionID)) {
+            download->setObserver(std::move(observer));
+        }
     }
 
     void setRegionDownloadState(int64_t regionID, OfflineRegionDownloadState state) {
-        getDownload(regionID).setState(state);
+        if (auto download = getDownload(regionID)) {
+            download->setState(state);
+        }
     }
 
     void request(AsyncRequest* req, Resource resource, ActorRef<FileSourceRequest> ref) {
@@ -181,13 +185,19 @@ class DefaultFileSource::Impl {
     }
 
 private:
-    OfflineDownload& getDownload(int64_t regionID) {
+    OfflineDownload* getDownload(int64_t regionID) {
         auto it = downloads.find(regionID);
         if (it != downloads.end()) {
-            return *it->second;
+            return it->second.get();
+        }
+        auto definition = offlineDatabase->getRegionDefinition(regionID);
+        if (!definition) {
+            return nullptr;
         }
-        return *downloads.emplace(regionID,
-            std::make_unique<OfflineDownload>(regionID, offlineDatabase->getRegionDefinition(regionID), *offlineDatabase, onlineFileSource)).first->second;
+
+        auto download = std::make_unique<OfflineDownload>(regionID, std::move(*definition),
+                                                          *offlineDatabase, onlineFileSource);
+        return downloads.emplace(regionID, std::move(download)).first->second.get();
     }
 
     // shared so that destruction is done on the creating thread