Merge pull request #4630 from matrix-org/t3chguy/fix_upload_abort

Fix media upload issues with abort and status bar

src/@types/global.d.ts

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 import * as ModernizrStatic from "modernizr";
+import ContentMessages from "../ContentMessages";
 import { IMatrixClientPeg } from "../MatrixClientPeg";
 
 declare global {
@@ -24,6 +25,8 @@ declare global {
         Olm: {
             init: () => Promise<void>;
         };
+
+        mx_ContentMessages: ContentMessages;
     }
 
     // workaround for https://github.com/microsoft/TypeScript/issues/30933

src/ContentMessages.tsx

@@ -1,6 +1,7 @@
 /*
 Copyright 2015, 2016 OpenMarket Ltd
 Copyright 2019 New Vector Ltd
+Copyright 2020 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -15,11 +16,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-'use strict';
+import React from "react";
-
 import extend from './extend';
 import dis from './dispatcher/dispatcher';
 import {MatrixClientPeg} from './MatrixClientPeg';
+import {MatrixClient} from "matrix-js-sdk/src/client";
 import * as sdk from './index';
 import { _t } from './languageHandler';
 import Modal from './Modal';
@@ -39,6 +40,50 @@ const PHYS_HIDPI = [0x00, 0x00, 0x16, 0x25, 0x00, 0x00, 0x16, 0x25, 0x01];
 
 export class UploadCanceledError extends Error {}
 
+type ThumbnailableElement = HTMLImageElement | HTMLVideoElement;
+
+interface IUpload {
+    fileName: string;
+    roomId: string;
+    total: number;
+    loaded: number;
+    promise: Promise<any>;
+    canceled?: boolean;
+}
+
+interface IMediaConfig {
+    "m.upload.size"?: number;
+}
+
+interface IContent {
+    body: string;
+    msgtype: string;
+    info: {
+        size: number;
+        mimetype?: string;
+    };
+    file?: string;
+    url?: string;
+}
+
+interface IThumbnail {
+    info: {
+        thumbnail_info: {
+            w: number;
+            h: number;
+            mimetype: string;
+            size: number;
+        };
+        w: number;
+        h: number;
+    };
+    thumbnail: Blob;
+}
+
+interface IAbortablePromise<T> extends Promise<T> {
+    abort(): void;
+}
+
 /**
  * Create a thumbnail for a image DOM element.
  * The image will be smaller than MAX_WIDTH and MAX_HEIGHT.
@@ -51,13 +96,13 @@ export class UploadCanceledError extends Error {}
  * about the original image and the thumbnail.
  *
  * @param {HTMLElement} element The element to thumbnail.
- * @param {integer} inputWidth The width of the image in the input element.
+ * @param {number} inputWidth The width of the image in the input element.
- * @param {integer} inputHeight the width of the image in the input element.
+ * @param {number} inputHeight the width of the image in the input element.
  * @param {String} mimeType The mimeType to save the blob as.
  * @return {Promise} A promise that resolves with an object with an info key
  *  and a thumbnail key.
  */
-function createThumbnail(element, inputWidth, inputHeight, mimeType) {
+function createThumbnail(element: ThumbnailableElement, inputWidth: number, inputHeight: number, mimeType: string): Promise<IThumbnail> {
     return new Promise((resolve) => {
         let targetWidth = inputWidth;
         let targetHeight = inputHeight;
@@ -98,7 +143,7 @@ function createThumbnail(element, inputWidth, inputHeight, mimeType) {
  * @param {File} imageFile The file to load in an image element.
  * @return {Promise} A promise that resolves with the html image element.
  */
-async function loadImageElement(imageFile) {
+async function loadImageElement(imageFile: File) {
     // Load the file into an html element
     const img = document.createElement("img");
     const objectUrl = URL.createObjectURL(imageFile);
@@ -128,8 +173,7 @@ async function loadImageElement(imageFile) {
             for (const chunk of chunks) {
                 if (chunk.name === 'pHYs') {
                     if (chunk.data.byteLength !== PHYS_HIDPI.length) return;
-                    const hidpi = chunk.data.every((val, i) => val === PHYS_HIDPI[i]);
+                    return chunk.data.every((val, i) => val === PHYS_HIDPI[i]);
-                    return hidpi;
                 }
             }
             return false;
@@ -152,7 +196,7 @@ async function loadImageElement(imageFile) {
  */
 function infoForImageFile(matrixClient, roomId, imageFile) {
     let thumbnailType = "image/png";
-    if (imageFile.type == "image/jpeg") {
+    if (imageFile.type === "image/jpeg") {
         thumbnailType = "image/jpeg";
     }
 
@@ -175,15 +219,15 @@ function infoForImageFile(matrixClient, roomId, imageFile) {
  * @param {File} videoFile The file to load in an video element.
  * @return {Promise} A promise that resolves with the video image element.
  */
-function loadVideoElement(videoFile) {
+function loadVideoElement(videoFile): Promise<HTMLVideoElement> {
     return new Promise((resolve, reject) => {
         // Load the file into an html element
         const video = document.createElement("video");
 
         const reader = new FileReader();
 
-        reader.onload = function(e) {
+        reader.onload = function(ev) {
-            video.src = e.target.result;
+            video.src = ev.target.result as string;
 
             // Once ready, returns its size
             // Wait until we have enough data to thumbnail the first frame.
@@ -231,11 +275,11 @@ function infoForVideoFile(matrixClient, roomId, videoFile) {
  * @return {Promise} A promise that resolves with an ArrayBuffer when the file
  *   is read.
  */
-function readFileAsArrayBuffer(file) {
+function readFileAsArrayBuffer(file: File | Blob): Promise<ArrayBuffer> {
     return new Promise((resolve, reject) => {
         const reader = new FileReader();
         reader.onload = function(e) {
-            resolve(e.target.result);
+            resolve(e.target.result as ArrayBuffer);
         };
         reader.onerror = function(e) {
             reject(e);
@@ -257,11 +301,11 @@ function readFileAsArrayBuffer(file) {
  *  If the file is unencrypted then the object will have a "url" key.
  *  If the file is encrypted then the object will have a "file" key.
  */
-function uploadFile(matrixClient, roomId, file, progressHandler) {
+function uploadFile(matrixClient: MatrixClient, roomId: string, file: File | Blob, progressHandler?: any) {
+    let canceled = false;
     if (matrixClient.isRoomEncrypted(roomId)) {
         // If the room is encrypted then encrypt the file before uploading it.
         // First read the file into memory.
-        let canceled = false;
         let uploadPromise;
         let encryptInfo;
         const prom = readFileAsArrayBuffer(file).then(function(data) {
@@ -278,9 +322,9 @@ function uploadFile(matrixClient, roomId, file, progressHandler) {
                 progressHandler: progressHandler,
                 includeFilename: false,
             });
-
             return uploadPromise;
         }).then(function(url) {
+            if (canceled) throw new UploadCanceledError();
             // If the attachment is encrypted then bundle the URL along
             // with the information needed to decrypt the attachment and
             // add it under a file key.
@@ -290,7 +334,7 @@ function uploadFile(matrixClient, roomId, file, progressHandler) {
             }
             return {"file": encryptInfo};
         });
-        prom.abort = () => {
+        (prom as IAbortablePromise<any>).abort = () => {
             canceled = true;
             if (uploadPromise) MatrixClientPeg.get().cancelUpload(uploadPromise);
         };
@@ -300,55 +344,23 @@ function uploadFile(matrixClient, roomId, file, progressHandler) {
             progressHandler: progressHandler,
         });
         const promise1 = basePromise.then(function(url) {
+            if (canceled) throw new UploadCanceledError();
             // If the attachment isn't encrypted then include the URL directly.
             return {"url": url};
         });
-        // XXX: copy over the abort method to the new promise
+        promise1.abort = () => {
-        promise1.abort = basePromise.abort;
+            canceled = true;
+            MatrixClientPeg.get().cancelUpload(basePromise);
+        };
         return promise1;
     }
 }
 
 export default class ContentMessages {
-    constructor() {
+    private inprogress: IUpload[] = [];
-        this.inprogress = [];
+    private mediaConfig: IMediaConfig = null;
-        this.nextId = 0;
-        this._mediaConfig = null;
-    }
 
-    static sharedInstance() {
+    sendStickerContentToRoom(url: string, roomId: string, info: string, text: string, matrixClient: MatrixClient) {
-        if (global.mx_ContentMessages === undefined) {
-            global.mx_ContentMessages = new ContentMessages();
-        }
-        return global.mx_ContentMessages;
-    }
-
-    _isFileSizeAcceptable(file) {
-        if (this._mediaConfig !== null &&
-            this._mediaConfig["m.upload.size"] !== undefined &&
-            file.size > this._mediaConfig["m.upload.size"]) {
-            return false;
-        }
-        return true;
-    }
-
-    _ensureMediaConfigFetched() {
-        if (this._mediaConfig !== null) return;
-
-        console.log("[Media Config] Fetching");
-        return MatrixClientPeg.get().getMediaConfig().then((config) => {
-            console.log("[Media Config] Fetched config:", config);
-            return config;
-        }).catch(() => {
-            // Media repo can't or won't report limits, so provide an empty object (no limits).
-            console.log("[Media Config] Could not fetch config, so not limiting uploads.");
-            return {};
-        }).then((config) => {
-            this._mediaConfig = config;
-        });
-    }
-
-    sendStickerContentToRoom(url, roomId, info, text, matrixClient) {
         return MatrixClientPeg.get().sendStickerMessage(roomId, url, info, text).catch((e) => {
             console.warn(`Failed to send content with URL ${url} to room ${roomId}`, e);
             throw e;
@@ -356,14 +368,14 @@ export default class ContentMessages {
     }
 
     getUploadLimit() {
-        if (this._mediaConfig !== null && this._mediaConfig["m.upload.size"] !== undefined) {
+        if (this.mediaConfig !== null && this.mediaConfig["m.upload.size"] !== undefined) {
-            return this._mediaConfig["m.upload.size"];
+            return this.mediaConfig["m.upload.size"];
         } else {
             return null;
         }
     }
 
-    async sendContentListToRoom(files, roomId, matrixClient) {
+    async sendContentListToRoom(files: File[], roomId: string, matrixClient: MatrixClient) {
         if (matrixClient.isGuest()) {
             dis.dispatch({action: 'require_registration'});
             return;
@@ -372,8 +384,7 @@ export default class ContentMessages {
         const isQuoting = Boolean(RoomViewStore.getQuotingEvent());
         if (isQuoting) {
             const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog");
-            const shouldUpload = await new Promise((resolve) => {
+            const {finished} = Modal.createTrackedDialog('Upload Reply Warning', '', QuestionDialog, {
-                Modal.createTrackedDialog('Upload Reply Warning', '', QuestionDialog, {
                 title: _t('Replying With Files'),
                 description: (
                     <div>{_t(
@@ -383,21 +394,18 @@ export default class ContentMessages {
                 ),
                 hasCancelButton: true,
                 button: _t("Continue"),
-                    onFinished: (shouldUpload) => {
-                        resolve(shouldUpload);
-                    },
-                });
             });
+            const [shouldUpload]: [boolean] = await finished;
             if (!shouldUpload) return;
         }
 
-        await this._ensureMediaConfigFetched();
+        await this.ensureMediaConfigFetched();
 
         const tooBigFiles = [];
         const okFiles = [];
 
         for (let i = 0; i < files.length; ++i) {
-            if (this._isFileSizeAcceptable(files[i])) {
+            if (this.isFileSizeAcceptable(files[i])) {
                 okFiles.push(files[i]);
             } else {
                 tooBigFiles.push(files[i]);
@@ -406,17 +414,12 @@ export default class ContentMessages {
 
         if (tooBigFiles.length > 0) {
             const UploadFailureDialog = sdk.getComponent("dialogs.UploadFailureDialog");
-            const uploadFailureDialogPromise = new Promise((resolve) => {
+            const {finished} = Modal.createTrackedDialog('Upload Failure', '', UploadFailureDialog, {
-                Modal.createTrackedDialog('Upload Failure', '', UploadFailureDialog, {
                 badFiles: tooBigFiles,
                 totalFiles: files.length,
                 contentMessages: this,
-                    onFinished: (shouldContinue) => {
-                        resolve(shouldContinue);
-                    },
             });
-            });
+            const [shouldContinue]: [boolean] = await finished;
-            const shouldContinue = await uploadFailureDialogPromise;
             if (!shouldContinue) return;
         }
 
@@ -428,31 +431,47 @@ export default class ContentMessages {
         for (let i = 0; i < okFiles.length; ++i) {
             const file = okFiles[i];
             if (!uploadAll) {
-                const shouldContinue = await new Promise((resolve) => {
+                const {finished} = Modal.createTrackedDialog('Upload Files confirmation', '', UploadConfirmDialog, {
-                    Modal.createTrackedDialog('Upload Files confirmation', '', UploadConfirmDialog, {
                     file,
                     currentIndex: i,
                     totalFiles: okFiles.length,
-                        onFinished: (shouldContinue, shouldUploadAll) => {
+                });
+                const [shouldContinue, shouldUploadAll]: [boolean, boolean] = await finished;
+                if (!shouldContinue) break;
                 if (shouldUploadAll) {
                     uploadAll = true;
                 }
-                            resolve(shouldContinue);
-                        },
-                    });
-                });
-                if (!shouldContinue) break;
             }
-            promBefore = this._sendContentToRoom(file, roomId, matrixClient, promBefore);
+            promBefore = this.sendContentToRoom(file, roomId, matrixClient, promBefore);
         }
     }
 
-    _sendContentToRoom(file, roomId, matrixClient, promBefore) {
+    getCurrentUploads() {
-        const content = {
+        return this.inprogress.filter(u => !u.canceled);
+    }
+
+    cancelUpload(promise: Promise<any>) {
+        let upload: IUpload;
+        for (let i = 0; i < this.inprogress.length; ++i) {
+            if (this.inprogress[i].promise === promise) {
+                upload = this.inprogress[i];
+                break;
+            }
+        }
+        if (upload) {
+            upload.canceled = true;
+            MatrixClientPeg.get().cancelUpload(upload.promise);
+            dis.dispatch({action: 'upload_canceled', upload});
+        }
+    }
+
+    private sendContentToRoom(file: File, roomId: string, matrixClient: MatrixClient, promBefore: Promise<any>) {
+        const content: IContent = {
             body: file.name || 'Attachment',
             info: {
                 size: file.size,
             },
+            msgtype: "", // set later
         };
 
         // if we have a mime type for the file, add it to the message metadata
@@ -461,25 +480,25 @@ export default class ContentMessages {
         }
 
         const prom = new Promise((resolve) => {
-            if (file.type.indexOf('image/') == 0) {
+            if (file.type.indexOf('image/') === 0) {
                 content.msgtype = 'm.image';
-                infoForImageFile(matrixClient, roomId, file).then((imageInfo)=>{
+                infoForImageFile(matrixClient, roomId, file).then((imageInfo) => {
                     extend(content.info, imageInfo);
                     resolve();
-                }, (error)=>{
+                }, (e) => {
-                    console.error(error);
+                    console.error(e);
                     content.msgtype = 'm.file';
                     resolve();
                 });
-            } else if (file.type.indexOf('audio/') == 0) {
+            } else if (file.type.indexOf('audio/') === 0) {
                 content.msgtype = 'm.audio';
                 resolve();
-            } else if (file.type.indexOf('video/') == 0) {
+            } else if (file.type.indexOf('video/') === 0) {
                 content.msgtype = 'm.video';
-                infoForVideoFile(matrixClient, roomId, file).then((videoInfo)=>{
+                infoForVideoFile(matrixClient, roomId, file).then((videoInfo) => {
                     extend(content.info, videoInfo);
                     resolve();
-                }, (error)=>{
+                }, (e) => {
                     content.msgtype = 'm.file';
                     resolve();
                 });
@@ -489,11 +508,17 @@ export default class ContentMessages {
             }
         });
 
-        const upload = {
+        // create temporary abort handler for before the actual upload gets passed off to js-sdk
+        (prom as IAbortablePromise<any>).abort = () => {
+            upload.canceled = true;
+        };
+
+        const upload: IUpload = {
             fileName: file.name || 'Attachment',
             roomId: roomId,
-            total: 0,
+            total: file.size,
             loaded: 0,
+            promise: prom,
         };
         this.inprogress.push(upload);
         dis.dispatch({action: 'upload_started'});
@@ -501,15 +526,15 @@ export default class ContentMessages {
         // Focus the composer view
         dis.dispatch({action: 'focus_composer'});
 
-        let error;
-
         function onProgress(ev) {
             upload.total = ev.total;
             upload.loaded = ev.loaded;
             dis.dispatch({action: 'upload_progress', upload: upload});
         }
 
+        let error;
         return prom.then(function() {
+            if (upload.canceled) throw new UploadCanceledError();
             // XXX: upload.promise must be the promise that
             // is returned by uploadFile as it has an abort()
             // method hacked onto it.
@@ -520,16 +545,17 @@ export default class ContentMessages {
                 content.file = result.file;
                 content.url = result.url;
             });
-        }).then((url) => {
+        }).then(() => {
             // Await previous message being sent into the room
             return promBefore;
         }).then(function() {
+            if (upload.canceled) throw new UploadCanceledError();
             return matrixClient.sendMessage(roomId, content);
         }, function(err) {
             error = err;
             if (!upload.canceled) {
                 let desc = _t("The file '%(fileName)s' failed to upload.", {fileName: upload.fileName});
-                if (err.http_status == 413) {
+                if (err.http_status === 413) {
                     desc = _t(
                         "The file '%(fileName)s' exceeds this homeserver's size limit for uploads",
                         {fileName: upload.fileName},
@@ -542,11 +568,9 @@ export default class ContentMessages {
                 });
             }
         }).finally(() => {
-            const inprogressKeys = Object.keys(this.inprogress);
             for (let i = 0; i < this.inprogress.length; ++i) {
-                const k = inprogressKeys[i];
+                if (this.inprogress[i].promise === upload.promise) {
-                if (this.inprogress[k].promise === upload.promise) {
+                    this.inprogress.splice(i, 1);
-                    this.inprogress.splice(k, 1);
                     break;
                 }
             }
@@ -555,7 +579,7 @@ export default class ContentMessages {
                 // clear the media size limit so we fetch it again next time
                 // we try to upload
                 if (error && error.http_status === 413) {
-                    this._mediaConfig = null;
+                    this.mediaConfig = null;
                 }
                 dis.dispatch({action: 'upload_failed', upload, error});
             } else {
@@ -565,24 +589,35 @@ export default class ContentMessages {
         });
     }
 
-    getCurrentUploads() {
+    private isFileSizeAcceptable(file: File) {
-        return this.inprogress.filter(u => !u.canceled);
+        if (this.mediaConfig !== null &&
+            this.mediaConfig["m.upload.size"] !== undefined &&
+            file.size > this.mediaConfig["m.upload.size"]) {
+            return false;
+        }
+        return true;
     }
 
-    cancelUpload(promise) {
+    private ensureMediaConfigFetched() {
-        const inprogressKeys = Object.keys(this.inprogress);
+        if (this.mediaConfig !== null) return;
-        let upload;
+
-        for (let i = 0; i < this.inprogress.length; ++i) {
+        console.log("[Media Config] Fetching");
-            const k = inprogressKeys[i];
+        return MatrixClientPeg.get().getMediaConfig().then((config) => {
-            if (this.inprogress[k].promise === promise) {
+            console.log("[Media Config] Fetched config:", config);
-                upload = this.inprogress[k];
+            return config;
-                break;
+        }).catch(() => {
+            // Media repo can't or won't report limits, so provide an empty object (no limits).
+            console.log("[Media Config] Could not fetch config, so not limiting uploads.");
+            return {};
+        }).then((config) => {
+            this.mediaConfig = config;
+        });
     }
+
+    static sharedInstance() {
+        if (window.mx_ContentMessages === undefined) {
+            window.mx_ContentMessages = new ContentMessages();
         }
-        if (upload) {
+        return window.mx_ContentMessages;
-            upload.canceled = true;
-            MatrixClientPeg.get().cancelUpload(upload.promise);
-            dis.dispatch({action: 'upload_canceled', upload});
-        }
     }
 }