From c5b721b21cb4c0fb13980e75f8005178ccf2841c Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Fri, 8 Jan 2016 08:33:39 -0800 Subject: [PATCH] Web Worker improvements and fixes - Do not clean up the documentElement in enableProllyfill if it is currently being rendered. - Moved Worker event handlers to before postMessage is called. --- dist/diffhtml.js | 34 ++++++++++++++++++++++++---------- lib/index.js | 7 ++++++- lib/node/patch.js | 13 +++++++------ lib/node/release.js | 4 ++++ lib/patches/process.js | 8 ++++++-- lib/worker/source.js | 2 +- package.json | 2 +- 7 files changed, 49 insertions(+), 21 deletions(-) diff --git a/dist/diffhtml.js b/dist/diffhtml.js index 247edfab..d18735d2 100644 --- a/dist/diffhtml.js +++ b/dist/diffhtml.js @@ -265,6 +265,8 @@ var _transitions = _dereq_('./transitions'); var _custom = _dereq_('./element/custom'); +var _tree = _dereq_('./node/tree'); + function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } /** @@ -550,8 +552,12 @@ function enableProllyfill() { // After the initial render, clean up the resources, no point in lingering. documentElement.addEventListener('renderComplete', function render() { + var elementMeta = _tree.TreeCache.get(documentElement) || {}; + // Release resources allocated to the element. - documentElement.diffRelease(documentElement); + if (!elementMeta.isRendering) { + documentElement.diffRelease(documentElement); + } // Remove this event listener. documentElement.removeEventListener('renderComplete', render); @@ -574,7 +580,7 @@ function enableProllyfill() { } } -},{"./element/custom":1,"./errors":4,"./node/patch":7,"./node/release":8,"./transitions":13}],6:[function(_dereq_,module,exports){ +},{"./element/custom":1,"./errors":4,"./node/patch":7,"./node/release":8,"./node/tree":10,"./transitions":13}],6:[function(_dereq_,module,exports){ 'use strict'; Object.defineProperty(exports, "__esModule", { @@ -858,13 +864,13 @@ function patchNode(element, newHTML, options) { if (typeof newHTML !== 'string') { transferObject.newTree = (0, _make2.default)(newHTML); + // Wait for the worker to finish processing and then apply the patchset. + worker.onmessage = (0, _render.completeWorkerRender)(element, elementMeta); + // Transfer this buffer to the worker, which will take over and process the // markup. worker.postMessage(transferObject); - // Wait for the worker to finish processing and then apply the patchset. - worker.onmessage = (0, _render.completeWorkerRender)(element, elementMeta); - return; } @@ -875,12 +881,12 @@ function patchNode(element, newHTML, options) { // Add properties to send to worker. transferObject.isInner = options.inner; + // Wait for the worker to finish processing and then apply the patchset. + worker.onmessage = (0, _render.completeWorkerRender)(element, elementMeta); + // Transfer this buffer to the worker, which will take over and process the // markup. worker.postMessage(transferObject); - - // Wait for the worker to finish processing and then apply the patchset. - worker.onmessage = (0, _render.completeWorkerRender)(element, elementMeta); } else { // We're rendering in the UI thread. elementMeta.isRendering = true; @@ -951,10 +957,14 @@ function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { de function releaseNode(element) { var elementMeta = _tree.TreeCache.get(element); + // Do not remove an element that is in process of rendering. User intentions + // come first, so if we are cleaning up an element being used by another part + // of the code, keep it alive. if (elementMeta) { // If there is a worker associated with this element, then kill it. if (elementMeta.worker) { elementMeta.worker.terminate(); + delete elementMeta.worker; } // If there was a tree set up, recycle the memory allocated for it. @@ -1488,7 +1498,11 @@ function process(element, patches) { } // Append the element first, before doing the replacement. - patch.old.parentNode.insertBefore(patch.new, patch.old.nextSibling); + if (patch.old.nextSibling) { + patch.old.parentNode.insertBefore(patch.new, patch.old.nextSibling); + } else { + patch.old.parentNode.appendChild(patch.new); + } // Removed state for transitions API. var allPromises = []; @@ -2623,7 +2637,7 @@ function startup(worker) { syncNode(oldTree, newTree, patches); // Protect the current `oldTree` so that no Nodes will be accidentally - // recycled in the + // recycled in the cleanup process. protectElement(oldTree); // Send the patches back to the userland. diff --git a/lib/index.js b/lib/index.js index ef1027a4..b2cdd032 100644 --- a/lib/index.js +++ b/lib/index.js @@ -2,6 +2,7 @@ import patchNode from './node/patch'; import releaseNode from './node/release'; import { states } from './transitions'; import { components } from './element/custom'; +import { TreeCache } from './node/tree'; import { TransitionStateError, DOMException } from './errors'; // Export the custom Error constructors so that instanceof checks can be made @@ -288,8 +289,12 @@ export function enableProllyfill() { // After the initial render, clean up the resources, no point in lingering. documentElement.addEventListener('renderComplete', function render() { + var elementMeta = TreeCache.get(documentElement) || {}; + // Release resources allocated to the element. - documentElement.diffRelease(documentElement); + if (!elementMeta.isRendering) { + documentElement.diffRelease(documentElement); + } // Remove this event listener. documentElement.removeEventListener('renderComplete', render); diff --git a/lib/node/patch.js b/lib/node/patch.js index d88502e8..d02ee16f 100644 --- a/lib/node/patch.js +++ b/lib/node/patch.js @@ -71,6 +71,7 @@ export default function patchNode(element, newHTML, options) { return elementMeta.renderBuffer; } + // If the operation is `innerHTML`, but the contents haven't changed. var sameInnerHTML = options.inner && element.innerHTML === newHTML; // If the operation is `outerHTML`, but the contents haven't changed. @@ -138,13 +139,13 @@ export default function patchNode(element, newHTML, options) { if (typeof newHTML !== 'string') { transferObject.newTree = makeNode(newHTML); + // Wait for the worker to finish processing and then apply the patchset. + worker.onmessage = completeWorkerRender(element, elementMeta); + // Transfer this buffer to the worker, which will take over and process the // markup. worker.postMessage(transferObject); - // Wait for the worker to finish processing and then apply the patchset. - worker.onmessage = completeWorkerRender(element, elementMeta); - return; } @@ -155,12 +156,12 @@ export default function patchNode(element, newHTML, options) { // Add properties to send to worker. transferObject.isInner = options.inner; + // Wait for the worker to finish processing and then apply the patchset. + worker.onmessage = completeWorkerRender(element, elementMeta); + // Transfer this buffer to the worker, which will take over and process the // markup. worker.postMessage(transferObject); - - // Wait for the worker to finish processing and then apply the patchset. - worker.onmessage = completeWorkerRender(element, elementMeta); } else { // We're rendering in the UI thread. diff --git a/lib/node/release.js b/lib/node/release.js index db4049da..0fd644cb 100644 --- a/lib/node/release.js +++ b/lib/node/release.js @@ -10,10 +10,14 @@ import makeNode from './make'; export default function releaseNode(element) { let elementMeta = TreeCache.get(element); + // Do not remove an element that is in process of rendering. User intentions + // come first, so if we are cleaning up an element being used by another part + // of the code, keep it alive. if (elementMeta) { // If there is a worker associated with this element, then kill it. if (elementMeta.worker) { elementMeta.worker.terminate(); + delete elementMeta.worker; } // If there was a tree set up, recycle the memory allocated for it. diff --git a/lib/patches/process.js b/lib/patches/process.js index f583a7c8..08e9ecbc 100644 --- a/lib/patches/process.js +++ b/lib/patches/process.js @@ -185,9 +185,13 @@ export default function process(element, patches) { 'document root?'); } - // Append the element first, before doing the replacement. - patch.old.parentNode.insertBefore(patch.new, patch.old.nextSibling); + if (patch.old.nextSibling) { + patch.old.parentNode.insertBefore(patch.new, patch.old.nextSibling); + } + else { + patch.old.parentNode.appendChild(patch.new); + } // Removed state for transitions API. let allPromises = []; diff --git a/lib/worker/source.js b/lib/worker/source.js index 5729bbe4..c795d065 100644 --- a/lib/worker/source.js +++ b/lib/worker/source.js @@ -73,7 +73,7 @@ export default function startup(worker) { syncNode(oldTree, newTree, patches); // Protect the current `oldTree` so that no Nodes will be accidentally - // recycled in the + // recycled in the cleanup process. protectElement(oldTree); // Send the patches back to the userland. diff --git a/package.json b/package.json index ab9b9752..6b46acb5 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "scripts": { "start": "http-server", "build": "browserify -t [ babelify --presets [ es2015 ] ] -s diff lib/index.js | derequire > dist/diffhtml.js", - "watch": "watchify -t [ babelify --presets [ es2015 ] ] -s diff lib/index.js -o dist/diffhtml.js -v", + "watch": "watchify -t [ babelify --presets [ es2015 ] ] -s diff lib/index.js -o 'derequire > dist/diffhtml.js' -v", "jshint": "jshint lib/**/*.js", "test": "npm run jshint && karma start --single-run" },