Wait to start loading source assets until onAdd is called

[lbud]

Fixes #3735 by taking advantage of the onAdd method to load assets/fire events. Previously we were doing those in the source constructors, and they were constructed before establishing ownership (map -> style -> source_cache -> source), meaning events fired almost immediately wouldn't bubble up to the map object — so for example, vector tile sources (loaded over network connection) would reliably trigger map.on('source.load', fn) listeners, but canvas sources which loaded very quickly (from the same page) would not.

This isn't a perfect solution, but since sources will soon undergo a pretty big overhaul… 🔨

cc @lucaswoj

[lucaswoj]

In what cases did calling setEventedParent in the source constructors fail?

[lucaswoj]

Are the changes to this file significant?

[lbud]

@lucaswoj the way it has been working:

• Style creates instances of SourceCaches, then sets their parent: style.js#L376-L382
  • it does this because its eventedParentData is dependent on the Source having already been created (sourceCache.serialize() and sourceCache.loaded() both just forward to sourceCache._source methods)
  • this is where the race occurs: when sources fire events in their constructors before we've set up SourceCache's parent.
    • Specifically I was seeing this with the canvas source branch, since we load so quickly (from the same page).
    • However, at a glance I can see that other events like VectorTileSource#dataloading wouldn't always bubble up to Map for the same reason. Was just able to reproduce this with dynamically adding a vector tile set (it fires source.load because it's slow enough, but never dataloading)
• SourceCache creates instances of Sources, passing itself as a constructor so a source can be responsible for calling setEventedParent
  • We were unnecessarily setting this twice: in individual source constructors and also in Source.create https://github.com/mapbox/mapbox-gl-js/pull/3761/files#diff-d5506f072d1241401a7e31ff642c1606L24

Another minor, related change I made in this PR is to pass Style in the ImageSprite constructor, which looked like it could feasibly suffer the same race condition: https://github.com/mapbox/mapbox-gl-js/pull/3761/files#diff-6369a555a491e3e9a58fd29c3fd5a371L97

[lucaswoj]

Looks good! I appreciate that this is blocking #3765 and want to 🚢 to keep that project moving.

Some thoughts on future refactors: (I'll tix these out seperately)

• we could make eventedParentData a function to eliminate any circular dependency problems Make "eventedParentData" a function rather than a static object #3777
• we could pass map to Source cache as a constructor arg and move Source#onAdd logic into the Source constructor now that we don't support "orphaned" sources Eliminate Source#on{Add,Remove}(map) #3350

[lucaswoj]

@lbud Are you ready to merge this PR? I'd hate for a rebase conflict to emerge 😅

[lbud]

Oops yes @lucaswoj — I paused on this with all my naïve optimism about #3186 😬
🚢