New issue
Advanced search Search tips

Issue 881680 link

Starred by 1 user
Status: Fixed
Owner:
tapted@chromium.org
Closed: Sep 11
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Files.app ImageLoaderClient leaks all thumbnail |tasks_|

Project Member Reported by tapted@chromium.org, Sep 7 
Chrome Version       : 70.0.3538.7
OS Version: 11021.5.0

What steps will reproduce the problem?
1. Open a folder with ~1000 photos
2. Scroll the folder so thumbnails load
3. Switch to a different folder

What is the expected result?

Memory should restore to a sane state.


What happens instead of that?

Big leak :(. Around 200MB per 1000 photos.

Successful requests to the ImageLoader extension are never cleared from cancellable |tasks_|. Each task wraps a closure which captures a |box| element for storing the thumbnail in order to determine whether it's still in the DOM.

I think https://crrev.com/c/1198673 will fix this.
Screenshot 2018-09-07 at 14.32.28.png
204 KB View Download
Screenshot 2018-09-07 at 14.34.23.png
153 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f589a6c740cfb6ca4f28cce3fdd536684dbbfc3f

commit f589a6c740cfb6ca4f28cce3fdd536684dbbfc3f
Author: Trent Apted <tapted@chromium.org>
Date: Mon Sep 10 07:53:32 2018

CrOS Files App: Remove ImageLoaderClient::tasks_, opt_isValid and some always-undefined args.

ImageLoaderClient::tasks_ is effectively unused since r316185. All it
does now is cause a big JS memory leak whenever a thumbnail is loaded.

This obsoletes ThumbnailLoader.OptimizationMode. Only the Gallery app's
"ribbon" uses this, and it just passed the default arg. It is the only
caller of ThumbnailLoader.load(), so there's no need to have any optional
arguments.

Bug:  881680 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ic1e9a7affee8e81ed3ba4b235b956dd2c3c71d13
Reviewed-on: https://chromium-review.googlesource.com/1214966
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589849}
[modify] https://crrev.com/f589a6c740cfb6ca4f28cce3fdd536684dbbfc3f/ui/file_manager/file_manager/foreground/js/thumbnail_loader.js
[modify] https://crrev.com/f589a6c740cfb6ca4f28cce3fdd536684dbbfc3f/ui/file_manager/gallery/js/ribbon.js
[modify] https://crrev.com/f589a6c740cfb6ca4f28cce3fdd536684dbbfc3f/ui/file_manager/image_loader/image_loader_client.js

Comment 2 by tapted@chromium.org, Sep 11

Status: Fixed (was: Started)
►

Sign in to add a comment