From 480110625ab1e9b256a4e677b6a69d046f4154ac Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 13 Oct 2018 13:25:39 +0200 Subject: [PATCH] Try to, completely, avoid loading the `ReadableStream` polyfill in MOZCENTRAL builds With https://bugzilla.mozilla.org/show_bug.cgi?id=1505122 landing in Firefox 65, the native `ReadableStream` implementation is now enabled by default in Firefox. Obviously it would be nice to simply stop bundling the polyfill in MOZCENTRAL builds altogether, however given that it's still possible to disable[1] `ReadableStream` this is probably not a good idea just yet. Nonetheless, now that native support is available, it seems unnecessary (and wasteful) to keep bundling the polyfill twice[2] in MOZCENTRAL builds. Hence this patch, which contains a suggest approach for packing the polyfill in a *separate* file which is then *only* loaded if/when needed. With this patch, the size of the `gulp mozcentral` build target is thus reduced accordingly: | | `build/mozcentral` |-------|------------------- |master | 3 461 089 |patch | 3 340 268 Besides the PDF.js files taking up less space in Firefox this way, the additional benefit is that there's (by default) less code that needs to be loaded and parsed when the PDF Viewer is used which also cannot hurt. --- [1] In `about:config`, by toggling the `javascript.options.streams` preference. [2] Once in the `build/pdf.js` file, and once in the `build/pdf.worker.js` file. --- .../firefox/content/streams_polyfill.js | 19 +++++++ gulpfile.js | 12 +++++ src/pdf.worker.js | 12 +++-- src/shared/streams_polyfill.js | 41 +++++++++------- src/shared/url_polyfill.js | 49 +++++++++---------- web/viewer-snippet-firefox-extension.html | 7 +++ 6 files changed, 94 insertions(+), 46 deletions(-) create mode 100644 extensions/firefox/content/streams_polyfill.js diff --git a/extensions/firefox/content/streams_polyfill.js b/extensions/firefox/content/streams_polyfill.js new file mode 100644 index 000000000..73ef7c75d --- /dev/null +++ b/extensions/firefox/content/streams_polyfill.js @@ -0,0 +1,19 @@ +/* Copyright 2018 Mozilla Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +if (typeof ReadableStream === "undefined") { + require("../../../src/shared/global_scope").ReadableStream = + require("../../../external/streams/streams-lib").ReadableStream; +} diff --git a/gulpfile.js b/gulpfile.js index 685a4d575..6604ef23b 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -766,6 +766,16 @@ function preprocessDefaultPreferences(content) { content + '\n'); } +function createMozcentralStreamsPolyfillBundle(defines) { + var streamsPolyfillOutputName = 'streams_polyfill.js'; + + var streamsPolyfillFileConfig = createWebpackConfig(defines, { + filename: streamsPolyfillOutputName, + }); + return gulp.src('./extensions/firefox/content/streams_polyfill.js') + .pipe(webpack2Stream(streamsPolyfillFileConfig)); +} + gulp.task('mozcentral-pre', gulp.series('buildnumber', 'locale', function () { console.log(); console.log('### Building mozilla-central extension'); @@ -787,6 +797,8 @@ gulp.task('mozcentral-pre', gulp.series('buildnumber', 'locale', function () { return merge([ createBundle(defines).pipe(gulp.dest(MOZCENTRAL_CONTENT_DIR + 'build')), createWebBundle(defines).pipe(gulp.dest(MOZCENTRAL_CONTENT_DIR + 'web')), + createMozcentralStreamsPolyfillBundle(defines) + .pipe(gulp.dest(MOZCENTRAL_CONTENT_DIR + 'build')), gulp.src(COMMON_WEB_FILES, { base: 'web/', }) .pipe(gulp.dest(MOZCENTRAL_CONTENT_DIR + 'web')), gulp.src(['external/bcmaps/*.bcmap', 'external/bcmaps/LICENSE'], diff --git a/src/pdf.worker.js b/src/pdf.worker.js index 7306f17d2..afce1daeb 100644 --- a/src/pdf.worker.js +++ b/src/pdf.worker.js @@ -12,13 +12,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -/* eslint-disable no-unused-vars */ +/* eslint-disable no-restricted-globals, no-unused-vars */ 'use strict'; -var pdfjsVersion = PDFJSDev.eval('BUNDLE_VERSION'); -var pdfjsBuild = PDFJSDev.eval('BUNDLE_BUILD'); +if (PDFJSDev.test('MOZCENTRAL') && typeof ReadableStream === 'undefined') { + importScripts('./streams_polyfill.js'); +} -var pdfjsCoreWorker = require('./core/worker.js'); +const pdfjsVersion = PDFJSDev.eval('BUNDLE_VERSION'); +const pdfjsBuild = PDFJSDev.eval('BUNDLE_BUILD'); + +const pdfjsCoreWorker = require('./core/worker.js'); exports.WorkerMessageHandler = pdfjsCoreWorker.WorkerMessageHandler; diff --git a/src/shared/streams_polyfill.js b/src/shared/streams_polyfill.js index 9dbaaf5c8..fbe70c092 100644 --- a/src/shared/streams_polyfill.js +++ b/src/shared/streams_polyfill.js @@ -14,25 +14,32 @@ */ /* eslint-disable no-restricted-globals */ -let isReadableStreamSupported = false; -if (typeof ReadableStream !== 'undefined') { - // MS Edge may say it has ReadableStream but they are not up to spec yet. - try { - // eslint-disable-next-line no-new - new ReadableStream({ - start(controller) { - controller.close(); - }, - }); - isReadableStreamSupported = true; - } catch (e) { - // The ReadableStream constructor cannot be used. - } -} -if (isReadableStreamSupported) { +if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('MOZCENTRAL')) { + // On the main-thread the `streams_polyfill.js` file is loaded using a + //