[Firefox regression] Fix disableRange=true bug in PDFDataTransportStream

Currently if trying to set `disableRange=true` in the built-in PDF Viewer in Firefox, either through `about:config` or via the URL hash, the PDF document will never load. It appears that this has been broken for a couple of years, without anyone noticing.

Obviously it's not a good idea to set `disableRange=true`, however it seems that this bug affects the PDF Viewer in Firefox even with default settings:
 - In the case where `initialData` already contains the *entire* file, we're forced to dispatch a range request to re-fetch already available data just so that file loading may complete.
 - (In the case where the data arrives, via streaming, before being specifically requested through `requestDataRange`, we're also forced to re-fetch data unnecessarily.) *This part was removed, to reduce the scope/risk of the patch somewhat.*

In the cases outlined above, we're having to re-fetch already available data thus potentially delaying loading/rendering of PDF files in Firefox (and wasting resources in the process).
This commit is contained in:
Jonas Jenwald 2019-03-26 16:05:30 +01:00
parent 9b5a937f78
commit bb384dd5ed
4 changed files with 74 additions and 6 deletions

View File

@ -344,6 +344,7 @@ function getDocument(src) {
networkStream = new PDFDataTransportStream({ networkStream = new PDFDataTransportStream({
length: params.length, length: params.length,
initialData: params.initialData, initialData: params.initialData,
progressiveDone: params.progressiveDone,
disableRange: params.disableRange, disableRange: params.disableRange,
disableStream: params.disableStream, disableStream: params.disableStream,
}, rangeTransport); }, rangeTransport);
@ -389,6 +390,7 @@ function _fetchDocument(worker, source, pdfDataRangeTransport, docId) {
if (pdfDataRangeTransport) { if (pdfDataRangeTransport) {
source.length = pdfDataRangeTransport.length; source.length = pdfDataRangeTransport.length;
source.initialData = pdfDataRangeTransport.initialData; source.initialData = pdfDataRangeTransport.initialData;
source.progressiveDone = pdfDataRangeTransport.progressiveDone;
} }
return worker.messageHandler.sendWithPromise('GetDocRequest', { return worker.messageHandler.sendWithPromise('GetDocRequest', {
docId, docId,
@ -515,13 +517,15 @@ const PDFDocumentLoadingTask = (function PDFDocumentLoadingTaskClosure() {
* @param {Uint8Array} initialData * @param {Uint8Array} initialData
*/ */
class PDFDataRangeTransport { class PDFDataRangeTransport {
constructor(length, initialData) { constructor(length, initialData, progressiveDone = false) {
this.length = length; this.length = length;
this.initialData = initialData; this.initialData = initialData;
this.progressiveDone = progressiveDone;
this._rangeListeners = []; this._rangeListeners = [];
this._progressListeners = []; this._progressListeners = [];
this._progressiveReadListeners = []; this._progressiveReadListeners = [];
this._progressiveDoneListeners = [];
this._readyCapability = createPromiseCapability(); this._readyCapability = createPromiseCapability();
} }
@ -537,6 +541,10 @@ class PDFDataRangeTransport {
this._progressiveReadListeners.push(listener); this._progressiveReadListeners.push(listener);
} }
addProgressiveDoneListener(listener) {
this._progressiveDoneListeners.push(listener);
}
onDataRange(begin, chunk) { onDataRange(begin, chunk) {
for (const listener of this._rangeListeners) { for (const listener of this._rangeListeners) {
listener(begin, chunk); listener(begin, chunk);
@ -559,6 +567,14 @@ class PDFDataRangeTransport {
}); });
} }
onDataProgressiveDone() {
this._readyCapability.promise.then(() => {
for (const listener of this._progressiveDoneListeners) {
listener();
}
});
}
transportReady() { transportReady() {
this._readyCapability.resolve(); this._readyCapability.resolve();
} }

View File

@ -21,7 +21,9 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
assert(pdfDataRangeTransport); assert(pdfDataRangeTransport);
this._queuedChunks = []; this._queuedChunks = [];
var initialData = params.initialData; this._progressiveDone = params.progressiveDone || false;
const initialData = params.initialData;
if (initialData && initialData.length > 0) { if (initialData && initialData.length > 0) {
let buffer = new Uint8Array(initialData).buffer; let buffer = new Uint8Array(initialData).buffer;
this._queuedChunks.push(buffer); this._queuedChunks.push(buffer);
@ -47,6 +49,10 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
this._onReceiveData({ chunk, }); this._onReceiveData({ chunk, });
}); });
this._pdfDataRangeTransport.addProgressiveDoneListener(() => {
this._onProgressiveDone();
});
this._pdfDataRangeTransport.transportReady(); this._pdfDataRangeTransport.transportReady();
} }
PDFDataTransportStream.prototype = { PDFDataTransportStream.prototype = {
@ -80,6 +86,13 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
} }
}, },
_onProgressiveDone() {
if (this._fullRequestReader) {
this._fullRequestReader.progressiveDone();
}
this._progressiveDone = true;
},
_removeRangeReader: _removeRangeReader:
function PDFDataTransportStream_removeRangeReader(reader) { function PDFDataTransportStream_removeRangeReader(reader) {
var i = this._rangeReaders.indexOf(reader); var i = this._rangeReaders.indexOf(reader);
@ -92,7 +105,8 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
assert(!this._fullRequestReader); assert(!this._fullRequestReader);
var queuedChunks = this._queuedChunks; var queuedChunks = this._queuedChunks;
this._queuedChunks = null; this._queuedChunks = null;
return new PDFDataTransportStreamReader(this, queuedChunks); return new PDFDataTransportStreamReader(this, queuedChunks,
this._progressiveDone);
}, },
getRangeReader: function PDFDataTransportStream_getRangeReader(begin, end) { getRangeReader: function PDFDataTransportStream_getRangeReader(begin, end) {
@ -116,9 +130,10 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
}; };
/** @implements {IPDFStreamReader} */ /** @implements {IPDFStreamReader} */
function PDFDataTransportStreamReader(stream, queuedChunks) { function PDFDataTransportStreamReader(stream, queuedChunks,
progressiveDone = false) {
this._stream = stream; this._stream = stream;
this._done = false; this._done = progressiveDone || false;
this._filename = null; this._filename = null;
this._queuedChunks = queuedChunks || []; this._queuedChunks = queuedChunks || [];
this._requests = []; this._requests = [];
@ -180,6 +195,13 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
}); });
this._requests = []; this._requests = [];
}, },
progressiveDone() {
if (this._done) {
return;
}
this._done = true;
},
}; };
/** @implements {IPDFStreamRangeReader} */ /** @implements {IPDFStreamRangeReader} */

