From f4cd73bf9c032783ef9a92a3c661a91629d21818 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= <mail@kkaefer.com>
Date: Wed, 21 Feb 2018 12:35:44 +0100
Subject: [core] continue loading style even if we mutate it

When we load a stale style from cache, and the user immediately starts mutating it, we should continue loading the style so that we'll get a fresh copy of the data into our cache and avoid perpetually showing the stale style.
---
 src/mbgl/style/style_impl.cpp |  5 -----
 test/map/map.test.cpp         | 21 ++++++++++++++++-----
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/mbgl/style/style_impl.cpp b/src/mbgl/style/style_impl.cpp
index d330b3120a..0c7f924917 100644
--- a/src/mbgl/style/style_impl.cpp
+++ b/src/mbgl/style/style_impl.cpp
@@ -55,11 +55,6 @@ void Style::Impl::loadURL(const std::string& url_) {
     url = url_;
 
     styleRequest = fileSource.request(Resource::style(url), [this](Response res) {
-        // Once we get a fresh style, or the style is mutated, stop revalidating.
-        if (res.isFresh() || mutated) {
-            styleRequest.reset();
-        }
-
         // Don't allow a loaded, mutated style to be overwritten with a new version.
         if (mutated && loaded) {
             return;
diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp
index 9b34ea89b0..30c076ad89 100644
--- a/test/map/map.test.cpp
+++ b/test/map/map.test.cpp
@@ -252,7 +252,7 @@ TEST(Map, DoubleStyleLoad) {
 }
 
 TEST(Map, StyleFresh) {
-    // The map should not revalidate fresh styles.
+    // The map should continue to revalidate fresh styles.
 
     MapTest<FakeFileSource> test;
 
@@ -264,11 +264,11 @@ TEST(Map, StyleFresh) {
     response.expires = Timestamp::max();
 
     test.fileSource.respond(Resource::Style, response);
-    EXPECT_EQ(0u, test.fileSource.requests.size());
+    EXPECT_EQ(1u, test.fileSource.requests.size());
 }
 
 TEST(Map, StyleExpired) {
-    // The map should allow expired styles to be revalidated, so long as no mutations are made.
+    // The map should allow expired styles to be revalidated until we get a fresh style.
 
     using namespace std::chrono_literals;
 
@@ -284,11 +284,22 @@ TEST(Map, StyleExpired) {
     test.fileSource.respond(Resource::Style, response);
     EXPECT_EQ(1u, test.fileSource.requests.size());
 
+    // Mutate layer. From now on, sending a response to the style won't overwrite it anymore, but
+    // we should continue to wait for a fresh response.
     test.map.getStyle().addLayer(std::make_unique<style::BackgroundLayer>("bg"));
     EXPECT_EQ(1u, test.fileSource.requests.size());
 
+    // Send another expired response, and confirm that we didn't overwrite the style, but continue
+    // to wait for a fresh response.
+    test.fileSource.respond(Resource::Style, response);
+    EXPECT_EQ(1u, test.fileSource.requests.size());
+    EXPECT_NE(nullptr, test.map.getStyle().getLayer("bg"));
+
+    // Send a fresh response, and confirm that we didn't overwrite the style, but continue to wait
+    // for a fresh response.
+    response.expires = util::now() + 1h;
     test.fileSource.respond(Resource::Style, response);
-    EXPECT_EQ(0u, test.fileSource.requests.size());
+    EXPECT_EQ(1u, test.fileSource.requests.size());
     EXPECT_NE(nullptr, test.map.getStyle().getLayer("bg"));
 }
 
@@ -352,7 +363,7 @@ TEST(Map, StyleEarlyMutation) {
     response.data = std::make_shared<std::string>(util::read_file("test/fixtures/api/water.json"));
     test.fileSource.respond(Resource::Style, response);
 
-    EXPECT_EQ(0u, test.fileSource.requests.size());
+    EXPECT_EQ(1u, test.fileSource.requests.size());
     EXPECT_NE(nullptr, test.map.getStyle().getLayer("water"));
 }
 
-- 
cgit v1.2.1