Cancel pending style request when loading style JSON (#3989)

Signed-off-by: Bart Louwers <[email protected]>

Author: louwers

src/mbgl/style/style_impl.cpp

@@ -39,10 +39,15 @@ Style::Impl::Impl(std::shared_ptr<FileSource> fileSource_, float pixelRatio, con
 
 Style::Impl::~Impl() = default;
 
+void Style::Impl::cancelPendingRequest() noexcept {
+    styleRequest.reset();
+}
+
 void Style::Impl::loadJSON(const std::string& json_) {
     lastError = nullptr;
     observer->onStyleLoading();
 
+    cancelPendingRequest();
     url.clear();
     parse(json_);
 }

src/mbgl/style/style_impl.hpp

@@ -45,6 +45,14 @@ class Style::Impl : public SpriteLoaderObserver,
     void loadJSON(const std::string&);
     void loadURL(const std::string&);
 
+    /**
+     * @brief Cancels any pending style request.
+     *
+     * This will cancel any in-progress style URL load. Has no effect if no
+     * style load is in progress.
+     */
+    void cancelPendingRequest() noexcept;
+
     std::string getJSON() const;
     std::string getURL() const;
 

test/src/mbgl/test/stub_file_source.cpp

@@ -63,8 +63,7 @@ StubFileSource::~StubFileSource() = default;
 std::unique_ptr<AsyncRequest> StubFileSource::request(const Resource& resource, Callback callback) {
     auto req = std::make_unique<StubFileRequest>(*this);
     if (type == ResponseType::Synchronous) {
-        std::optional<Response> res = response(resource);
-        if (res) {
+        if (const std::optional<Response> res = response(resource)) {
             callback(*res);
         }
     } else {
@@ -79,6 +78,18 @@ void StubFileSource::remove(AsyncRequest* req) {
         pending.erase(it);
     }
 }
+void StubFileSource::respondToAll() {
+    if (type != ResponseType::Manual) {
+        throw std::runtime_error("this method can only be called with ResponseType::Manual");
+    }
+    for (const auto& [key, value] : pending) {
+        auto& [resource, responseFunction, callback] = value;
+        if (const std::optional<Response> res = responseFunction(resource)) {
+            callback(*res);
+        }
+        pending.erase(key);
+    }
+}
 
 void StubFileSource::setProperty(const std::string& key, const mapbox::base::Value& value) {
     properties[key] = value;

test/src/mbgl/test/stub_file_source.hpp

@@ -16,7 +16,8 @@ class StubFileSource : public FileSource {
 public:
     enum class ResponseType {
         Asynchronous = 0,
-        Synchronous
+        Synchronous,
+        Manual
     };
 
     StubFileSource(const ResourceOptions&, const ClientOptions&, ResponseType = ResponseType::Asynchronous);
@@ -26,6 +27,8 @@ class StubFileSource : public FileSource {
     std::unique_ptr<AsyncRequest> request(const Resource&, Callback) override;
     bool canRequest(const Resource&) const override { return true; }
     void remove(AsyncRequest*);
+    void respondToAll();
+
     void setProperty(const std::string&, const mapbox::base::Value&) override;
     mapbox::base::Value getProperty(const std::string&) const override;
 

test/style/style.test.cpp

@@ -1,3 +1,4 @@
+#include <chrono>
 #include <mbgl/test/util.hpp>
 #include <mbgl/test/stub_file_source.hpp>
 #include <mbgl/test/fixture_log_observer.hpp>
@@ -9,6 +10,7 @@
 #include <mbgl/style/layers/line_layer.hpp>
 #include <mbgl/util/io.hpp>
 #include <mbgl/util/run_loop.hpp>
+#include <mbgl/util/client_options.hpp>
 
 #include <memory>
 
@@ -110,6 +112,43 @@ TEST(Style, RemoveSourceInUse) {
     EXPECT_EQ(log.count(logMessage), 1u);
 }
 
+TEST(Style, LoadJSONCancelsPendingLoadURL) {
+    util::RunLoop loop;
+
+    auto fileSource = std::make_shared<::StubFileSource>(
+        ResourceOptions::Default(), ClientOptions(), StubFileSource::ResponseType::Manual);
+    Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}};
+
+    // Start loading a URL (this will be pending)
+    auto url = "http://some-url";
+    fileSource->styleResponse = [](const Resource&) {
+        Response result;
+        result.data = std::make_shared<std::string>(util::read_file("test/fixtures/resources/style_vector.json"));
+        return result;
+    };
+    style.loadURL(url);
+
+    // Before the URL request completes, load JSON directly
+    const std::string json = R"STYLE({
+        "version": 8,
+        "name": "Test Style",
+        "sources": {},
+        "layers": []
+    })STYLE";
+    style.loadJSON(json);
+
+    // The style should now be loaded with our JSON content
+    ASSERT_EQ("Test Style", style.getName());
+    ASSERT_EQ("", style.getURL());
+    ASSERT_TRUE(style.getJSON().find("Test Style") != std::string::npos);
+
+    fileSource->respondToAll();
+
+    // The style should still show our JSON content, not the URL content
+    ASSERT_EQ("Test Style", style.getName());
+    ASSERT_NE("Streets", style.getName());
+}
+
 TEST(Style, SourceImplsOrder) {
     util::RunLoop loop;
     auto fileSource = std::make_shared<StubFileSource>();