[core] Use the ResourceTransform instead of callbacks

Based on the actor model, can be considered safer to use since
we don't need to worry about object lifetime anymore.

include/mbgl/storage/default_file_source.hpp

@@ -1,8 +1,10 @@
 #pragma once
 
+#include <mbgl/actor/actor_ref.hpp>
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/storage/offline.hpp>
 #include <mbgl/util/constants.hpp>
+#include <mbgl/util/optional.hpp>
 
 #include <vector>
 #include <mutex>
@@ -13,6 +15,8 @@ namespace util {
 template <typename T> class Thread;
 } // namespace util
 
+class ResourceTransform;
+
 class DefaultFileSource : public FileSource {
 public:
     /*
@@ -40,7 +44,7 @@ public:
     void setAccessToken(const std::string&);
     std::string getAccessToken();
 
-    void setResourceTransform(std::function<std::string(Resource::Kind, std::string&&)>);
+    void setResourceTransform(optional<ActorRef<ResourceTransform>>&&);
 
     std::unique_ptr<AsyncRequest> request(const Resource&, Callback) override;
 

include/mbgl/storage/online_file_source.hpp

@@ -1,10 +1,14 @@
 #pragma once
 
+#include <mbgl/actor/actor_ref.hpp>
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/util/constants.hpp>
+#include <mbgl/util/optional.hpp>
 
 namespace mbgl {
 
+class ResourceTransform;
+
 class OnlineFileSource : public FileSource {
 public:
     OnlineFileSource();
@@ -16,9 +20,7 @@ public:
     void setAccessToken(const std::string& t) { accessToken = t; }
     std::string getAccessToken() const { return accessToken; }
 
-    using ResourceTransform =
-        std::function<std::unique_ptr<AsyncRequest>(Resource::Kind, std::string&&, std::function<void(std::string&&)>)>;
-    void setResourceTransform(ResourceTransform&& cb);
+    void setResourceTransform(optional<ActorRef<ResourceTransform>>&&);
 
     std::unique_ptr<AsyncRequest> request(const Resource&, Callback) override;
 

platform/default/default_file_source.cpp

@@ -5,6 +5,7 @@
 #include <mbgl/storage/online_file_source.hpp>
 #include <mbgl/storage/offline_database.hpp>
 #include <mbgl/storage/offline_download.hpp>
+#include <mbgl/storage/resource_transform.hpp>
 
 #include <mbgl/util/platform.hpp>
 #include <mbgl/util/url.hpp>
@@ -49,7 +50,7 @@ public:
         return onlineFileSource.getAccessToken();
     }
 
-    void setResourceTransform(OnlineFileSource::ResourceTransform&& transform) {
+    void setResourceTransform(optional<ActorRef<ResourceTransform>>&& transform) {
         onlineFileSource.setResourceTransform(std::move(transform));
     }
 
@@ -227,17 +228,8 @@ std::string DefaultFileSource::getAccessToken() {
     return cachedAccessToken;
 }
 
-void DefaultFileSource::setResourceTransform(std::function<std::string(Resource::Kind, std::string&&)> transform) {
-    if (transform) {
-        auto loop = util::RunLoop::Get();
-        impl->actor().invoke(&Impl::setResourceTransform, [loop, transform](Resource::Kind kind_, std::string&& url_, auto callback_) {
-            return loop->invokeWithCallback([transform](Resource::Kind kind, std::string&& url, auto callback) {
-                callback(transform(kind, std::move(url)));
-            }, kind_, std::move(url_), callback_);
-        });
-    } else {
-        impl->actor().invoke(&Impl::setResourceTransform, nullptr);
-    }
+void DefaultFileSource::setResourceTransform(optional<ActorRef<ResourceTransform>>&& transform) {
+    impl->actor().invoke(&Impl::setResourceTransform, std::move(transform));
 }
 
 std::unique_ptr<AsyncRequest> DefaultFileSource::request(const Resource& resource, Callback callback) {

platform/default/online_file_source.cpp

@@ -2,15 +2,18 @@
 #include <mbgl/storage/http_file_source.hpp>
 #include <mbgl/storage/network_status.hpp>
 
+#include <mbgl/storage/resource_transform.hpp>
 #include <mbgl/storage/response.hpp>
 #include <mbgl/util/logging.hpp>
 
+#include <mbgl/actor/mailbox.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/mapbox.hpp>
 #include <mbgl/util/exception.hpp>
 #include <mbgl/util/chrono.hpp>
 #include <mbgl/util/async_task.hpp>
 #include <mbgl/util/noncopyable.hpp>
+#include <mbgl/util/run_loop.hpp>
 #include <mbgl/util/timer.hpp>
 #include <mbgl/util/http_timeout.hpp>
 
@@ -34,12 +37,17 @@ public:
     void schedule(optional<Timestamp> expires);
     void completed(Response);
 
+    void setTransformedURL(const std::string&& url);
+    ActorRef<OnlineFileRequest> actor();
+
     OnlineFileSource::Impl& impl;
     Resource resource;
     std::unique_ptr<AsyncRequest> request;
     util::Timer timer;
     Callback callback;
 
+    std::shared_ptr<Mailbox> mailbox;
+
     // Counts the number of times a response was already expired when received. We're using
     // this to add a delay when making a new request so we don't keep retrying immediately
     // in case of a server serving expired tiles.
@@ -65,15 +73,12 @@ public:
     void add(OnlineFileRequest* request) {
         allRequests.insert(request);
         if (resourceTransform) {
-            // When there's a Resource transform callback set, replace the resource with the
+            // Request the ResourceTransform actor a new url and replace the resource url with the
             // transformed one before proceeding to schedule the request.
-            request->request =
-                resourceTransform(request->resource.kind, std::move(request->resource.url),
-                                  [request](std::string&& url) {
-                                      request->request.release();
-                                      request->resource.url = std::move(url);
-                                      request->schedule();
-                                  });
+            resourceTransform->invoke(&ResourceTransform::transform, request->resource.kind,
+                std::move(request->resource.url), [ref = request->actor()](const std::string&& url) mutable {
+                    ref.invoke(&OnlineFileRequest::setTransformedURL, std::move(url));
+                });
         } else {
             request->schedule();
         }
@@ -144,7 +149,7 @@ public:
         return activeRequests.find(request) != activeRequests.end();
     }
 
-    void setResourceTransform(ResourceTransform&& transform) {
+    void setResourceTransform(optional<ActorRef<ResourceTransform>>&& transform) {
         resourceTransform = std::move(transform);
     }
 
@@ -155,7 +160,7 @@ private:
         }
     }
 
-    ResourceTransform resourceTransform;
+    optional<ActorRef<ResourceTransform>> resourceTransform;
 
     /**
      * The lifetime of a request is:
@@ -216,7 +221,7 @@ std::unique_ptr<AsyncRequest> OnlineFileSource::request(const Resource& resource
     return std::make_unique<OnlineFileRequest>(std::move(res), std::move(callback), *impl);
 }
 
-void OnlineFileSource::setResourceTransform(ResourceTransform&& transform) {
+void OnlineFileSource::setResourceTransform(optional<ActorRef<ResourceTransform>>&& transform) {
     impl->setResourceTransform(std::move(transform));
 }
 
@@ -361,4 +366,19 @@ void OnlineFileRequest::networkIsReachableAgain() {
     }
 }
 
+void OnlineFileRequest::setTransformedURL(const std::string&& url) {
+     resource.url = std::move(url);
+     schedule();
+}
+
+ActorRef<OnlineFileRequest> OnlineFileRequest::actor() {
+    if (!mailbox) {
+        // Lazy constructed because this can be costly and
+        // the ResourceTransform is not used by many apps.
+        mailbox = std::make_shared<Mailbox>(*util::RunLoop::Get());
+    }
+
+    return ActorRef<OnlineFileRequest>(*this, mailbox);
+}
+
 } // namespace mbgl

test/storage/default_file_source.test.cpp

@@ -1,5 +1,7 @@
+#include <mbgl/actor/actor.hpp>
 #include <mbgl/test/util.hpp>
 #include <mbgl/storage/default_file_source.hpp>
+#include <mbgl/storage/resource_transform.hpp>
 #include <mbgl/util/run_loop.hpp>
 
 using namespace mbgl;
@@ -482,7 +484,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(SetResourceTransform)) {
     DefaultFileSource fs(":memory:", ".");
 
     // Translates the URL "localhost://test to http://127.0.0.1:3000/test
-    fs.setResourceTransform([](Resource::Kind, std::string&& url) -> std::string {
+    Actor<ResourceTransform> transform(loop, [](Resource::Kind, const std::string&& url) -> std::string {
         if (url == "localhost://test") {
             return "http://127.0.0.1:3000/test";
         } else {
@@ -490,10 +492,27 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(SetResourceTransform)) {
         }
     });
 
-    const Resource resource { Resource::Unknown, "localhost://test" };
+    fs.setResourceTransform(transform.self());
+    const Resource resource1 { Resource::Unknown, "localhost://test" };
 
     std::unique_ptr<AsyncRequest> req;
-    req = fs.request(resource, [&](Response res) {
+    req = fs.request(resource1, [&](Response res) {
+        req.reset();
+        EXPECT_EQ(nullptr, res.error);
+        ASSERT_TRUE(res.data.get());
+        EXPECT_EQ("Hello World!", *res.data);
+        EXPECT_FALSE(bool(res.expires));
+        EXPECT_FALSE(bool(res.modified));
+        EXPECT_FALSE(bool(res.etag));
+        loop.stop();
+    });
+
+    loop.run();
+
+    fs.setResourceTransform({});
+    const Resource resource2 { Resource::Unknown, "http://127.0.0.1:3000/test" };
+
+    req = fs.request(resource2, [&](Response res) {
         req.reset();
         EXPECT_EQ(nullptr, res.error);
         ASSERT_TRUE(res.data.get());