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

Co-authored-by: Tim Sylvester <[email protected]>

Author: louwers

include/mbgl/style/style.hpp

@@ -71,6 +71,8 @@ class Style {
     void addLayer(std::unique_ptr<Layer>, const std::optional<std::string>& beforeLayerID = std::nullopt);
     std::unique_ptr<Layer> removeLayer(const std::string& layerID);
 
+    void cancelPendingRequests();
+
     // Private implementation
     class Impl;
     const std::unique_ptr<Impl> impl;

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/CMakeLists.txt

@@ -29,6 +29,7 @@ add_library(
     ${PROJECT_SOURCE_DIR}/test/src/mbgl/test/getrss.cpp
     ${PROJECT_SOURCE_DIR}/test/src/mbgl/test/sqlite3_test_fs.cpp
     ${PROJECT_SOURCE_DIR}/test/src/mbgl/test/stub_file_source.cpp
+    ${PROJECT_SOURCE_DIR}/test/src/mbgl/test/delayed_file_source.cpp
     ${PROJECT_SOURCE_DIR}/test/src/mbgl/test/test.cpp
     ${PROJECT_SOURCE_DIR}/test/src/mbgl/test/util.cpp
     ${PROJECT_SOURCE_DIR}/test/storage/asset_file_source.test.cpp

test/src/mbgl/test/delayed_file_source.cpp

@@ -0,0 +1,32 @@
+#include <mbgl/test/delayed_file_source.hpp>
+
+namespace mbgl {
+
+DelayedFileSource::AsyncRequestImpl::AsyncRequestImpl(DelayedFileSource& fs, Callback callback_)
+    : fileSource(fs),
+      callback(std::move(callback_)) {}
+
+void DelayedFileSource::AsyncRequestImpl::respond() {
+    if (callback) {
+        callback(*fileSource.response);
+    }
+}
+
+DelayedFileSource::DelayedFileSource()
+    : StubFileSource(StubFileSource::ResponseType::Synchronous) {}
+
+std::unique_ptr<AsyncRequest> DelayedFileSource::request(const Resource&, Callback callback) {
+    auto req = std::make_unique<AsyncRequestImpl>(*this, std::move(callback));
+    pendingRequest = req.get();
+    return req;
+}
+
+void DelayedFileSource::respondToRequest(const std::string& data) {
+    response = Response();
+    response->data = std::make_shared<std::string>(data);
+    if (pendingRequest) {
+        pendingRequest->respond();
+    }
+}
+
+} // namespace mbgl

test/src/mbgl/test/delayed_file_source.hpp

@@ -0,0 +1,43 @@
+#pragma once
+
+#include <mbgl/storage/file_source.hpp>
+#include <mbgl/util/async_request.hpp>
+#include <mbgl/test/stub_file_source.hpp>
+#include <memory>
+#include <string>
+#include <optional>
+
+namespace mbgl {
+
+/*
+  This is a file source that allows control of when a request is responded to.
+*/
+class DelayedFileSource : public StubFileSource {
+public:
+    DelayedFileSource();
+    ~DelayedFileSource() override = default;
+
+    std::unique_ptr<AsyncRequest> request(const Resource&, Callback) override;
+
+    // Custom method to respond to pending request
+    void respondToRequest(const std::string& data);
+
+private:
+    class AsyncRequestImpl : public AsyncRequest {
+    public:
+        using Callback = std::function<void(const Response&)>;
+
+        AsyncRequestImpl(DelayedFileSource& fs, Callback callback_);
+        void respond();
+
+    private:
+        DelayedFileSource& fileSource;
+        Callback callback;
+    };
+
+    friend class AsyncRequestImpl;
+    std::optional<Response> response;
+    AsyncRequestImpl* pendingRequest = nullptr;
+};
+
+} // namespace mbgl

test/style/style.test.cpp

@@ -9,6 +9,7 @@
 #include <mbgl/style/layers/line_layer.hpp>
 #include <mbgl/util/io.hpp>
 #include <mbgl/util/run_loop.hpp>
+#include <mbgl/test/delayed_file_source.hpp>
 
 #include <memory>
 
@@ -110,6 +111,38 @@ TEST(Style, RemoveSourceInUse) {
     EXPECT_EQ(log.count(logMessage), 1u);
 }
 
+TEST(Style, LoadJSONCancelsPendingLoadURL) {
+    util::RunLoop loop;
+
+    auto fileSource = std::make_shared<::DelayedFileSource>();
+    Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}};
+
+    // Start loading a URL (this will be pending)
+    auto url = "http://some-url";
+    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);
+
+    // Now respond to the original URL request - this should be ignored
+    fileSource->respondToRequest(R"STYLE({"version":8,"name":"Streets"})STYLE");
+
+    // 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>();