fix Map#areTilesLoading from not returning true on final sourcedata event

[mollymerp]

Fixes #4975

When tiles are reloading they don't fire a sourcedata event when they finish loading unless debug collision boxes are enabled (https://github.com/mapbox/mapbox-gl-js/blob/master/src/source/tile.js#L232 🤔 )

this approach changes the areTilesLoaded check to accept reloading tiles as loaded, but an alternative approach could be removing the conditional in tile.js linked above so that a sourcedata event would be fired when all reloading tiles switch back to loaded.

cc @ksummerill

Launch Checklist

• briefly describe the changes in this PR
• write tests for all new functionality
• document any changes to public APIs
• post benchmark scores
• manually test the debug page

[anandthakker]

so that a sourcedata event would be fired when all reloading tiles switch back to loaded

I would lean towards this approach, because of the use case where someone does setData() and then wants to know when the new data is loaded/rendered.... but that's a pretty mysterious conditional 😬

[mollymerp]

yeah – I think that makes sense @anandthakker and it doesn't seem to break anything 🤞
finding this tough to test though 💭

[anandthakker]

@mollymerp On a second look, I'm thinking it's non-ideal to be firing this event via Tile#_immediateRedoPlacement. Do you know why this line in SourceCache doesn't already give us the sourcedata event we need?

[mollymerp]

@anandthakker it does fire the event, areTilesLoaded won't be true yet for tiles where tile.redoWhenDone=true because https://github.com/mapbox/mapbox-gl-js/blob/master/src/source/vector_tile_source.js#L135 sets the tile's state to reloading and kicks off the async reloading, so it's not loaded when the sourcedata event fires. We could potentially invoke _tileLoaded when a tile finishes reloading though 💭

[anandthakker]

Ohhhh, I see 🤦‍♂️ . In that case, I withdraw my previous comment -- since it's Tile that's responsible for invoking the redoPlacement action, it does make sense for it to fire the associated events.

[mollymerp]

ready for your 👓 ✅ @anandthakker !

[anandthakker]

LGTM!

One very small nitpick inline

[anandthakker]

nit: could we call this source rather than options?

[volkanunsal]

This is still happening.

[ryanhamley]

@volkanunsal if you're still seeing this happening, can you please open a new issue with a minimal reproduction of the problem so that we can better diagnose it? thanks!