Update: Increase file size limit for range requests (#296)

Increase document file size limit for disabling range requests for users with a non-US locale from 5MB to 25MB. This will hopefully improve performance of previews for larger documents outside the US since we are still limited by latency from round trips.
box · Aug 11, 2017 · 21f490f · 21f490f
1 parent 2dc0200
commit 21f490f
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 24 deletions.
diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js
@@ -35,8 +35,7 @@ const SCROLL_END_TIMEOUT = this.isMobile ? 500 : 250;
 
 const RANGE_REQUEST_CHUNK_SIZE_US = 1048576; // 1MB
 const RANGE_REQUEST_CHUNK_SIZE_NON_US = 524288; // 512KB
-const MINIMUM_RANGE_REQUEST_FILE_SIZE_NON_US = 5242880; // 5MB
-const DISABLE_RANGE_REQUEST_EXENSIONS = ['xls', 'xlsm', 'xlsx'];
+const MINIMUM_RANGE_REQUEST_FILE_SIZE_NON_US = 26214400; // 25MB
 const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536
 
 @autobind
@@ -582,7 +581,7 @@ class DocBaseViewer extends BaseViewer {
     setupPdfjs() {
         // Set PDFJS worker & character maps
         const { file, location } = this.options;
-        const { size, extension, watermark_info: watermarkInfo } = file;
+        const { size, watermark_info: watermarkInfo } = file;
         const assetUrlCreator = createAssetUrlCreator(location);
         PDFJS.workerSrc = assetUrlCreator(`third-party/doc/${DOC_STATIC_ASSETS_VERSION}/pdf.worker.min.js`);
         PDFJS.imageResourcesPath = assetUrlCreator(`third-party/doc/${DOC_STATIC_ASSETS_VERSION}/images/`);
@@ -596,17 +595,12 @@ class DocBaseViewer extends BaseViewer {
         // @NOTE(JustinHoldstock) 2017-04-11: Check to remove this after next IOS release after 10.3.1
         PDFJS.disableFontFace = PDFJS.disableFontFace || Browser.isIOSWithFontIssue();
 
-        // Disable range requests for files smaller than MINIMUM_RANGE_REQUEST_FILE_SIZE (5MB) for
+        // Disable range requests for files smaller than MINIMUM_RANGE_REQUEST_FILE_SIZE (25MB) for
         // previews outside of the US since the additional latency overhead per range request can be
-        // more than the additional time for a continuous request. We don't do this for Excel files
-        // since their representations can be many times larger than the original file. Remove
-        // the Excel check once WinExcel starts generating appropriately-sized representations. This
-        // also overrides any range request disabling that may be set by pdf.js's compatibility checking
-        // since the browsers we support should all be able to properly handle range requests.
-        PDFJS.disableRange =
-            location.locale !== 'en-US' &&
-            size < MINIMUM_RANGE_REQUEST_FILE_SIZE_NON_US &&
-            !DISABLE_RANGE_REQUEST_EXENSIONS.includes(extension);
+        // more than the additional time for a continuous request. This also overrides any range request
+        // disabling that may be set by pdf.js's compatibility checking since the browsers we support
+        // should all be able to properly handle range requests.
+        PDFJS.disableRange = location.locale !== 'en-US' && size < MINIMUM_RANGE_REQUEST_FILE_SIZE_NON_US;
 
         // Disable range requests for watermarked files since they are streamed
         PDFJS.disableRange = PDFJS.disableRange || (watermarkInfo && watermarkInfo.is_watermarked);

diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
@@ -927,31 +927,24 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             expect(PDFJS.disableRange).to.be.false;
         });
 
-        it('should disable range requests if the file is smaller than 5MB and is not an Excel file', () => {
-            docBase.options.file.size = 5242870;
+        it('should disable range requests if locale is not en-US, the file is smaller than 25MB and is not an Excel file', () => {
+            docBase.options.file.size = 26000000;
             docBase.options.extension = 'pdf';
             docBase.options.location.locale = 'ja-JP';
             docBase.setupPdfjs();
             expect(PDFJS.disableRange).to.be.true;
         });
 
-        it('should not disable range requests if the file is an Excel file', () => {
-            docBase.options.location.locale = 'ja-JP';
-            docBase.options.extension = 'xlsx';
-            docBase.setupPdfjs();
-            expect(PDFJS.disableRange).to.be.false;
-        });
-
         it('should disable range requests if the file is watermarked', () => {
             docBase.options.location.locale = 'ja-JP';
             docBase.options.file.watermark_info.is_watermarked = true;
             docBase.setupPdfjs();
             expect(PDFJS.disableRange).to.be.true;
         });
 
-        it('should enable range requests if the file is greater than 5MB, is not Excel, and is not watermarked', () => {
+        it('should enable range requests if locale is not en-US, the file is greater than 25MB and is not watermarked', () => {
             docBase.options.location.locale = 'ja-JP';
-            docBase.options.size = 5242890;
+            docBase.options.file.size = 26500000;
             docBase.options.extension = 'pdf';
             docBase.options.file.watermark_info.is_watermarked = false;
             docBase.setupPdfjs();