View File

@ -1526,5 +1526,30 @@ describe('api', function() {
}); });
}).catch(done.fail); }).catch(done.fail);
}); });
it('should fetch document info and page, without range, ' +
'using complete initialData', function(done) {
let fetches = 0, loadingTask;
dataPromise.then(function(data) {
const transport =
new PDFDataRangeTransport(data.length, data,
/* progressiveDone = */ true);
transport.requestDataRange = function(begin, end) {
fetches++;
};
loadingTask = getDocument({ disableRange: true, range: transport, });
return loadingTask.promise;
}).then(function(pdfDocument) {
expect(pdfDocument.numPages).toEqual(14);
return pdfDocument.getPage(10);
}).then(function(pdfPage) {
expect(pdfPage.rotate).toEqual(0);
expect(fetches).toEqual(0);
loadingTask.destroy().then(done);
}).catch(done.fail);
});
}); });
}); });

View File

@ -256,7 +256,7 @@ PDFViewerApplication.externalServices = {
switch (args.pdfjsLoadAction) { switch (args.pdfjsLoadAction) {
case 'supportsRangedLoading': case 'supportsRangedLoading':
pdfDataRangeTransport = pdfDataRangeTransport =
new FirefoxComDataRangeTransport(args.length, args.data); new FirefoxComDataRangeTransport(args.length, args.data, args.done);
callbacks.onOpenWithTransport(args.pdfUrl, args.length, callbacks.onOpenWithTransport(args.pdfUrl, args.length,
pdfDataRangeTransport); pdfDataRangeTransport);
@ -270,6 +270,11 @@ PDFViewerApplication.externalServices = {
case 'progressiveRead': case 'progressiveRead':
pdfDataRangeTransport.onDataProgressiveRead(args.chunk); pdfDataRangeTransport.onDataProgressiveRead(args.chunk);
break; break;
case 'progressiveDone':
if (pdfDataRangeTransport) {
pdfDataRangeTransport.onDataProgressiveDone();
}
break;
case 'progress': case 'progress':
callbacks.onProgress(args.loaded, args.total); callbacks.onProgress(args.loaded, args.total);
break; break;