[core] Don't allow style mutations to be overwritten by revalidation

* Once we get a fresh style, stop revalidating.
* If the style is mutated, stop revalidating and preserve the existing mutations.

2 files changed, 124 insertions, 0 deletions

diff --git a/test/map/map.cpp b/test/map/map.cpp
index 1f6e59f3ea..79c1993cb4 100644
--- a/test/map/map.cpp
+++ b/test/map/map.cpp
@@ -1,5 +1,6 @@
 #include <mbgl/test/util.hpp>
 #include <mbgl/test/stub_file_source.hpp>
+#include <mbgl/test/fake_file_source.hpp>
 #include <mbgl/test/fixture_log_observer.hpp>
 
 #include <mbgl/map/map.hpp>
@@ -80,6 +81,69 @@ TEST(Map, DoubleStyleLoad) {
     map.setStyleJSON("");
 }
 
+TEST(Map, StyleFresh) {
+    // The map should not revalidate fresh styles.
+
+    MapTest test;
+    FakeFileSource fileSource;
+
+    Map map(test.view, fileSource, MapMode::Still);
+    map.setStyleURL("mapbox://styles/test");
+    EXPECT_EQ(1u, fileSource.requests.size());
+
+    Response response;
+    response.data = std::make_shared<std::string>(util::read_file("test/fixtures/api/empty.json"));
+    response.expires = Timestamp::max();
+
+    fileSource.respond(Resource::Style, response);
+    EXPECT_EQ(0u, fileSource.requests.size());
+}
+
+TEST(Map, StyleExpired) {
+    // The map should allow expired styles to be revalidated, so long as no mutations are made.
+
+    using namespace std::chrono_literals;
+
+    MapTest test;
+    FakeFileSource fileSource;
+
+    Map map(test.view, fileSource, MapMode::Still);
+    map.setStyleURL("mapbox://styles/test");
+    EXPECT_EQ(1u, fileSource.requests.size());
+
+    Response response;
+    response.data = std::make_shared<std::string>(util::read_file("test/fixtures/api/empty.json"));
+    response.expires = util::now() - 1h;
+
+    fileSource.respond(Resource::Style, response);
+    EXPECT_EQ(1u, fileSource.requests.size());
+
+    map.addLayer(std::make_unique<style::BackgroundLayer>("bg"));
+    EXPECT_EQ(1u, fileSource.requests.size());
+
+    fileSource.respond(Resource::Style, response);
+    EXPECT_EQ(0u, fileSource.requests.size());
+    EXPECT_NE(nullptr, map.getLayer("bg"));
+}
+
+TEST(Map, StyleEarlyMutation) {
+    // An early mutation should not prevent the initial style load.
+
+    MapTest test;
+    FakeFileSource fileSource;
+
+    Map map(test.view, fileSource, MapMode::Still);
+    map.setStyleURL("mapbox://styles/test");
+    map.addLayer(std::make_unique<style::BackgroundLayer>("bg"));
+
+    Response response;
+    response.data = std::make_shared<std::string>(util::read_file("test/fixtures/api/water.json"));
+    fileSource.respond(Resource::Style, response);
+
+    EXPECT_EQ(0u, fileSource.requests.size());
+    EXPECT_NE(nullptr, map.getLayer("water"));
+}
+
 TEST(Map, AddLayer) {
     MapTest test;
 
diff --git a/test/src/mbgl/test/fake_file_source.hpp b/test/src/mbgl/test/fake_file_source.hpp
new file mode 100644
index 0000000000..7ebb4cf0cb
--- /dev/null
+++ b/test/src/mbgl/test/fake_file_source.hpp
@@ -0,0 +1,60 @@
+#pragma once
+
+#include <mbgl/storage/file_source.hpp>
+
+#include <list>
+
+namespace mbgl {
+
+/*
+   FakeFileSource is similar to StubFileSource, but it follows a post hoc, "push" model rather than
+   a pre hoc, "pull" model. You pass it to code that makes requests, it records what requests are
+   made, then you can examine them, make assertions about them, and respond as desired.
+
+   This is particularly useful if you want to simulate multiple responses, e.g. as part of a resource
+   revalidation flow. StubFileSource allows only a single response.
+*/
+class FakeFileSource : public FileSource {
+public:
+    class FakeFileRequest : public AsyncRequest {
+    public:
+        Resource resource;
+        Callback callback;
+
+        std::list<FakeFileRequest*>& list;
+        std::list<FakeFileRequest*>::iterator link;
+
+        FakeFileRequest(Resource resource_, Callback callback_, std::list<FakeFileRequest*>& list_)
+            : resource(std::move(resource_)),
+              callback(std::move(callback_)),
+              list(list_),
+              link((list.push_back(this), std::prev(list.end()))) {
+        }
+
+        ~FakeFileRequest() override {
+            list.erase(link);
+        }
+    };
+
+    std::unique_ptr<AsyncRequest> request(const Resource& resource, Callback callback) override {
+        return std::make_unique<FakeFileRequest>(resource, callback, requests);
+    }
+
+    bool respond(Resource::Kind kind, const Response& response) {
+        auto it = std::find_if(requests.begin(), requests.end(), [&] (FakeFileRequest* request) {
+            return request->resource.kind == kind;
+        });
+
+        if (it != requests.end()) {
+            // Copy the callback, in case calling it deallocates the AsyncRequest.
+            Callback callback_ = (*it)->callback;
+            callback_(response);
+        }
+
+        return it != requests.end();
+    }
+
+    std::list<FakeFileRequest*> requests;
+};
+
+} // namespace mbgl