Remove Loader.loader and replace Loader.getResponseHeader() with Laoder.getCacheAge()

[jwalton]

This PR removes Loader.loader replaces Loader.getResponseHeader() with Loader.getCacheAge().

Why is this Pull Request needed?

This is two small changes in one PR, but they're both related to the Loader API.

Loader.loader

First: The Loader interface has a property named loader of type any. This property is not read by any hls.js code. The two implementations of loader (xhr-loader and fetch-loader) set this to an XMLHttpRequest and a fetch Response, respectively.

I'm writing a loader that loads data from a local cache - I don't even have an HTTP request involved in my loader, so I have nothing to put here. But, since no one reads from it anyways, easiest just to remove it, and then I don't have to set it to undefined. The existing MockXhr loader is an example of a loader, used in fragment loading tests, is another example of a loader that never defines this.

I'm not sure why this property was here in the first place - was it maybe intended to make it so third parties could read from the response when they instantiate a loader? We could leave it public in fetch-loader and xhr-loader I suppose, but again I'm not sure what it's for.

Loader.getResponseHeader()

The second change: Loader has a function called getResponseHeader() (again, added in 1.0.0), which returns the value of a response header. It's used to find the "age" header to see if a request has been in a proxy for a while, when playing live.

But, this API is way too broad - first it assumes the underlying transport is HTTP based. If the underlying transport is something else, like WebRTC or websocket, what should getResponseHeader() return? Or what if (as in my above case) you're caching playlists locally? You could argue it should just return undefined for WebRTC and websocket, and maybe in my local cache case I should construct an "age" based on how long I've cached it, but making such an argument implies that getResponseHeader() will only ever be used to fetch the "age" header. But there's nothing about this API which restricts hls.js from calling it and asking for other headers.

So, as an implementer, you're left trying to work out what possible HTTP headers hls.js might want in this or future versions, and trying to reconstruct them from whatever data you have available to you from your transport. I replaced this with getCacheAge(), which is a much nicer API to try to implement as someone writing a non-HTTP loader.

Are there any points in the code the reviewer needs to double check?

No.

Resolves issues:

None.

Checklist

• changes have been done against master branch, and PR does not conflict
• new unit / functional tests have been added (whenever applicable)
• API or design changes are documented in API.md

[robwalch]

Ah, this would have been good to get in prior to v1.0.0, but maybe we can get it in v1.1. Consider that these are breaking changes for folks with custom loaders, and we may need to consider a little more backward compatibility with the older interface.

[jwalton]

Oh, I was an hour too late. -_-

[jwalton]

Well... we can still remove Loader.loader - hls.js doesn't call into it, and if existing implementations have the field, they can continue to have the field, and we can continue to not call it. :P

We could make getResponseHeader() an optional field, and then loaders can choose to implement it or not. It's still a strange API for a loader, though.

[jwalton]

Closed in favor of #3709 and #3711.

[robwalch]

Thanks @jwalton,

I appreciate the detailed description, rework, and breakdown on this.

Closing this and reviewing #3709 and #3711. Giving this more attention today than on the initial reaction/review.