Skip to content

[Firefox regression] Fix disableRange=true bug in PDFDataTransportStream #10675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ function getDocument(src) {
networkStream = new PDFDataTransportStream({
length: params.length,
initialData: params.initialData,
progressiveDone: params.progressiveDone,
disableRange: params.disableRange,
disableStream: params.disableStream,
}, rangeTransport);
Expand Down Expand Up @@ -389,6 +390,7 @@ function _fetchDocument(worker, source, pdfDataRangeTransport, docId) {
if (pdfDataRangeTransport) {
source.length = pdfDataRangeTransport.length;
source.initialData = pdfDataRangeTransport.initialData;
source.progressiveDone = pdfDataRangeTransport.progressiveDone;
}
return worker.messageHandler.sendWithPromise('GetDocRequest', {
docId,
Expand Down Expand Up @@ -515,13 +517,15 @@ const PDFDocumentLoadingTask = (function PDFDocumentLoadingTaskClosure() {
* @param {Uint8Array} initialData
*/
class PDFDataRangeTransport {
constructor(length, initialData) {
constructor(length, initialData, progressiveDone = false) {
this.length = length;
this.initialData = initialData;
this.progressiveDone = progressiveDone;

this._rangeListeners = [];
this._progressListeners = [];
this._progressiveReadListeners = [];
this._progressiveDoneListeners = [];
this._readyCapability = createPromiseCapability();
}

Expand All @@ -537,6 +541,10 @@ class PDFDataRangeTransport {
this._progressiveReadListeners.push(listener);
}

addProgressiveDoneListener(listener) {
this._progressiveDoneListeners.push(listener);
}

onDataRange(begin, chunk) {
for (const listener of this._rangeListeners) {
listener(begin, chunk);
Expand All @@ -559,6 +567,14 @@ class PDFDataRangeTransport {
});
}

onDataProgressiveDone() {
this._readyCapability.promise.then(() => {
for (const listener of this._progressiveDoneListeners) {
listener();
}
});
}

transportReady() {
this._readyCapability.resolve();
}
Expand Down
30 changes: 26 additions & 4 deletions src/display/transport_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
assert(pdfDataRangeTransport);

this._queuedChunks = [];
var initialData = params.initialData;
this._progressiveDone = params.progressiveDone || false;

const initialData = params.initialData;
if (initialData && initialData.length > 0) {
let buffer = new Uint8Array(initialData).buffer;
this._queuedChunks.push(buffer);
Expand All @@ -47,6 +49,10 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
this._onReceiveData({ chunk, });
});

this._pdfDataRangeTransport.addProgressiveDoneListener(() => {
this._onProgressiveDone();
});

this._pdfDataRangeTransport.transportReady();
}
PDFDataTransportStream.prototype = {
Expand Down Expand Up @@ -80,6 +86,13 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
}
},

_onProgressiveDone() {
if (this._fullRequestReader) {
this._fullRequestReader.progressiveDone();
}
this._progressiveDone = true;
},

_removeRangeReader:
function PDFDataTransportStream_removeRangeReader(reader) {
var i = this._rangeReaders.indexOf(reader);
Expand All @@ -92,7 +105,8 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
assert(!this._fullRequestReader);
var queuedChunks = this._queuedChunks;
this._queuedChunks = null;
return new PDFDataTransportStreamReader(this, queuedChunks);
return new PDFDataTransportStreamReader(this, queuedChunks,
this._progressiveDone);
},

getRangeReader: function PDFDataTransportStream_getRangeReader(begin, end) {
Expand All @@ -116,9 +130,10 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
};

/** @implements {IPDFStreamReader} */
function PDFDataTransportStreamReader(stream, queuedChunks) {
function PDFDataTransportStreamReader(stream, queuedChunks,
progressiveDone = false) {
this._stream = stream;
this._done = false;
this._done = progressiveDone || false;
this._filename = null;
this._queuedChunks = queuedChunks || [];
this._requests = [];
Expand Down Expand Up @@ -180,6 +195,13 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
});
this._requests = [];
},

progressiveDone() {
if (this._done) {
return;
}
this._done = true;
},
};

/** @implements {IPDFStreamRangeReader} */
Expand Down
25 changes: 25 additions & 0 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1526,5 +1526,30 @@ describe('api', function() {
});
}).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);
});
});
});
7 changes: 6 additions & 1 deletion web/firefoxcom.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ PDFViewerApplication.externalServices = {
switch (args.pdfjsLoadAction) {
case 'supportsRangedLoading':
pdfDataRangeTransport =
new FirefoxComDataRangeTransport(args.length, args.data);
new FirefoxComDataRangeTransport(args.length, args.data, args.done);

callbacks.onOpenWithTransport(args.pdfUrl, args.length,
pdfDataRangeTransport);
Expand All @@ -270,6 +270,11 @@ PDFViewerApplication.externalServices = {
case 'progressiveRead':
pdfDataRangeTransport.onDataProgressiveRead(args.chunk);
break;
case 'progressiveDone':
if (pdfDataRangeTransport) {
pdfDataRangeTransport.onDataProgressiveDone();
}
break;
case 'progress':
callbacks.onProgress(args.loaded, args.total);
break;
Expand Down