Destroy file loaders not delayed.

Telegram/SourceFiles/data/data_document.cpp

@@ -725,7 +725,7 @@ void DocumentData::performActionOnLoad() {
 bool DocumentData::loaded(FilePathResolveType type) const {
 	if (loading() && _loader->finished()) {
 		if (_loader->cancelled()) {
-			destroyLoaderDelayed(CancelledMtpFileLoader);
+			destroyLoader(CancelledMtpFileLoader);
 		} else {
 			auto that = const_cast<DocumentData*>(this);
 			that->_location = FileLocation(_loader->fileName());
@@ -748,17 +748,20 @@ bool DocumentData::loaded(FilePathResolveType type) const {
 			}
 
 			that->refreshGoodThumbnail();
-			destroyLoaderDelayed();
+			destroyLoader();
 		}
 		_session->data().notifyDocumentLayoutChanged(this);
 	}
 	return !data().isEmpty() || !filepath(type).isEmpty();
 }
 
-void DocumentData::destroyLoaderDelayed(mtpFileLoader *newValue) const {
-	_loader->stop();
-	auto loader = std::unique_ptr<FileLoader>(std::exchange(_loader, newValue));
-	_session->downloader().delayedDestroyLoader(std::move(loader));
+void DocumentData::destroyLoader(mtpFileLoader *newValue) const {
+	const auto loader = std::exchange(_loader, newValue);
+	if (_loader == CancelledMtpFileLoader) {
+		loader->cancel();
+	}
+	loader->stop();
+	delete loader;
 }
 
 bool DocumentData::loading() const {
@@ -838,7 +841,9 @@ void DocumentData::save(
 		return;
 	}
 
-	if (_loader == CancelledMtpFileLoader) _loader = nullptr;
+	if (_loader == CancelledMtpFileLoader) {
+		_loader = nullptr;
+	}
 	if (_loader) {
 		if (!_loader->setFileName(toFile)) {
 			cancel(); // changes _actionOnLoad
@@ -894,10 +899,7 @@ void DocumentData::cancel() {
 		return;
 	}
 
-	auto loader = std::unique_ptr<FileLoader>(std::exchange(_loader, CancelledMtpFileLoader));
-	loader->cancel();
-	loader->stop();
-	_session->downloader().delayedDestroyLoader(std::move(loader));
+	destroyLoader(CancelledMtpFileLoader);
 	_session->data().notifyDocumentLayoutChanged(this);
 	if (auto main = App::main()) {
 		main->documentLoadProgress(this);
@@ -1382,7 +1384,7 @@ void DocumentData::collectLocalData(DocumentData *local) {
 
 DocumentData::~DocumentData() {
 	if (loading()) {
-		destroyLoaderDelayed();
+		destroyLoader();
 	}
 	unload();
 	ActiveCache().remove(this);

Telegram/SourceFiles/data/data_document.h

@@ -220,7 +220,7 @@ private:
 	LocationType locationType() const;
 	void validateGoodThumbnail();
 
-	void destroyLoaderDelayed(mtpFileLoader *newValue = nullptr) const;
+	void destroyLoader(mtpFileLoader *newValue = nullptr) const;
 
 	// Two types of location: from MTProto by dc+access or from web by url
 	int32 _dc = 0;

Telegram/SourceFiles/storage/file_download.cpp

@@ -22,13 +22,7 @@ https://github.com/telegramdesktop/tdesktop/blob/master/LEGAL
 
 namespace Storage {
 
-Downloader::Downloader()
-: _delayedLoadersDestroyer([this] { _delayedDestroyedLoaders.clear(); }) {
-}
-
-void Downloader::delayedDestroyLoader(std::unique_ptr<FileLoader> loader) {
-	_delayedDestroyedLoaders.push_back(std::move(loader));
-	_delayedLoadersDestroyer.call();
+Downloader::Downloader() {
 }
 
 void Downloader::clearPriorities() {
@@ -63,12 +57,7 @@ int Downloader::chooseDcIndexForRequest(MTP::DcId dcId) const {
 	return result;
 }
 
-Downloader::~Downloader() {
-	// The file loaders have pointer to downloader and they cancel
-	// requests in destructor where they use that pointer, so all
-	// of them need to be destroyed before any internal state of Downloader.
-	_delayedDestroyedLoaders.clear();
-}
+Downloader::~Downloader() = default;
 
 } // namespace Storage
 
@@ -202,13 +191,19 @@ void FileLoader::permitLoadFromCloud() {
 	_fromCloud = LoadFromCloudOrLocal;
 }
 
-void FileLoader::loadNext() {
-	if (_queue->queriesCount >= _queue->queriesLimit) {
+void FileLoader::notifyAboutProgress() {
+	const auto queue = _queue;
+	emit progress(this);
+	LoadNextFromQueue(queue);
+}
+
+void FileLoader::LoadNextFromQueue(not_null<FileLoaderQueue*> queue) {
+	if (queue->queriesCount >= queue->queriesLimit) {
 		return;
 	}
-	for (auto i = _queue->start; i;) {
+	for (auto i = queue->start; i;) {
 		if (i->loadPart()) {
-			if (_queue->queriesCount >= _queue->queriesLimit) {
+			if (queue->queriesCount >= queue->queriesLimit) {
 				return;
 			}
 		} else {
@@ -259,10 +254,7 @@ void FileLoader::localLoaded(
 		_imageData = imageData;
 	}
 	finishWithBytes(result.data);
-
-	emit progress(this);
-
-	loadNext();
+	notifyAboutProgress();
 }
 
 void FileLoader::start(bool loadFirst, bool prior) {
@@ -428,12 +420,14 @@ bool FileLoader::tryLoadLocal() {
 		return true;
 	}
 
+	const auto weak = make_weak(this);
 	if (const auto key = cacheKey()) {
 		loadLocal(*key);
 		emit progress(this);
 	}
-
-	if (_localStatus != LocalStatus::NotTried) {
+	if (!weak) {
+		return false;
+	} else if (_localStatus != LocalStatus::NotTried) {
 		return _finished;
 	} else if (_localLoading.alive()) {
 		_localStatus = LocalStatus::Loading;
@@ -460,16 +454,18 @@ void FileLoader::cancel(bool fail) {
 	_data = QByteArray();
 	removeFromQueue();
 
+	const auto queue = _queue;
+	const auto weak = make_weak(this);
 	if (fail) {
 		emit failed(this, started);
 	} else {
 		emit progress(this);
 	}
-
-	_filename = QString();
-	_file.setFileName(_filename);
-
-	loadNext();
+	if (weak) {
+		_filename = QString();
+		_file.setFileName(_filename);
+	}
+	LoadNextFromQueue(queue);
 }
 
 void FileLoader::startLoading(bool loadFirst, bool prior) {
@@ -901,8 +897,7 @@ void mtpFileLoader::getCdnFileHashesDone(const MTPVector<MTPFileHash> &result, m
 				|| !weak) {
 				return;
 			} else if (_finished) {
-				emit progress(this);
-				loadNext();
+				notifyAboutProgress();
 				return;
 			}
 		} break;
@@ -911,9 +906,12 @@ void mtpFileLoader::getCdnFileHashesDone(const MTPVector<MTPFileHash> &result, m
 		}
 	}
 	if (someMoreChecked) {
-		emit progress(this);
-		loadNext();
-		return requestMoreCdnFileHashes();
+		const auto weak = make_weak(this);
+		notifyAboutProgress();
+		if (weak) {
+			requestMoreCdnFileHashes();
+		}
+		return;
 	}
 	LOG(("API Error: Could not find cdnFileHash for offset %1 after getCdnFileHashes request.").arg(offset));
 	cancel(true);
@@ -1029,8 +1027,7 @@ bool mtpFileLoader::feedPart(int offset, bytes::const_span buffer) {
 
 void mtpFileLoader::partLoaded(int offset, bytes::const_span buffer) {
 	if (feedPart(offset, buffer)) {
-		emit progress(this);
-		loadNext();
+		notifyAboutProgress();
 	}
 }
 
@@ -1251,9 +1248,7 @@ void webFileLoader::onFinished(const QByteArray &data) {
 	}
 	_downloader->taskFinished().notify();
 
-	emit progress(this);
-
-	loadNext();
+	notifyAboutProgress();
 }
 
 void webFileLoader::onError() {

Telegram/SourceFiles/storage/file_download.h

@@ -30,8 +30,6 @@ public:
 	}
 	void clearPriorities();
 
-	void delayedDestroyLoader(std::unique_ptr<FileLoader> loader);
-
 	base::Observable<void> &taskFinished() {
 		return _taskFinishedObservable;
 	}
@@ -45,9 +43,6 @@ private:
 	base::Observable<void> _taskFinishedObservable;
 	int _priority = 1;
 
-	SingleQueuedInvokation _delayedLoadersDestroyer;
-	std::vector<std::unique_ptr<FileLoader>> _delayedDestroyedLoaders;
-
 	using RequestedInDc = std::array<int64, MTP::kDownloadSessionsCount>;
 	std::map<MTP::DcId, RequestedInDc> _requestedBytesAmount;
 
@@ -161,7 +156,8 @@ protected:
 	void removeFromQueue();
 	void cancel(bool failed);
 
-	void loadNext();
+	void notifyAboutProgress();
+	static void LoadNextFromQueue(not_null<FileLoaderQueue*> queue);
 	virtual bool loadPart() = 0;
 
 	not_null<Storage::Downloader*> _downloader;

Telegram/SourceFiles/ui/image/image_source.cpp

@@ -287,13 +287,13 @@ QImage RemoteSource::takeLoaded() {
 
 	auto data = _loader->imageData(shrinkBox());
 	if (data.isNull()) {
-		destroyLoaderDelayed(CancelledFileLoader);
+		destroyLoader(CancelledFileLoader);
 		return QImage();
 	}
 
 	setInformation(_loader->bytes().size(), data.width(), data.height());
 
-	destroyLoaderDelayed();
+	destroyLoader();
 
 	return data;
 }
@@ -302,12 +302,15 @@ bool RemoteSource::loaderValid() const {
 	return _loader && _loader != CancelledFileLoader;
 }
 
-void RemoteSource::destroyLoaderDelayed(FileLoader *newValue) {
+void RemoteSource::destroyLoader(FileLoader *newValue) {
 	Expects(loaderValid());
 
-	_loader->stop();
-	auto loader = std::unique_ptr<FileLoader>(std::exchange(_loader, newValue));
-	Auth().downloader().delayedDestroyLoader(std::move(loader));
+	const auto loader = std::exchange(_loader, newValue);
+	if (_loader == CancelledFileLoader) {
+		loader->cancel();
+	}
+	loader->stop();
+	delete loader;
 }
 
 void RemoteSource::loadLocal() {
@@ -401,11 +404,7 @@ bool RemoteSource::displayLoading() {
 void RemoteSource::cancel() {
 	if (!loaderValid()) return;
 
-	const auto loader = std::exchange(_loader, CancelledFileLoader);
-	loader->cancel();
-	loader->stop();
-	Auth().downloader().delayedDestroyLoader(
-		std::unique_ptr<FileLoader>(loader));
+	destroyLoader(CancelledFileLoader);
 }
 
 void RemoteSource::unload() {

Telegram/SourceFiles/ui/image/image_source.h

@@ -169,7 +169,7 @@ protected:
 
 private:
 	bool loaderValid() const;
-	void destroyLoaderDelayed(FileLoader *newValue = nullptr);
+	void destroyLoader(FileLoader *newValue = nullptr);
 
 	FileLoader *_loader = nullptr;
 

Telegram/SourceFiles/ui/widgets/inner_dropdown.cpp

@@ -96,7 +96,7 @@ void InnerDropdown::onScroll() {
 void InnerDropdown::paintEvent(QPaintEvent *e) {
 	Painter p(this);
 
-	auto ms = getms();
+	const auto ms = getms();
 	if (_a_show.animating(ms)) {
 		if (auto opacity = _a_opacity.current(ms, _hiding ? 0. : 1.)) {
 			// _a_opacity.current(ms)->opacityAnimationCallback()->_showAnimation.reset()
@@ -115,7 +115,7 @@ void InnerDropdown::paintEvent(QPaintEvent *e) {
 		showChildren();
 	} else {
 		if (!_cache.isNull()) _cache = QPixmap();
-		auto inner = rect().marginsRemoved(_st.padding);
+		const auto inner = rect().marginsRemoved(_st.padding);
 		Shadow::paint(p, inner, width(), _st.shadow);
 		App::roundRect(p, inner, _st.bg, ImageRoundRadius::Small);
 	}
@@ -130,7 +130,7 @@ void InnerDropdown::enterEventHook(QEvent *e) {
 
 void InnerDropdown::leaveEventHook(QEvent *e) {
 	if (_autoHiding) {
-		auto ms = getms();
+		const auto ms = getms();
 		if (_a_show.animating(ms) || _a_opacity.animating(ms)) {
 			hideAnimated();
 		} else {
@@ -148,7 +148,7 @@ void InnerDropdown::otherEnter() {
 
 void InnerDropdown::otherLeave() {
 	if (_autoHiding) {
-		auto ms = getms();
+		const auto ms = getms();
 		if (_a_show.animating(ms) || _a_opacity.animating(ms)) {
 			hideAnimated();
 		} else {