From 8895d94408c4b9cada12ba3d837916133258605b Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Mon, 5 Jun 2017 16:28:17 -0700 Subject: [PATCH 1/2] Use Renderer command queue to render outstream --- src/Renderer.js | 35 +++++++++++++++++++++++++++++++---- src/adapters/appnexusAst.js | 26 ++++++++++++++++---------- test/spec/renderer_spec.js | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 14 deletions(-) diff --git a/src/Renderer.js b/src/Renderer.js index c4e926ca4d5..77722c4a758 100644 --- a/src/Renderer.js +++ b/src/Renderer.js @@ -2,19 +2,30 @@ import { loadScript } from 'src/adloader'; import * as utils from 'src/utils'; export function Renderer(options) { - const { url, config, id, callback } = options; + const { url, config, id, callback, loaded } = options; this.url = url; this.config = config; - this.callback = callback; this.handlers = {}; this.id = id; + // a renderer may use the following properties with Renderer.prototype.process + // to delay rendering until the render function is loaded + this.loaded = loaded; + this.cmd = []; + this.push = func => { + if (typeof func !== 'function') { + utils.logError('Commands given to Renderer.push must be wrapped in a function'); + return; + } + this.loaded ? func.call() : this.cmd.push(func); + }; + // we expect to load a renderer url once only so cache the request to load script loadScript(url, callback, true); } -Renderer.install = function({ url, config, id, callback }) { - return new Renderer({ url, config, id, callback }); +Renderer.install = function({ url, config, id, callback, loaded }) { + return new Renderer({ url, config, id, callback, loaded }); }; Renderer.prototype.getConfig = function() { @@ -36,3 +47,19 @@ Renderer.prototype.handleVideoEvent = function({ id, eventName }) { utils.logMessage(`Prebid Renderer event for id ${id} type ${eventName}`); }; + +/* + * If a Renderer is not loaded at the time renderer.render(bid) is called, add + * the render function to the command queue with render.push(() => renderFunc) + * then create a renderer callback that sets renderer.loaded to true and call + * this function as renderer.process() to begin rendering + */ +Renderer.prototype.process = function() { + while (this.cmd.length > 0) { + try { + this.cmd.shift().call(); + } catch (error) { + utils.logError('Error processing Renderer command: ', error); + } + } +}; diff --git a/src/adapters/appnexusAst.js b/src/adapters/appnexusAst.js index a3bd0e72441..f9b02efced4 100644 --- a/src/adapters/appnexusAst.js +++ b/src/adapters/appnexusAst.js @@ -269,18 +269,23 @@ function AppnexusAstAdapter() { } function outstreamRender(bid) { - window.ANOutstreamVideo.renderAd({ - tagId: bid.adResponse.tag_id, - sizes: [bid.getSize().split('x')], - targetId: bid.adUnitCode, // target div id to render video - uuid: bid.adResponse.uuid, - adResponse: bid.adResponse, - rendererOptions: bid.renderer.getConfig() - }, handleOutstreamRendererEvents.bind(bid)); + // push to render queue because ANOutstreamVideo may not be loaded yet + bid.renderer.push(() => { + window.ANOutstreamVideo.renderAd({ + tagId: bid.adResponse.tag_id, + sizes: [bid.getSize().split('x')], + targetId: bid.adUnitCode, // target div id to render video + uuid: bid.adResponse.uuid, + adResponse: bid.adResponse, + rendererOptions: bid.renderer.getConfig() + }, handleOutstreamRendererEvents.bind(bid)); + }); } - function onOutstreamRendererLoaded() { - // setup code for renderer, if any + function onOutstreamRendererLoaded(bid) { + // adload.loadScript has loaded ANOutstreamVideo + bid.renderer.loaded = true; + bid.renderer.process(); } function handleOutstreamRendererEvents(id, eventName) { @@ -313,6 +318,7 @@ function AppnexusAstAdapter() { id: ad.renderer_id, url: ad.renderer_url, config: { adText: `AppNexus Outstream Video Ad via Prebid.js` }, + loaded: false, callback: () => onOutstreamRendererLoaded.call(null, bid) }); diff --git a/test/spec/renderer_spec.js b/test/spec/renderer_spec.js index ba60923ed2a..282c4841ac0 100644 --- a/test/spec/renderer_spec.js +++ b/test/spec/renderer_spec.js @@ -53,4 +53,39 @@ describe('Renderer: A renderer installed on a bid response', () => { testRenderer1.handleVideoEvent({ id: 1, eventName: 'testEvent' }); expect(spyEventHandler.called).to.equal(true); }); + + it('pushes commands to queue if renderer is not loaded', () => { + testRenderer1.push(spyRenderFn); + + expect(testRenderer1.cmd.length).to.equal(1); + + // clear queue for next tests + testRenderer1.cmd = []; + }); + + it('fires commands immediately if the renderer is loaded', () => { + const func = sinon.spy(); + + testRenderer1.loaded = true; + testRenderer1.push(func); + + expect(testRenderer1.cmd.length).to.equal(0); + sinon.assert.calledOnce(func); + }); + + it('processes queue by calling each function in queue', () => { + testRenderer1.loaded = false; + const func1 = sinon.spy(); + const func2 = sinon.spy(); + + testRenderer1.push(func1); + testRenderer1.push(func2); + expect(testRenderer1.cmd.length).to.equal(2); + + testRenderer1.process(); + + sinon.assert.calledOnce(func1); + sinon.assert.calledOnce(func2); + expect(testRenderer1.cmd.length).to.equal(0); + }); }); From 406b287675f338e91c4017de412583bba342fe8f Mon Sep 17 00:00:00 2001 From: Matt Lane Date: Tue, 13 Jun 2017 16:20:50 -0700 Subject: [PATCH 2/2] Abstract automatic processing in renderer --- src/Renderer.js | 19 ++++++++++++------- src/adapters/appnexusAst.js | 8 -------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/Renderer.js b/src/Renderer.js index 77722c4a758..c81049fa1f6 100644 --- a/src/Renderer.js +++ b/src/Renderer.js @@ -8,8 +8,9 @@ export function Renderer(options) { this.handlers = {}; this.id = id; - // a renderer may use the following properties with Renderer.prototype.process - // to delay rendering until the render function is loaded + // a renderer may push to the command queue to delay rendering until the + // render function is loaded by loadScript, at which point the the command + // queue will be processed this.loaded = loaded; this.cmd = []; this.push = func => { @@ -20,8 +21,14 @@ export function Renderer(options) { this.loaded ? func.call() : this.cmd.push(func); }; + // bidders may override this with the `callback` property given to `install` + this.callback = callback || (() => { + this.loaded = true; + this.process(); + }); + // we expect to load a renderer url once only so cache the request to load script - loadScript(url, callback, true); + loadScript(url, this.callback, true); } Renderer.install = function({ url, config, id, callback, loaded }) { @@ -49,10 +56,8 @@ Renderer.prototype.handleVideoEvent = function({ id, eventName }) { }; /* - * If a Renderer is not loaded at the time renderer.render(bid) is called, add - * the render function to the command queue with render.push(() => renderFunc) - * then create a renderer callback that sets renderer.loaded to true and call - * this function as renderer.process() to begin rendering + * Calls functions that were pushed to the command queue before the + * renderer was loaded by `loadScript` */ Renderer.prototype.process = function() { while (this.cmd.length > 0) { diff --git a/src/adapters/appnexusAst.js b/src/adapters/appnexusAst.js index f9b02efced4..4ad2fe4502c 100644 --- a/src/adapters/appnexusAst.js +++ b/src/adapters/appnexusAst.js @@ -282,12 +282,6 @@ function AppnexusAstAdapter() { }); } - function onOutstreamRendererLoaded(bid) { - // adload.loadScript has loaded ANOutstreamVideo - bid.renderer.loaded = true; - bid.renderer.process(); - } - function handleOutstreamRendererEvents(id, eventName) { const bid = this; bid.renderer.handleVideoEvent({ id, eventName }); @@ -319,9 +313,7 @@ function AppnexusAstAdapter() { url: ad.renderer_url, config: { adText: `AppNexus Outstream Video Ad via Prebid.js` }, loaded: false, - callback: () => onOutstreamRendererLoaded.call(null, bid) }); - try { bid.renderer.setRender(outstreamRender); } catch (err) {