[core, android] Factor JSON string conversions

This adds a `convertJSON` template, to be used like:

   Error error
   optional<Foo> foo = convertJSON<Foo>(string, error);

Internally, it parses the string with RapidJSON and then calls `convert<Foo>(parsed, error)`.

While here, rationalize GeoJSON converters and fix error handling for Tileset conversion in OfflineDownload.

19 files changed, 131 insertions, 187 deletions

diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake
index 51dedb0052..6026374de6 100644
--- a/cmake/test-files.cmake
+++ b/cmake/test-files.cmake
@@ -59,6 +59,7 @@ set(MBGL_TEST_FILES
     test/src/mbgl/test/stub_file_source.hpp
     test/src/mbgl/test/stub_geometry_tile_feature.hpp
     test/src/mbgl/test/stub_layer_observer.hpp
+    test/src/mbgl/test/stub_render_source_observer.hpp
     test/src/mbgl/test/stub_style_observer.hpp
     test/src/mbgl/test/stub_tile_observer.hpp
     test/src/mbgl/test/test.cpp
@@ -100,7 +101,6 @@ set(MBGL_TEST_FILES
     test/style/style_image.test.cpp
     test/style/style_layer.test.cpp
     test/style/style_parser.test.cpp
-    test/style/tile_source.test.cpp
 
     # text
     test/text/glyph_atlas.test.cpp
diff --git a/include/mbgl/style/conversion/geojson.hpp b/include/mbgl/style/conversion/geojson.hpp
index 6e6e97dbc0..0b594f066c 100644
--- a/include/mbgl/style/conversion/geojson.hpp
+++ b/include/mbgl/style/conversion/geojson.hpp
@@ -1,14 +1,22 @@
 #pragma once
 
 #include <mbgl/style/conversion.hpp>
-#include <mbgl/style/sources/geojson_source.hpp>
+#include <mbgl/util/geojson.hpp>
 
 namespace mbgl {
 namespace style {
 namespace conversion {
 
-template <class V>
-optional<GeoJSON> convertGeoJSON(const V& value, Error& error);
+template <>
+struct Converter<GeoJSON> {
+public:
+    optional<GeoJSON> operator()(const std::string&, Error&) const;
+
+    // This is explicitly specialized in the .cpp file for JSValue. It may also be explicitly
+    // specialized for SDK-specific types (e.g. mbgl::android::Value).
+    template <class V>
+    optional<GeoJSON> operator()(const V&, Error&) const;
+};
 
 } // namespace conversion
 } // namespace style
diff --git a/include/mbgl/style/conversion/source.hpp b/include/mbgl/style/conversion/source.hpp
index 2371183b1c..dc7cdc0d42 100644
--- a/include/mbgl/style/conversion/source.hpp
+++ b/include/mbgl/style/conversion/source.hpp
@@ -122,7 +122,7 @@ private:
         auto result = std::make_unique<GeoJSONSource>(id, *options);
 
         if (isObject(*dataValue)) {
-            optional<GeoJSON> geoJSON = convertGeoJSON(*dataValue, error);
+            optional<GeoJSON> geoJSON = convert<GeoJSON>(*dataValue, error);
             if (!geoJSON) {
                 return {};
             }
diff --git a/platform/android/src/style/conversion/geojson.hpp b/platform/android/src/style/conversion/geojson.hpp
index 32a473b092..748fe7361e 100644
--- a/platform/android/src/style/conversion/geojson.hpp
+++ b/platform/android/src/style/conversion/geojson.hpp
@@ -10,25 +10,15 @@ namespace style {
 namespace conversion {
 
 template <>
-optional<GeoJSON> convertGeoJSON(const mbgl::android::Value& value, Error& error) {
-
+optional<GeoJSON> Converter<GeoJSON>::operator()(const mbgl::android::Value& value, Error& error) const {
     if(value.isNull() || !value.isString()) {
         error = { "no json data found" };
         return {};
     }
 
-    return convertGeoJSON(value.toString(), error);
+    return convert<GeoJSON>(value.toString(), error);
 }
 
-template <>
-struct Converter<GeoJSON> {
-
-    optional<GeoJSON> operator()(const mbgl::android::Value& value, Error& error) const {
-        return convertGeoJSON(value, error);
-    }
-
-};
-
 } // namespace conversion
 } // namespace style
 } // namespace mbgl
diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp
index cba2f70df8..901f996a4f 100644
--- a/platform/default/mbgl/storage/offline_download.cpp
+++ b/platform/default/mbgl/storage/offline_download.cpp
@@ -7,6 +7,8 @@
 #include <mbgl/style/parser.hpp>
 #include <mbgl/style/sources/geojson_source_impl.hpp>
 #include <mbgl/style/tile_source_impl.hpp>
+#include <mbgl/style/conversion/json.hpp>
+#include <mbgl/style/conversion/tileset.hpp>
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/util/mapbox.hpp>
 #include <mbgl/util/run_loop.hpp>
@@ -87,9 +89,12 @@ OfflineRegionStatus OfflineDownload::getStatus() const {
                 const std::string& url = urlOrTileset.get<std::string>();
                 optional<Response> sourceResponse = offlineDatabase.get(Resource::source(url));
                 if (sourceResponse) {
-                    result.requiredResourceCount +=
-                        definition.tileCover(type, tileSize, style::TileSourceImpl::parseTileJSON(
-                            *sourceResponse->data, url, type, tileSize).zoomRange).size();
+                    style::conversion::Error error;
+                    optional<Tileset> tileset = style::conversion::convertJSON<Tileset>(*sourceResponse->data, error);
+                    if (tileset) {
+                        result.requiredResourceCount +=
+                            definition.tileCover(type, tileSize, (*tileset).zoomRange).size();
+                    }
                 } else {
                     result.requiredResourceCountIsPrecise = false;
                 }
@@ -152,12 +157,16 @@ void OfflineDownload::activateDownload() {
                     requiredSourceURLs.insert(url);
 
                     ensureResource(Resource::source(url), [=](Response sourceResponse) {
-                        queueTiles(type, tileSize, style::TileSourceImpl::parseTileJSON(
-                            *sourceResponse.data, url, type, tileSize));
-
-                        requiredSourceURLs.erase(url);
-                        if (requiredSourceURLs.empty()) {
-                            status.requiredResourceCountIsPrecise = true;
+                        style::conversion::Error error;
+                        optional<Tileset> tileset = style::conversion::convertJSON<Tileset>(*sourceResponse.data, error);
+                        if (tileset) {
+                            util::mapbox::canonicalizeTileset(*tileset, url, type, tileSize);
+                            queueTiles(type, tileSize, *tileset);
+
+                            requiredSourceURLs.erase(url);
+                            if (requiredSourceURLs.empty()) {
+                                status.requiredResourceCountIsPrecise = true;
+                            }
                         }
                     });
                 }
diff --git a/platform/node/src/node_geojson.hpp b/platform/node/src/node_geojson.hpp
index e4b2ca47e7..9bae86b76a 100644
--- a/platform/node/src/node_geojson.hpp
+++ b/platform/node/src/node_geojson.hpp
@@ -5,7 +5,7 @@ namespace style {
 namespace conversion {
 
 template <>
-optional<GeoJSON> convertGeoJSON(const v8::Local<v8::Value>&, Error& error) {
+optional<GeoJSON> Converter<GeoJSON>::operator()(const v8::Local<v8::Value>&, Error& error) const {
     error = { "not implemented" };
     return {};
 }
diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp
index edb8afa28a..ce7e237afb 100644
--- a/platform/qt/src/qmapboxgl.cpp
+++ b/platform/qt/src/qmapboxgl.cpp
@@ -1303,7 +1303,7 @@ void QMapboxGL::updateSource(const QString &id, const QVariantMap &params)
 
     if (params.contains("data")) {
         Error error;
-        auto result = convertGeoJSON(params["data"], error);
+        auto result = convert<mbgl::GeoJSON>(params["data"], error);
         if (result) {
             sourceGeoJSON->setGeoJSON(*result);
         }
diff --git a/platform/qt/src/qt_geojson.hpp b/platform/qt/src/qt_geojson.hpp
index 7d12660aec..a6958b7edc 100644
--- a/platform/qt/src/qt_geojson.hpp
+++ b/platform/qt/src/qt_geojson.hpp
@@ -178,25 +178,20 @@ namespace style {
 namespace conversion {
 
 template <>
-optional<GeoJSON> convertGeoJSON(const QMapbox::Feature& feature, Error&) {
-    return GeoJSON { asMapboxGLFeature(feature) };
-}
-
-template <>
-optional<GeoJSON> convertGeoJSON(const QVariant& value, Error& error) {
+optional<GeoJSON> Converter<GeoJSON>::operator()(const QVariant& value, Error& error) const {
 #if QT_VERSION >= 0x050000
     if (value.typeName() == QStringLiteral("QMapbox::Feature")) {
 #else
     if (value.typeName() == QString("QMapbox::Feature")) {
 #endif
-        return convertGeoJSON(value.value<QMapbox::Feature>(), error);
+        return GeoJSON { asMapboxGLFeature(value.value<QMapbox::Feature>()) };
     } else if (value.type() != QVariant::ByteArray) {
         error = { "JSON data must be in QByteArray" };
         return {};
     }
 
     QByteArray data = value.toByteArray();
-    return convertGeoJSON(std::string(data.constData(), data.size()), error);
+    return convert<GeoJSON>(std::string(data.constData(), data.size()), error);
 }
 
 } // namespace conversion
diff --git a/src/mbgl/style/conversion/geojson.cpp b/src/mbgl/style/conversion/geojson.cpp
index 0fba6e5b4d..8103e9014a 100644
--- a/src/mbgl/style/conversion/geojson.cpp
+++ b/src/mbgl/style/conversion/geojson.cpp
@@ -1,33 +1,26 @@
 #include <mbgl/style/conversion/geojson.hpp>
-
+#include <mbgl/style/conversion/json.hpp>
 #include <mbgl/util/rapidjson.hpp>
 
-#include <string>
-#include <sstream>
+#include <mapbox/geojson.hpp>
+#include <mapbox/geojson/rapidjson.hpp>
 
 namespace mbgl {
 namespace style {
 namespace conversion {
 
-template <>
-optional<GeoJSON> convertGeoJSON(const std::string& string, Error& error) {
-    rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> d;
-    d.Parse(string.c_str());
-
-    if (d.HasParseError()) {
-        std::stringstream message;
-        message << d.GetErrorOffset() << " - " << rapidjson::GetParseError_En(d.GetParseError());
-        error = { message.str() };
-        return {};
-    }
+optional<GeoJSON> Converter<GeoJSON>::operator()(const std::string& value, Error& error) const {
+    return convertJSON<GeoJSON>(value, error);
+}
 
-    optional<GeoJSON> geoJSON = conversion::convertGeoJSON<JSValue>(d, error);
-    if (!geoJSON) {
-        error = { error.message };
+template <>
+optional<GeoJSON> Converter<GeoJSON>::operator()(const JSValue& value, Error& error) const {
+    try {
+        return mapbox::geojson::convert(value);
+    } catch (const std::exception& ex) {
+        error = { ex.what() };
         return {};
     }
-
-    return geoJSON;
 }
 
 } // namespace conversion
diff --git a/src/mbgl/style/conversion/json.hpp b/src/mbgl/style/conversion/json.hpp
new file mode 100644
index 0000000000..0817ac09df
--- /dev/null
+++ b/src/mbgl/style/conversion/json.hpp
@@ -0,0 +1,28 @@
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/style/rapidjson_conversion.hpp>
+
+#include <string>
+#include <sstream>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+template <class T, class...Args>
+optional<T> convertJSON(const std::string& json, Error& error, Args&&...args) {
+    JSDocument document;
+    document.Parse<0>(json.c_str());
+
+    if (document.HasParseError()) {
+        std::stringstream message;
+        message << document.GetErrorOffset() << " - " << rapidjson::GetParseError_En(document.GetParseError());
+        error = { message.str() };
+        return {};
+    }
+
+    return convert<T, JSValue>(document, error, std::forward<Args>(args)...);
+}
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl
diff --git a/src/mbgl/style/sources/geojson_source_impl.cpp b/src/mbgl/style/sources/geojson_source_impl.cpp
index 08ed95ea3f..1a686ff9bc 100644
--- a/src/mbgl/style/sources/geojson_source_impl.cpp
+++ b/src/mbgl/style/sources/geojson_source_impl.cpp
@@ -1,37 +1,18 @@
 #include <mbgl/style/sources/geojson_source_impl.hpp>
+#include <mbgl/style/conversion/json.hpp>
 #include <mbgl/style/conversion/geojson.hpp>
 #include <mbgl/style/source_observer.hpp>
 #include <mbgl/tile/tile_id.hpp>
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/renderer/sources/render_geojson_source.hpp>
-#include <mbgl/util/rapidjson.hpp>
 #include <mbgl/util/constants.cpp>
 #include <mbgl/util/logging.hpp>
 
-#include <mapbox/geojson.hpp>
-#include <mapbox/geojson/rapidjson.hpp>
 #include <mapbox/geojsonvt.hpp>
-#include <mapbox/geojsonvt/convert.hpp>
 #include <supercluster.hpp>
 
-#include <rapidjson/error/en.h>
-
-#include <sstream>
-
 namespace mbgl {
 namespace style {
-namespace conversion {
-
-template <>
-optional<GeoJSON> convertGeoJSON(const JSValue& value, Error& error) {
-    try {
-        return mapbox::geojson::convert(value);
-    } catch (const std::exception& ex) {
-        error = { ex.what() };
-        return {};
-    }
-}
-} // namespace conversion
 
 class GeoJSONVTData : public GeoJSONData {
 public:
@@ -129,20 +110,8 @@ void GeoJSONSource::Impl::loadDescription(FileSource& fileSource) {
             observer->onSourceError(
                 base, std::make_exception_ptr(std::runtime_error("unexpectedly empty GeoJSON")));
         } else {
-            rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> d;
-            d.Parse<0>(res.data->c_str());
-
-            if (d.HasParseError()) {
-                std::stringstream message;
-                message << d.GetErrorOffset() << " - "
-                        << rapidjson::GetParseError_En(d.GetParseError());
-                observer->onSourceError(base,
-                                        std::make_exception_ptr(std::runtime_error(message.str())));
-                return;
-            }
-
             conversion::Error error;
-            optional<GeoJSON> geoJSON = conversion::convertGeoJSON<JSValue>(d, error);
+            optional<GeoJSON> geoJSON = conversion::convertJSON<GeoJSON>(*res.data, error);
             if (!geoJSON) {
                 Log::Error(Event::ParseStyle, "Failed to parse GeoJSON data: %s",
                            error.message.c_str());
diff --git a/src/mbgl/style/tile_source_impl.cpp b/src/mbgl/style/tile_source_impl.cpp
index 4623714794..d2ce3def9f 100644
--- a/src/mbgl/style/tile_source_impl.cpp
+++ b/src/mbgl/style/tile_source_impl.cpp
@@ -1,45 +1,13 @@
 #include <mbgl/style/tile_source_impl.hpp>
 #include <mbgl/style/source_observer.hpp>
-#include <mbgl/style/rapidjson_conversion.hpp>
+#include <mbgl/style/conversion/json.hpp>
 #include <mbgl/style/conversion/tileset.hpp>
-#include <mbgl/util/tileset.hpp>
 #include <mbgl/util/mapbox.hpp>
 #include <mbgl/storage/file_source.hpp>
 
-#include <rapidjson/document.h>
-#include <rapidjson/error/en.h>
-
-#include <sstream>
-
 namespace mbgl {
 namespace style {
 
-Tileset TileSourceImpl::parseTileJSON(const std::string& json, const std::string& sourceURL, SourceType type, uint16_t tileSize) {
-    rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> document;
-    document.Parse<0>(json.c_str());
-
-    if (document.HasParseError()) {
-        std::stringstream message;
-        message << document.GetErrorOffset() << " - " << rapidjson::GetParseError_En(document.GetParseError());
-        throw std::runtime_error(message.str());
-    }
-
-    conversion::Error error;
-    optional<Tileset> result = conversion::convert<Tileset, JSValue>(document, error);
-    if (!result) {
-        throw std::runtime_error(error.message);
-    }
-
-    // TODO: Remove this hack by delivering proper URLs in the TileJSON to begin with.
-    if (util::mapbox::isMapboxURL(sourceURL)) {
-        for (auto& url : (*result).tiles) {
-            url = util::mapbox::canonicalizeTileURL(url, type, tileSize);
-        }
-    }
-
-    return *result;
-}
-
 TileSourceImpl::TileSourceImpl(SourceType type_, std::string id_, Source& base_,
                                variant<std::string, Tileset> urlOrTileset_,
                                uint16_t tileSize_)
@@ -70,21 +38,17 @@ void TileSourceImpl::loadDescription(FileSource& fileSource) {
         } else if (res.noContent) {
             observer->onSourceError(base, std::make_exception_ptr(std::runtime_error("unexpectedly empty TileJSON")));
         } else {
-            Tileset newTileset;
-
-            // Create a new copy of the Tileset object that holds the base values we've parsed
-            // from the stylesheet. Then merge in the values parsed from the TileJSON we retrieved
-            // via the URL.
-            try {
-                newTileset = parseTileJSON(*res.data, url, type, tileSize);
-            } catch (...) {
-                observer->onSourceError(base, std::current_exception());
+            conversion::Error error;
+            optional<Tileset> newTileset = conversion::convertJSON<Tileset>(*res.data, error);
+            if (!newTileset) {
+                observer->onSourceError(base, std::make_exception_ptr(std::runtime_error(error.message)));
                 return;
             }
 
-            bool attributionChanged = tileset.attribution != newTileset.attribution;
+            util::mapbox::canonicalizeTileset(*newTileset, url, type, tileSize);
+            bool attributionChanged = tileset.attribution != (*newTileset).attribution;
 
-            tileset = newTileset;
+            tileset = *newTileset;
             loaded = true;
 
             observer->onSourceLoaded(base);
diff --git a/src/mbgl/style/tile_source_impl.hpp b/src/mbgl/style/tile_source_impl.hpp
index 405408ff54..0e5a53add7 100644
--- a/src/mbgl/style/tile_source_impl.hpp
+++ b/src/mbgl/style/tile_source_impl.hpp
@@ -17,8 +17,6 @@ namespace style {
 */
 class TileSourceImpl : public Source::Impl {
 public:
-    static Tileset parseTileJSON(const std::string& json, const std::string& sourceURL, SourceType, uint16_t tileSize);
-
     TileSourceImpl(SourceType, std::string id, Source&,
                    variant<std::string, Tileset> urlOrTileset,
                    uint16_t tileSize);
diff --git a/src/mbgl/util/mapbox.cpp b/src/mbgl/util/mapbox.cpp
index c03cc2a8e7..8cbc85d492 100644
--- a/src/mbgl/util/mapbox.cpp
+++ b/src/mbgl/util/mapbox.cpp
@@ -2,6 +2,7 @@
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/url.hpp>
+#include <mbgl/util/tileset.hpp>
 
 #include <stdexcept>
 #include <vector>
@@ -170,6 +171,15 @@ canonicalizeTileURL(const std::string& str, const SourceType type, const uint16_
     return result;
 }
 
+void canonicalizeTileset(Tileset& tileset, const std::string& sourceURL, SourceType type, uint16_t tileSize) {
+    // TODO: Remove this hack by delivering proper URLs in the TileJSON to begin with.
+    if (isMapboxURL(sourceURL)) {
+        for (auto& url : tileset.tiles) {
+            url = canonicalizeTileURL(url, type, tileSize);
+        }
+    }
+}
+
 const uint64_t DEFAULT_OFFLINE_TILE_COUNT_LIMIT = 6000;
 
 } // end namespace mapbox
diff --git a/src/mbgl/util/mapbox.hpp b/src/mbgl/util/mapbox.hpp
index 72f4e0a567..f3dfdd0b01 100644
--- a/src/mbgl/util/mapbox.hpp
+++ b/src/mbgl/util/mapbox.hpp
@@ -4,6 +4,9 @@
 #include <mbgl/style/types.hpp>
 
 namespace mbgl {
+
+class Tileset;
+
 namespace util {
 namespace mapbox {
 
@@ -18,6 +21,9 @@ std::string normalizeTileURL(const std::string& baseURL, const std::string& url,
 // Return a "mapbox://tiles/..." URL (suitable for normalizeTileURL) for the given Mapbox tile URL.
 std::string canonicalizeTileURL(const std::string& url, SourceType, uint16_t tileSize);
 
+// Replace URL templates with "mapbox://tiles/..." URLs (suitable for normalizeTileURL).
+void canonicalizeTileset(Tileset&, const std::string& url, SourceType, uint16_t tileSize);
+
 extern const uint64_t DEFAULT_OFFLINE_TILE_COUNT_LIMIT;
 
 } // namespace mapbox
diff --git a/test/fixtures/style_parser/tilejson.raster.json b/test/fixtures/style_parser/tilejson.raster.json
deleted file mode 100644
index 3fc819f292..0000000000
--- a/test/fixtures/style_parser/tilejson.raster.json
+++ /dev/null
@@ -1,8 +0,0 @@
-{
-  "minzoom": 0,
-  "maxzoom": 15,
-  "center": [ 1, 2, 3 ],
-  "bounds": [ 4, 5, 6, 7 ],
-  "attribution": "attribution",
-  "tiles": [ "http://a.tiles.mapbox.com/v4/mapbox.satellite/{z}/{x}/{y}.png?access_token=key" ]
-}
diff --git a/test/fixtures/style_parser/tilejson.vector.json b/test/fixtures/style_parser/tilejson.vector.json
deleted file mode 100644
index 9144bd502c..0000000000
--- a/test/fixtures/style_parser/tilejson.vector.json
+++ /dev/null
@@ -1,8 +0,0 @@
-{
-  "minzoom": 0,
-  "maxzoom": 15,
-  "center": [ 1, 2, 3 ],
-  "bounds": [ 4, 5, 6, 7 ],
-  "attribution": "attribution",
-  "tiles": [ "http://a.tiles.mapbox.com/v4/mapbox.streets/{z}/{x}/{y}.vector.pbf?access_token=key" ]
-}
diff --git a/test/style/tile_source.test.cpp b/test/style/tile_source.test.cpp
deleted file mode 100644
index 35d037a049..0000000000
--- a/test/style/tile_source.test.cpp
+++ /dev/null
@@ -1,37 +0,0 @@
-#include <mbgl/test/util.hpp>
-
-#include <mbgl/style/tile_source_impl.hpp>
-#include <mbgl/util/io.hpp>
-
-using namespace mbgl;
-using namespace mbgl::style;
-
-TEST(TileSourceImpl, ParseTileJSONRaster) {
-    auto result = TileSourceImpl::parseTileJSON(
-        util::read_file("test/fixtures/style_parser/tilejson.raster.json"),
-        "mapbox://mapbox.satellite",
-        SourceType::Raster,
-        256);
-
-    EXPECT_EQ(0, result.zoomRange.min);
-    EXPECT_EQ(15, result.zoomRange.max);
-    EXPECT_EQ("attribution", result.attribution);
-#if !defined(__ANDROID__) && !defined(__APPLE__)
-    EXPECT_EQ("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.webp", result.tiles[0]);
-#else
-    EXPECT_EQ("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.png", result.tiles[0]);
-#endif
-}
-
-TEST(TileSourceImpl, ParseTileJSONVector) {
-    auto result = TileSourceImpl::parseTileJSON(
-        util::read_file("test/fixtures/style_parser/tilejson.vector.json"),
-        "mapbox://mapbox.streets",
-        SourceType::Vector,
-        256);
-
-    EXPECT_EQ(0, result.zoomRange.min);
-    EXPECT_EQ(15, result.zoomRange.max);
-    EXPECT_EQ("attribution", result.attribution);
-    EXPECT_EQ("mapbox://tiles/mapbox.streets/{z}/{x}/{y}.vector.pbf", result.tiles[0]);
-}
diff --git a/test/util/mapbox.test.cpp b/test/util/mapbox.test.cpp
index 299f0df833..ba867ccb5c 100644
--- a/test/util/mapbox.test.cpp
+++ b/test/util/mapbox.test.cpp
@@ -3,6 +3,7 @@
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/mapbox.hpp>
 #include <mbgl/util/constants.hpp>
+#include <mbgl/util/tileset.hpp>
 #include <stdexcept>
 
 using namespace mbgl;
@@ -210,3 +211,29 @@ TEST(Mapbox, CanonicalURL) {
         "http://api.mapbox.com/v4/a.b/{z}/{x}/{y}/.",
         mbgl::util::mapbox::canonicalizeTileURL("http://api.mapbox.com/v4/a.b/{z}/{x}/{y}/.", SourceType::Raster, 256));
 }
+
+TEST(Mapbox, CanonicalizeRasterTileset) {
+    mbgl::Tileset tileset;
+    tileset.tiles = {
+        "http://a.tiles.mapbox.com/v4/mapbox.satellite/{z}/{x}/{y}.png?access_token=key"
+    };
+
+    mbgl::util::mapbox::canonicalizeTileset(tileset, "mapbox://mapbox.satellite", SourceType::Raster, 256);
+
+#if !defined(__ANDROID__) && !defined(__APPLE__)
+    EXPECT_EQ("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.webp", tileset.tiles[0]);
+#else
+    EXPECT_EQ("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.png", tileset.tiles[0]);
+#endif
+}
+
+TEST(Mapbox, CanonicalizeVectorTileset) {
+    mbgl::Tileset tileset;
+    tileset.tiles = {
+        "http://a.tiles.mapbox.com/v4/mapbox.streets/{z}/{x}/{y}.vector.pbf?access_token=key"
+    };
+
+    mbgl::util::mapbox::canonicalizeTileset(tileset, "mapbox://mapbox.streets", SourceType::Vector, 512);
+
+    EXPECT_EQ("mapbox://tiles/mapbox.streets/{z}/{x}/{y}.vector.pbf", tileset.tiles[0]);
+}