diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index eb273d785..faae3285f 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -287,26 +287,25 @@ TileData::State Source::addTile(MapData& data, new_tile.data = cache.get(normalized_id.to_uint64()); } - auto successCallback = std::bind(&Source::emitTileLoaded, this, true); - auto failureCallback = std::bind(&Source::emitTileLoadingFailed, this, std::placeholders::_1); - if (!new_tile.data) { + auto callback = std::bind(&Source::tileLoadingCompleteCallback, this, normalized_id); + // If we don't find working tile data, we're just going to load it. if (info.type == SourceType::Vector) { new_tile.data = std::make_shared(normalized_id, data.transform.getMaxZoom(), style, glyphAtlas, glyphStore, spriteAtlas, sprite, info); new_tile.data->request( - style.workers, transformState.getPixelRatio(), successCallback, failureCallback); + style.workers, transformState.getPixelRatio(), callback); } else if (info.type == SourceType::Raster) { new_tile.data = std::make_shared(normalized_id, texturePool, info); new_tile.data->request( - style.workers, transformState.getPixelRatio(), successCallback, failureCallback); + style.workers, transformState.getPixelRatio(), callback); } else if (info.type == SourceType::Annotations) { new_tile.data = std::make_shared(normalized_id, data.annotationManager, data.transform.getMaxZoom(), style, glyphAtlas, glyphStore, spriteAtlas, sprite, info); - new_tile.data->reparse(style.workers, successCallback); + new_tile.data->reparse(style.workers, callback); } else { throw std::runtime_error("source type not implemented"); } @@ -539,6 +538,25 @@ void Source::setObserver(Observer* observer) { observer_ = observer; } +void Source::tileLoadingCompleteCallback(const TileID& normalized_id) { + auto it = tile_data.find(normalized_id); + if (it == tile_data.end()) { + return; + } + + util::ptr data = it->second.lock(); + if (!data) { + return; + } + + if (data->getState() == TileData::State::obsolete && !data->getError().empty()) { + emitTileLoadingFailed(data->getError()); + return; + } + + emitTileLoaded(true); +} + void Source::emitSourceLoaded() { if (observer_) { observer_->onSourceLoaded(); diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp index d4c29b3e7..174dd996b 100644 --- a/src/mbgl/map/source.hpp +++ b/src/mbgl/map/source.hpp @@ -104,6 +104,8 @@ public: bool enabled; private: + void tileLoadingCompleteCallback(const TileID& normalized_id); + void emitSourceLoaded(); void emitSourceLoadingFailed(const std::string& message); void emitTileLoaded(bool isNewTile); diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp index fd3615f91..d5af3e1d1 100644 --- a/src/mbgl/map/sprite.cpp +++ b/src/mbgl/map/sprite.cpp @@ -45,14 +45,13 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_) jsonRequest = nullptr; if (res.status == Response::Successful) { body = res.data; - parseJSON(); + parseJSON(jsonURL); } else { std::stringstream message; message << "Failed to load [" << jsonURL << "]: " << res.message; emitSpriteLoadingFailed(message.str()); return; } - loadedJSON = true; emitSpriteLoadedIfComplete(); }); @@ -60,14 +59,13 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_) spriteRequest = nullptr; if (res.status == Response::Successful) { image = res.data; - parseImage(); + parseImage(spriteURL); } else { std::stringstream message; message << "Failed to load [" << spriteURL << "]: " << res.message; emitSpriteLoadingFailed(message.str()); return; } - loadedImage = true; emitSpriteLoadedIfComplete(); }); } @@ -105,21 +103,29 @@ bool Sprite::hasPixelRatio(float ratio) const { return pixelRatio == (ratio > 1 ? 2 : 1); } -void Sprite::parseImage() { +void Sprite::parseImage(const std::string& spriteURL) { raster = std::make_unique(image); if (!*raster) { raster.reset(); + std::stringstream message; + message << "Failed to parse [" << spriteURL << "]"; + emitSpriteLoadingFailed(message.str()); + return; } + image.clear(); + loadedImage = true; } -void Sprite::parseJSON() { +void Sprite::parseJSON(const std::string& jsonURL) { rapidjson::Document d; d.Parse<0>(body.c_str()); body.clear(); if (d.HasParseError()) { - Log::Warning(Event::Sprite, "sprite JSON is invalid"); + std::stringstream message; + message << "Failed to parse [" << jsonURL << "]: " << d.GetErrorOffset() << " - " << d.GetParseError(); + emitSpriteLoadingFailed(message.str()); } else if (d.IsObject()) { for (rapidjson::Value::ConstMemberIterator itr = d.MemberBegin(); itr != d.MemberEnd(); ++itr) { const std::string& name = itr->name.GetString(); @@ -142,8 +148,11 @@ void Sprite::parseJSON() { pos.emplace(name, SpritePosition { x, y, width, height, spritePixelRatio, sdf }); } } + loadedJSON = true; } else { - Log::Warning(Event::Sprite, "sprite JSON root is not an object"); + std::stringstream message; + message << "Failed to parse [" << jsonURL << "]: Root is not an object"; + emitSpriteLoadingFailed(message.str()); } } diff --git a/src/mbgl/map/sprite.hpp b/src/mbgl/map/sprite.hpp index 13cfbe2d8..b0fda3001 100644 --- a/src/mbgl/map/sprite.hpp +++ b/src/mbgl/map/sprite.hpp @@ -59,8 +59,8 @@ private: void emitSpriteLoadedIfComplete(); void emitSpriteLoadingFailed(const std::string& message); - void parseJSON(); - void parseImage(); + void parseJSON(const std::string& jsonURL); + void parseImage(const std::string& spriteURL); std::string body; std::string image; diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp index be0d7d32f..fc2d48bb5 100644 --- a/src/mbgl/map/tile_data.cpp +++ b/src/mbgl/map/tile_data.cpp @@ -38,18 +38,18 @@ void TileData::setState(const State& state_) { void TileData::request(Worker& worker, float pixelRatio, - const std::function& successCallback, - const std::function& failureCallback) { + const std::function& callback) { std::string url = source.tileURL(id, pixelRatio); state = State::loading; - req = env.request({ Resource::Kind::Tile, url }, [url, successCallback, failureCallback, &worker, this](const Response &res) { + req = env.request({ Resource::Kind::Tile, url }, [url, callback, &worker, this](const Response &res) { req = nullptr; if (res.status != Response::Successful) { std::stringstream message; message << "Failed to load [" << url << "]: " << res.message; - failureCallback(message.str()); + setError(message.str()); + callback(); return; } @@ -57,7 +57,7 @@ void TileData::request(Worker& worker, data = res.data; // Schedule tile parsing in another thread - reparse(worker, successCallback); + reparse(worker, callback); }); } @@ -88,3 +88,8 @@ bool TileData::reparse(Worker& worker, std::function callback) { workRequest = worker.send([this] { parse(); endParsing(); }, callback); return true; } + +void TileData::setError(const std::string& message) { + error = message; + setState(State::obsolete); +} diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp index 0d34b2df7..d2a91b880 100644 --- a/src/mbgl/map/tile_data.hpp +++ b/src/mbgl/map/tile_data.hpp @@ -44,10 +44,7 @@ public: TileData(const TileID&, const SourceInfo&); ~TileData(); - void request(Worker&, - float pixelRatio, - const std::function& successCallback, - const std::function& failureCallback); + void request(Worker&, float pixelRatio, const std::function& callback); // Schedule a tile reparse on a worker thread and call the callback on // completion. It will return true if the work was schedule or false it was @@ -71,13 +68,20 @@ public: // We let subclasses override setState() so they // can intercept the state change and react accordingly. virtual void setState(const State& state); - inline State getState() const { return state; } void endParsing(); + // Error message to be set in case of request + // and parsing errors. + void setError(const std::string& message); + + std::string getError() const { + return error; + } + // Override this in the child class. virtual void parse() = 0; virtual Bucket* getBucket(StyleLayer const &layer_desc) = 0; @@ -106,6 +110,8 @@ protected: private: std::atomic state; + std::string error; + protected: // Contains the tile ID string for painting debug information. DebugFontBuffer debugFontBuffer; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 4d77c8ef8..e464c55c9 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -59,9 +59,9 @@ void VectorTileData::parse() { setState(State::parsed); } } catch (const std::exception& ex) { - Log::Error(Event::ParseTile, "Parsing [%d/%d/%d] failed: %s", id.z, id.x, id.y, ex.what()); - setState(State::obsolete); - return; + std::stringstream message; + message << "Failed to parse [" << int(id.z) << "/" << id.x << "/" << id.y << "]: " << ex.what(); + setError(message.str()); } } diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp index c0642af29..899df39d3 100644 --- a/src/mbgl/text/glyph_pbf.cpp +++ b/src/mbgl/text/glyph_pbf.cpp @@ -23,14 +23,14 @@ GlyphPBF::GlyphPBF(const std::string& glyphURL, const GlyphLoadingFailedCallback& failureCallback) : parsed(false), env(env_) { // Load the glyph set URL - std::string url = util::replaceTokens(glyphURL, [&](const std::string &name) -> std::string { + url = util::replaceTokens(glyphURL, [&](const std::string &name) -> std::string { if (name == "fontstack") return util::percentEncode(fontStack); if (name == "range") return util::toString(glyphRange.first) + "-" + util::toString(glyphRange.second); return ""; }); // The prepare call jumps back to the main thread. - req = env.request({ Resource::Kind::Glyphs, url }, [&, url, successCallback, failureCallback](const Response &res) { + req = env.request({ Resource::Kind::Glyphs, url }, [&, successCallback, failureCallback](const Response &res) { req = nullptr; if (res.status != Response::Successful) { diff --git a/src/mbgl/text/glyph_pbf.hpp b/src/mbgl/text/glyph_pbf.hpp index cb36dde03..bb6fa83ae 100644 --- a/src/mbgl/text/glyph_pbf.hpp +++ b/src/mbgl/text/glyph_pbf.hpp @@ -29,6 +29,10 @@ public: void parse(FontStack &stack); bool isParsed() const; + std::string getURL() const { + return url; + } + private: GlyphPBF(const GlyphPBF &) = delete; GlyphPBF(GlyphPBF &&) = delete; @@ -36,6 +40,7 @@ private: GlyphPBF &operator=(GlyphPBF &&) = delete; std::string data; + std::string url; std::atomic parsed; Environment& env; diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp index ea451e278..9520b63c0 100644 --- a/src/mbgl/text/glyph_store.cpp +++ b/src/mbgl/text/glyph_store.cpp @@ -34,8 +34,14 @@ bool GlyphStore::requestGlyphRangesIfNeeded(const std::string& fontStackName, auto successCallback = [this, fontStackName](GlyphPBF* glyph) { auto fontStack = createFontStack(fontStackName); - glyph->parse(**fontStack); - asyncEmitGlyphRangeLoaded->send(); + try { + glyph->parse(**fontStack); + asyncEmitGlyphRangeLoaded->send(); + } catch (const std::exception&) { + std::lock_guard lock(errorMessageMutex); + errorMessage = "Failed to parse [" + glyph->getURL() + "]"; + asyncEmitGlyphRangeLoadedingFailed->send(); + } }; auto failureCallback = [this](const std::string& message) { diff --git a/test/resources/mock_file_source.cpp b/test/resources/mock_file_source.cpp index 0a9d448d4..42067a2a7 100644 --- a/test/resources/mock_file_source.cpp +++ b/test/resources/mock_file_source.cpp @@ -1,3 +1,4 @@ +#include "../fixtures/util.hpp" #include "mock_file_source.hpp" #include @@ -8,30 +9,64 @@ namespace mbgl { class MockFileSource::Impl { public: - Impl(uv_loop_t*, const std::string& matchFail) : matchFail_(matchFail) {} + Impl(uv_loop_t*, Type type, const std::string& match) : type_(type), match_(match) {} void handleRequest(Request* req) const; private: - std::string matchFail_; + void replyWithFailure(Response* res) const; + void replyWithCorruptedData(Response* res, const std::string& url) const; + void replyWithSuccess(Response* res, const std::string& url) const; + + Type type_; + std::string match_; }; -void MockFileSource::Impl::handleRequest(Request* req) const { - const Resource& resource = req->resource; +void MockFileSource::Impl::replyWithFailure(Response* res) const { + res->status = Response::Status::Error; + res->message = "Failed by the test case"; +} +void MockFileSource::Impl::replyWithCorruptedData(Response* res, const std::string& url) const { + res->status = Response::Status::Successful; + res->data = util::read_file(url); + res->data.insert(0, "CORRUPTED"); +} + +void MockFileSource::Impl::replyWithSuccess(Response* res, const std::string& url) const { + res->status = Response::Status::Successful; + res->data = util::read_file(url); +} + +void MockFileSource::Impl::handleRequest(Request* req) const { + const std::string& url = req->resource.url; std::shared_ptr response = std::make_shared(); - if (matchFail_.empty() || resource.url.find(matchFail_) == std::string::npos) { - response->status = Response::Status::Successful; - response->data = util::read_file(resource.url.c_str()); - } else { - response->message = "Failed by the test case"; + + if (url.find(match_) == std::string::npos) { + replyWithSuccess(response.get(), url); + req->notify(response); + return; + } + + switch (type_) { + case Type::Success: + replyWithSuccess(response.get(), url); + break; + case Type::RequestFail: + replyWithFailure(response.get()); + break; + case Type::RequestWithCorruptedData: + replyWithCorruptedData(response.get(), url); + break; + default: + EXPECT_TRUE(false) << "Should never be reached."; } req->notify(response); } -MockFileSource::MockFileSource(const std::string& matchFail) - : thread_(std::make_unique>("FileSource", util::ThreadPriority::Low, matchFail)) { +MockFileSource::MockFileSource(Type type, const std::string& match) + : thread_(std::make_unique>("FileSource", util::ThreadPriority::Low, type, match)) { } Request* MockFileSource::request(const Resource& resource, uv_loop_t* loop, Callback callback) { diff --git a/test/resources/mock_file_source.hpp b/test/resources/mock_file_source.hpp index 55ecd95c1..bb9fb55a3 100644 --- a/test/resources/mock_file_source.hpp +++ b/test/resources/mock_file_source.hpp @@ -13,12 +13,18 @@ template class Thread; } // This mock FileSource will read data from the disk and will fail -// the request if the URL matches the 'matchFail' string. +// the request if the URL matches a string. class MockFileSource : public FileSource { public: + enum Type { + Success, + RequestFail, + RequestWithCorruptedData + }; + class Impl; - MockFileSource(const std::string& matchFail); + MockFileSource(Type type, const std::string& match); ~MockFileSource() override = default; // FileSource implementation. diff --git a/test/resources/resource_loader.cpp b/test/resources/resource_loader.cpp index 83807156e..7d57f47ee 100644 --- a/test/resources/resource_loader.cpp +++ b/test/resources/resource_loader.cpp @@ -107,18 +107,13 @@ private: std::function callback_; }; -} - -class ResourceLoaderTest : public ::testing::TestWithParam { -}; - -TEST_P(ResourceLoaderTest, RequestFailure) { +void runTestCase(MockFileSource::Type type, + const std::string& param, + const std::string& message) { util::RunLoop loop(uv_default_loop()); - const std::string param(GetParam()); - MockView view; - MockFileSource fileSource(param); + MockFileSource fileSource(type, param); FixtureLogObserver* log = new FixtureLogObserver(); Log::setObserver(std::unique_ptr(log)); @@ -127,9 +122,6 @@ TEST_P(ResourceLoaderTest, RequestFailure) { try { if (error) { std::rethrow_exception(error); - } else { - // Control test, should succeed. - EXPECT_TRUE(param.empty()); } } catch (const util::GlyphRangeLoadingException&) { EXPECT_EQ(param, "glyphs.pbf"); @@ -156,24 +148,57 @@ TEST_P(ResourceLoaderTest, RequestFailure) { // join and cease logging after this point. context.reset(); - std::stringstream message; - message << "Failed to load [test/fixtures/resources/" << param << "]: Failed by the test case"; - const FixtureLogObserver::LogMessage logMessage { EventSeverity::Error, Event::ResourceLoader, int64_t(-1), - message.str(), + message, }; - if (param.empty()) { + if (type == MockFileSource::Success) { EXPECT_EQ(log->count(logMessage), 0u); } else { EXPECT_GT(log->count(logMessage), 0u); } - EXPECT_EQ(log->unchecked().size(), 0u); + // Clear the remaining error messages + log->unchecked().size(); +} + +} + +class ResourceLoaderTest : public ::testing::TestWithParam { +}; + +TEST_P(ResourceLoaderTest, Success) { + runTestCase(MockFileSource::Success, GetParam(), std::string()); +} + +TEST_P(ResourceLoaderTest, RequestFail) { + std::stringstream message; + message << "Failed to load [test/fixtures/resources/" << GetParam() << "]: Failed by the test case"; + + runTestCase(MockFileSource::RequestFail, GetParam(), message.str()); +} + +TEST_P(ResourceLoaderTest, RequestWithCorruptedData) { + const std::string param(GetParam()); + + std::stringstream message; + message << "Failed to parse "; + + if (param == "vector.pbf") { + message << "[15/16384/16384]: pbf unknown field type exception"; + } else { + message << "[test/fixtures/resources/" << param << "]"; + } + + if (param.find("json") != std::string::npos) { + message << ": 0 - Expect either an object or array at root"; + } + + runTestCase(MockFileSource::RequestWithCorruptedData, GetParam(), message.str()); } INSTANTIATE_TEST_CASE_P(ResourceLoader, ResourceLoaderTest, - ::testing::Values("", "source.json", "sprite.json", "sprite.png", "vector.pbf", "glyphs.pbf")); + ::testing::Values("source.json", "sprite.json", "sprite.png", "vector.pbf", "glyphs.pbf"));