Make requests for expired resources lower priority than requests for new resources

include/mbgl/storage/default_file_source.hpp

@@ -245,6 +245,7 @@ class DefaultFileSource : public FileSource {
 
     // For testing only.
     void setOnlineStatus(bool);
+    void setMaximumConcurrentRequests(uint32_t);
 
     class Impl;
 

platform/android/CHANGELOG.md

@@ -4,6 +4,7 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
 
 ## master
 - Added support for [image expression](https://docs.mapbox.com/mapbox-gl-js/style-spec/#expressions-types-image) in core library. Runtime APIs for image expression will be implemented separately. [#15877](https://github.com/mapbox/mapbox-gl-native/pull/15877)
+- Make network requests for expired resources lower priority than requests for new resources. ([#15950](https://github.com/mapbox/mapbox-gl-native/pull/15950))
 
 ### Bug fixes
  - Fixed the rendering bug caused by redundant pending requests for already requested images [#15864](https://github.com/mapbox/mapbox-gl-native/pull/15864)

platform/default/src/mbgl/storage/default_file_source.cpp

@@ -145,6 +145,8 @@ class DefaultFileSource::Impl {
 
                     if (offlineResponse->isUsable()) {
                         callback(*offlineResponse);
+                        // Set the priority of existing resource to low if it's expired but usable.
+                        resource.setPriority(Resource::Priority::Low);
                     }
                 }
             }
@@ -180,6 +182,10 @@ class DefaultFileSource::Impl {
         onlineFileSource.setOnlineStatus(status);
     }
 
+    void setMaximumConcurrentRequests(uint32_t maximumConcurrentRequests_) {
+        onlineFileSource.setMaximumConcurrentRequests(maximumConcurrentRequests_);
+    }
+
     void put(const Resource& resource, const Response& response) {
         offlineDatabase->put(resource, response);
     }
@@ -375,4 +381,8 @@ void DefaultFileSource::setOnlineStatus(const bool status) {
     impl->actor().invoke(&Impl::setOnlineStatus, status);
 }
 
+void DefaultFileSource::setMaximumConcurrentRequests(uint32_t maximumConcurrentRequests_) {
+    impl->actor().invoke(&Impl::setMaximumConcurrentRequests, maximumConcurrentRequests_);
+}
+
 } // namespace mbgl

platform/default/src/mbgl/storage/online_file_source.cpp

@@ -158,7 +158,9 @@ class OnlineFileSource::Impl {
 
     void setOnlineStatus(const bool status) {
         online = status;
-        networkIsReachableAgain();
+        if (online) {
+            networkIsReachableAgain();
+        }
     }
 
     uint32_t getMaximumConcurrentRequests() const {

platform/ios/CHANGELOG.md

@@ -4,6 +4,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 
 ## master
 * Added support for [image expression](https://docs.mapbox.com/mapbox-gl-js/style-spec/#expressions-types-image) in core library. Runtime APIs for image expression will be implemented separately. ([#15877](https://github.com/mapbox/mapbox-gl-native/pull/15877))
+* Make network requests for expired resources lower priority than requests for new resources. ([#15950](https://github.com/mapbox/mapbox-gl-native/pull/15950))
 
 ### Other changes
 * Convert GeoJSON features to tiles for the loaded source description in a background thread and thus unblock the UI thread ([#15885](https://github.com/mapbox/mapbox-gl-native/pull/15885))

platform/macos/CHANGELOG.md

@@ -20,6 +20,7 @@
 * Added an `-[MGLMapSnapshotter startWithOverlayHandler:completionHandler:]` method to provide the snapshot's current `CGContext` in order to perform custom drawing on `MGLMapSnapShot` objects. ([#15530](https://github.com/mapbox/mapbox-gl-native/pull/15530))
 * Fixed an issue that `maxzoom` in style `Sources` option was ignored when URL resource is provided. It may cause problems such as extra tiles downloading at higher zoom level than `maxzoom`, or problems that wrong setting of `overscaledZ` in `OverscaledTileID` that will be passed to `SymbolLayout`, leading wrong rendering appearance. ([#15581](https://github.com/mapbox/mapbox-gl-native/pull/15581))
 * Fixed a crash when `-[MGLOfflinePack invalidate]` is called on different threads. ([#15582](https://github.com/mapbox/mapbox-gl-native/pull/15582))
+* Make network requests for expired resources lower priority than requests for new resources. ([#15950](https://github.com/mapbox/mapbox-gl-native/pull/15950))
 
 ### Styles and rendering
 

test/storage/default_file_source.test.cpp

@@ -1,7 +1,8 @@
 #include <mbgl/actor/actor.hpp>
-#include <mbgl/test/util.hpp>
 #include <mbgl/storage/default_file_source.hpp>
+#include <mbgl/storage/network_status.hpp>
 #include <mbgl/storage/resource_transform.hpp>
+#include <mbgl/test/util.hpp>
 #include <mbgl/util/run_loop.hpp>
 
 using namespace mbgl;
@@ -661,3 +662,77 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(RespondToStaleMustRevalidate)) {
 
     loop.run();
 }
+
+// Test that requests for expired resources have lower priority than requests for new resources
+TEST(DefaultFileSource, TEST_REQUIRES_SERVER(CachedResourceLowPriority)) {
+    util::RunLoop loop;
+    DefaultFileSource fs(":memory:", ".");
+
+    Response response;
+    std::size_t online_response_counter = 0;
+
+    using namespace std::chrono_literals;
+    response.expires = util::now() - 1h;
+
+    // Put existing values into the cache.
+    Resource resource1{Resource::Unknown, "http://127.0.0.1:3000/load/3", {}, Resource::LoadingMethod::All};
+    response.data = std::make_shared<std::string>("Cached Request 3");
+    fs.put(resource1, response);
+
+    Resource resource2{Resource::Unknown, "http://127.0.0.1:3000/load/4", {}, Resource::LoadingMethod::All};
+    response.data = std::make_shared<std::string>("Cached Request 4");
+    fs.put(resource2, response);
+
+    fs.setMaximumConcurrentRequests(1);
+    NetworkStatus::Set(NetworkStatus::Status::Offline);
+
+    // Ensure that the online requests for new resources are processed first.
+    Resource nonCached1{Resource::Unknown, "http://127.0.0.1:3000/load/1", {}, Resource::LoadingMethod::All};
+    std::unique_ptr<AsyncRequest> req1 = fs.request(nonCached1, [&](Response res) {
+        online_response_counter++;
+        req1.reset();
+        EXPECT_EQ(online_response_counter, 1); // make sure this is responded first
+        EXPECT_EQ("Request 1", *res.data);
+    });
+
+    Resource nonCached2{Resource::Unknown, "http://127.0.0.1:3000/load/2", {}, Resource::LoadingMethod::All};
+    std::unique_ptr<AsyncRequest> req2 = fs.request(nonCached2, [&](Response res) {
+        online_response_counter++;
+        req2.reset();
+        EXPECT_EQ(online_response_counter, 2); // make sure this is responded second
+        EXPECT_EQ("Request 2", *res.data);
+    });
+
+    bool req3CachedResponseReceived = false;
+    std::unique_ptr<AsyncRequest> req3 = fs.request(resource1, [&](Response res) {
+        // Offline callback is received first
+        if (!req3CachedResponseReceived) {
+            EXPECT_EQ("Cached Request 3", *res.data);
+            req3CachedResponseReceived = true;
+        } else {
+            online_response_counter++;
+            req3.reset();
+            EXPECT_EQ(online_response_counter, 3);
+            EXPECT_EQ("Request 3", *res.data);
+        }
+    });
+
+    bool req4CachedResponseReceived = false;
+    std::unique_ptr<AsyncRequest> req4 = fs.request(resource2, [&](Response res) {
+        // Offline callback is received first
+        if (!req4CachedResponseReceived) {
+            EXPECT_EQ("Cached Request 4", *res.data);
+            req4CachedResponseReceived = true;
+        } else {
+            online_response_counter++;
+            req4.reset();
+            EXPECT_EQ(online_response_counter, 4);
+            EXPECT_EQ("Request 4", *res.data);
+            loop.stop();
+        }
+    });
+
+    NetworkStatus::Set(NetworkStatus::Status::Online);
+
+    loop.run();
+}