Improve network layer abstractions

Dash-Industry-Forum · Apr 11, 2018 · 40a0dca · 40a0dca
1 parent 3eacade
commit 40a0dca
Show file tree

Hide file tree

Showing 11 changed files with 112 additions and 74 deletions.
diff --git a/build/typings/index.d.ts b/build/typings/index.d.ts
@@ -101,7 +101,7 @@ declare namespace dashjs {
         isMuted(): boolean;
         setVolume(value: number): void;
         getVolume(): number;
-        time(streamId: string): number;
+        time(streamId: string | undefined): number;
         duration(): number;
         timeAsUTC(): number;
         durationAsUTC(): number;
@@ -203,8 +203,6 @@ declare namespace dashjs {
         getJumpGaps(): boolean;
         setSmallGapLimit(value: number): void;
         getSmallGapLimit(): number;
-        setLowLatencyEnabled(value: boolean): void;
-        getLowLatencyEnabled(): boolean;
         preload(): void;
         reset(): void;
     }

diff --git a/src/dash/SegmentBaseLoader.js b/src/dash/SegmentBaseLoader.js
@@ -38,7 +38,7 @@ import FactoryMaker from '../core/FactoryMaker';
 import Debug from '../core/Debug';
 import {HTTPRequest} from '../streaming/vo/metrics/HTTPRequest';
 import FragmentRequest from '../streaming/vo/FragmentRequest';
-import HTTPLoader from '../streaming/transport/HTTPLoader';
+import HTTPLoader from '../streaming/net/HTTPLoader';
 
 function SegmentBaseLoader() {
 

diff --git a/src/dash/WebmSegmentBaseLoader.js b/src/dash/WebmSegmentBaseLoader.js
@@ -9,7 +9,7 @@ import {
     HTTPRequest
 } from '../streaming/vo/metrics/HTTPRequest';
 import FragmentRequest from '../streaming/vo/FragmentRequest';
-import HTTPLoader from '../streaming/transport/HTTPLoader';
+import HTTPLoader from '../streaming/net/HTTPLoader';
 
 function WebmSegmentBaseLoader() {
 

diff --git a/src/streaming/FragmentLoader.js b/src/streaming/FragmentLoader.js
@@ -28,7 +28,7 @@
  *  ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
  *  POSSIBILITY OF SUCH DAMAGE.
  */
-import HTTPLoader from './transport/HTTPLoader';
+import HTTPLoader from './net/HTTPLoader';
 import HeadRequest from './vo/HeadRequest';
 import DashJSError from './vo/DashJSError';
 import EventBus from './../core/EventBus';

diff --git a/src/streaming/ManifestLoader.js b/src/streaming/ManifestLoader.js
@@ -30,7 +30,7 @@
  */
 import Constants from './constants/Constants';
 import XlinkController from './controllers/XlinkController';
-import HTTPLoader from './transport/HTTPLoader';
+import HTTPLoader from './net/HTTPLoader';
 import URLUtils from './utils/URLUtils';
 import TextRequest from './vo/TextRequest';
 import DashJSError from './vo/DashJSError';

diff --git a/src/streaming/XlinkLoader.js b/src/streaming/XlinkLoader.js
@@ -29,7 +29,7 @@
  *  POSSIBILITY OF SUCH DAMAGE.
  */
 import DashJSError from './vo/DashJSError';
-import HTTPLoader from './transport/HTTPLoader';
+import HTTPLoader from './net/HTTPLoader';
 import {HTTPRequest} from './vo/metrics/HTTPRequest';
 import TextRequest from './vo/TextRequest';
 import EventBus from '../core/EventBus';

diff --git a/src/streaming/transport/FetchLoader.js → src/streaming/net/FetchLoader.js b/src/streaming/transport/FetchLoader.js → src/streaming/net/FetchLoader.js
@@ -43,7 +43,7 @@ function FetchLoader(cfg) {
 
     let instance;
 
-    function send(httpRequest) {
+    function load(httpRequest) {
 
         // Variables will be used in the callback functions
         let firstProgress = true; /*jshint ignore:line*/
@@ -75,17 +75,17 @@ function FetchLoader(cfg) {
             });
         }
 
-        let controller;
+        let abortController;
         if (typeof window.AbortController === 'function') {
-            controller = new AbortController(); /*jshint ignore:line*/
-            httpRequest.controller = controller;
+            abortController = new AbortController(); /*jshint ignore:line*/
+            httpRequest.abortController = abortController;
         }
 
         const reqOptions = {
             method: httpRequest.method,
             headers: headers,
             credentials: httpRequest.withCredentials ? 'include' : undefined,
-            signal: controller ? controller.signal : undefined
+            signal: abortController ? abortController.signal : undefined
         };
 
         fetch(httpRequest.url, reqOptions).then(function (response) {
@@ -97,7 +97,7 @@ function FetchLoader(cfg) {
             httpRequest.response.responseURL = response.url;
 
             if (!response.ok) {
-                httpRequest.onend();
+                httpRequest.onerror();
             }
 
             let responseHeaders = '';
@@ -163,8 +163,19 @@ function FetchLoader(cfg) {
         });
     }
 
+    function abort(request) {
+        if (request.abortController) {
+            // For firefox and edge
+            request.abortController.abort();
+        } else if (request.reader) {
+            // For Chrome
+            request.reader.cancel();
+        }
+    }
+
     instance = {
-        send: send
+        load: load,
+        abort: abort
     };
 
     return instance;

diff --git a/src/streaming/transport/HTTPLoader.js → src/streaming/net/HTTPLoader.js b/src/streaming/transport/HTTPLoader.js → src/streaming/net/HTTPLoader.js
@@ -222,15 +222,16 @@ function HTTPLoader(cfg) {
             onend: onloadend,
             onerror: onloadend,
             progress: progress,
-            onabort: onabort
+            onabort: onabort,
+            loader: loader
         };
 
         // Adds the ability to delay single fragment loading time to control buffer.
         let now = new Date().getTime();
         if (isNaN(request.delayLoadingTime) || now >= request.delayLoadingTime) {
             // no delay - just send
             requests.push(httpRequest);
-            httpRequest.response = loader.send(httpRequest);
+            loader.load(httpRequest);
         } else {
             // delay
             let delayedRequest = { httpRequest: httpRequest };
@@ -245,7 +246,7 @@ function HTTPLoader(cfg) {
                     requestStartTime = new Date();
                     lastTraceTime = requestStartTime;
                     requests.push(delayedRequest.httpRequest);
-                    httpRequest.response = loader.send(delayedRequest.httpRequest);
+                    loader.load(delayedRequest.httpRequest);
                 } catch (e) {
                     delayedRequest.httpRequest.onerror();
                 }
@@ -287,15 +288,7 @@ function HTTPLoader(cfg) {
             // when deliberately aborting inflight requests -
             // set them to undefined so they are not called
             x.onloadend = x.onerror = x.onprogress = undefined;
-            if (x.response && x.response.abort) {
-                x.response.abort();
-            } else if (x.controller) {
-                // For firefox and edge
-                x.controller.abort();
-            } else if (x.reader) {
-                // For Chrome
-                x.reader.cancel();
-            }
+            x.loader.abort(x);
             x.onabort();
         });
         requests = [];

diff --git a/src/streaming/transport/XHRLoader.js → src/streaming/net/XHRLoader.js b/src/streaming/transport/XHRLoader.js → src/streaming/net/XHRLoader.js
@@ -42,7 +42,7 @@ function XHRLoader(cfg) {
 
     let instance;
 
-    function send(httpRequest) {
+    function load(httpRequest) {
 
         // Variables will be used in the callback functions
         let firstProgress = true; /*jshint ignore:line*/
@@ -76,17 +76,22 @@ function XHRLoader(cfg) {
 
         xhr.onload = httpRequest.onload;
         xhr.onloadend = httpRequest.onend;
-        xhr.onerror = httpRequest.onend;
+        xhr.onerror = httpRequest.onerror;
         xhr.onprogress = httpRequest.progress;
         xhr.onabort = httpRequest.onabort;
 
         xhr.send();
 
-        return xhr;
+        httpRequest.response = xhr;
+    }
+
+    function abort(request) {
+        request.response.abort();
     }
 
     instance = {
-        send: send
+        load: load,
+        abort: abort
     };
 
     return instance;

diff --git a/test/unit/streaming.HTTPLoader.js → test/unit/streaming.net.HTTPLoader.js b/test/unit/streaming.HTTPLoader.js → test/unit/streaming.net.HTTPLoader.js
@@ -1,4 +1,4 @@
-import HTTPLoader from '../../src/streaming/transport/HTTPLoader';
+import HTTPLoader from '../../src/streaming/net/HTTPLoader';
 import RequestModifier from '../../src/streaming/utils/RequestModifier';
 import ErrorHandler from '../../src/streaming/utils/ErrorHandler';
 import MetricsModel from '../../src/streaming/models/MetricsModel';

diff --git a/test/unit/streaming.XHRLoader.js → test/unit/streaming.net.XHRLoader.js b/test/unit/streaming.XHRLoader.js → test/unit/streaming.net.XHRLoader.js
@@ -1,31 +1,21 @@
-import XHRLoader from '../../src/streaming/transport/XHRLoader';
+import XHRLoader from '../../src/streaming/net/XHRLoader';
 import RequestModifier from '../../src/streaming/utils/RequestModifier';
-import ErrorHandler from '../../src/streaming/utils/ErrorHandler';
-import MetricsModel from '../../src/streaming/models/MetricsModel';
-
-import MediaPlayerModelMock from './mocks/MediaPlayerModelMock';
 
 const expect = require('chai').expect;
 const sinon = require('sinon');
 
 const context = {};
 
-let errHandler;
-let metricsModel;
 let requestModifier;
-let mediaPlayerModelMock;
 let xhrLoader;
 
 describe('XHRLoader', function () {
 
     beforeEach(function () {
-        mediaPlayerModelMock = new MediaPlayerModelMock();
-        errHandler = ErrorHandler(context).getInstance();
-        metricsModel = MetricsModel(context).getInstance();
         requestModifier = RequestModifier(context).getInstance();
     });
-    beforeEach(function () {
 
+    beforeEach(function () {
         global.XMLHttpRequest = sinon.useFakeXMLHttpRequest();
 
         this.requests = [];
@@ -39,29 +29,28 @@ describe('XHRLoader', function () {
     });
 
     afterEach(function () {
-        mediaPlayerModelMock = null;
-    });
-
-    it('should throw an exception when attempting to call load and config parameter has not been set properly', () => {
-        xhrLoader = XHRLoader(context).create({mediaPlayerModel: mediaPlayerModelMock});
-        expect(xhrLoader.load.bind(xhrLoader, {request: {}})).to.throw('config object is not correct or missing');
     });
 
     it('should call success and complete callback when load is called successfully', () => {
-        let self = this.ctx;
+        const self = this.ctx;
         const callbackSucceeded = sinon.spy();
         const callbackCompleted = sinon.spy();
         const callbackError = sinon.spy();
         const callbackAbort = sinon.spy();
 
         xhrLoader = XHRLoader(context).create({
-            errHandler: errHandler,
-            metricsModel: metricsModel,
-            requestModifier: requestModifier,
-            mediaPlayerModel: mediaPlayerModelMock
+            requestModifier: requestModifier
         });
-
-        xhrLoader.load({request: {checkExistenceOnly: true}, success: callbackSucceeded, complete: callbackCompleted, error: callbackError, abort: callbackAbort});
+        const request = {
+            request: {
+                checkExistenceOnly: true
+            },
+            onload: callbackSucceeded,
+            onend: callbackCompleted,
+            onerror: callbackError,
+            onabort: callbackAbort
+        };
+        xhrLoader.load(request);
         expect(self.requests.length).to.equal(1);
         self.requests[0].respond(200);
         sinon.assert.notCalled(callbackError);
@@ -71,26 +60,32 @@ describe('XHRLoader', function () {
         expect(callbackSucceeded.calledBefore(callbackCompleted)).to.be.true; // jshint ignore:line
     });
 
-    it('should call error and complete callback when load is called with error', () => {
-        let self = this.ctx;
+    it('should call onload and complete callback when load is called and there is an error response', () => {
+        const self = this.ctx;
         const callbackSucceeded = sinon.spy();
         const callbackCompleted = sinon.spy();
         const callbackError = sinon.spy();
         const callbackAbort = sinon.spy();
         xhrLoader = XHRLoader(context).create({
-            errHandler: errHandler,
-            metricsModel: metricsModel,
-            requestModifier: requestModifier,
-            mediaPlayerModel: mediaPlayerModelMock
+            requestModifier: requestModifier
         });
-        xhrLoader.load({request: {checkExistenceOnly: true}, success: callbackSucceeded, complete: callbackCompleted, error: callbackError, abort: callbackAbort});
+        const request = {
+            request: {
+                checkExistenceOnly: true
+            },
+            onload: callbackSucceeded,
+            onend: callbackCompleted,
+            onerror: callbackError,
+            onabort: callbackAbort
+        };
+        xhrLoader.load(request);
         expect(self.requests.length).to.equal(1);
         self.requests[0].respond(404);
-        sinon.assert.calledOnce(callbackError);
+        sinon.assert.notCalled(callbackError);
         sinon.assert.calledOnce(callbackCompleted);
-        sinon.assert.notCalled(callbackSucceeded);
+        sinon.assert.calledOnce(callbackSucceeded);
         sinon.assert.notCalled(callbackAbort);
-        expect(callbackError.calledBefore(callbackCompleted)).to.be.true; // jshint ignore:line
+        expect(callbackSucceeded.calledBefore(callbackCompleted)).to.be.true; // jshint ignore:line
     });
 
     it('should call onabort callback when abort is called', () => {
@@ -100,17 +95,53 @@ describe('XHRLoader', function () {
         const callbackAbort = sinon.spy();
 
         xhrLoader = XHRLoader(context).create({
-            errHandler: errHandler,
-            metricsModel: metricsModel,
-            requestModifier: requestModifier,
-            mediaPlayerModel: mediaPlayerModelMock
+            requestModifier: requestModifier
         });
-        xhrLoader.load({request: {checkExistenceOnly: true}, success: callbackSucceeded, complete: callbackCompleted, error: callbackError, abort: callbackAbort});
-        xhrLoader.abort();
+
+        const request = {
+            request: {
+                checkExistenceOnly: true
+            },
+            onload: callbackSucceeded,
+            onend: callbackCompleted,
+            onerror: callbackError,
+            onabort: callbackAbort
+        };
+        xhrLoader.load(request);
+        xhrLoader.abort(request);
 
         sinon.assert.notCalled(callbackError);
-        sinon.assert.notCalled(callbackCompleted);
         sinon.assert.notCalled(callbackSucceeded);
+        // abort triggers both onloadend and onabort
+        sinon.assert.calledOnce(callbackCompleted);
         sinon.assert.calledOnce(callbackAbort);
     });
+
+    it('should call onerror and onend when load is called and there is a network error', () => {
+        const self = this.ctx;
+        const callbackSucceeded = sinon.spy();
+        const callbackCompleted = sinon.spy();
+        const callbackError = sinon.spy();
+        const callbackAbort = sinon.spy();
+        xhrLoader = XHRLoader(context).create({
+            requestModifier: requestModifier
+        });
+        const request = {
+            request: {
+                checkExistenceOnly: true
+            },
+            onload: callbackSucceeded,
+            onend: callbackCompleted,
+            onerror: callbackError,
+            onabort: callbackAbort
+        };
+        xhrLoader.load(request);
+        expect(self.requests.length).to.equal(1);
+        self.requests[0].error();
+        sinon.assert.calledOnce(callbackError);
+        sinon.assert.calledOnce(callbackCompleted);
+        sinon.assert.notCalled(callbackSucceeded);
+        sinon.assert.notCalled(callbackAbort);
+        expect(callbackError.calledBefore(callbackCompleted)).to.be.true; // jshint ignore:line
+    });
 });