Introduce a priority queue to the image loader. (issue 14623021)

[mto...@chromium.org]

Reviewers: James Hawkins,

Message:
@jhawkins: PTAL. Thanks.



https://codereview.chromium.org/14623021/diff/1/chrome/browser/resources/image_loader/OWNERS
File chrome/browser/resources/image_loader/OWNERS (left):

https://codereview.chromium.org/14623021/diff/1/chrome/browser/resources/image_loader/OWNERS#oldcode2
chrome/browser/resources/image_loader/OWNERS:2: sat...@chromium.org
I don't understand this. I've added a new OWNERS file. Why it says that
I've removed satorux?

Description:
Introduce a priority queue to the image loader.

Image loader used to start loading all requests at once, which caused long
loading times in some scenarios. Eg. while images in the mosaic view are
being
loaded in the background, switching to the next image in the slide view is
slow.
It may happen that there are hundreds of pending images (for mosaic view
preload) which have to be handled before the one is important (slide view).

To fix this issue (1) priorities were introduces, (2) tasks are queued, (3)
number of tasks performed in parallel is limited to 5.
In practice, it gives a guarantee that there will be at most 5 tasks with
lower
priority executed before a task with higher priority. Taking into account an
average resizing time, which is around 200ms, this gives a guarantee that on
average, the image with the highest priority will be handled in the worst
case
with 1 second of delay.

Along the way, the selected images in the mosaic view have higher priority
than
other tiles. Also, background thumbnail generation for copying images has
now
very low priority to minimize load of important images.

TEST=Tested manually.
BUG=239237

Please review this at https://codereview.chromium.org/14623021/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M chrome/browser/resources/file_manager/js/file_manager.js
M chrome/browser/resources/file_manager/js/media/media_util.js
M chrome/browser/resources/file_manager/js/photo/mosaic_mode.js
A + chrome/browser/resources/image_loader/OWNERS
M chrome/browser/resources/image_loader/client.js
M chrome/browser/resources/image_loader/image_loader.js

[mto...@chromium.org]

https://codereview.chromium.org/14623021/diff/1/chrome/browser/resources/image_loader/OWNERS
File chrome/browser/resources/image_loader/OWNERS (left):

https://codereview.chromium.org/14623021/diff/1/chrome/browser/resources/image_loader/OWNERS#oldcode2
chrome/browser/resources/image_loader/OWNERS:2: sat...@chromium.org

On 2013/05/10 11:14:52, mtomasz wrote:
> I don't understand this. I've added a new OWNERS file. Why it says
that I've
> removed satorux?

Got it. Git cl upload recognized it as a copy of another OWNERS file.

https://codereview.chromium.org/14623021/

[mto...@chromium.org]

On 2013/05/10 12:09:51, mtomasz wrote:

https://codereview.chromium.org/14623021/diff/1/chrome/browser/resources/image_loader/OWNERS
> File chrome/browser/resources/image_loader/OWNERS (left):


https://codereview.chromium.org/14623021/diff/1/chrome/browser/resources/image_loader/OWNERS#oldcode2
> chrome/browser/resources/image_loader/OWNERS:2:

> mailto:sat...@chromium.org

> On 2013/05/10 11:14:52, mtomasz wrote:
> > I don't understand this. I've added a new OWNERS file. Why it says that
> I've
> > removed satorux?

> Got it. Git cl upload recognized it as a copy of another OWNERS file.

@jhawkins: Ping.


https://codereview.chromium.org/14623021/

[jhaw...@chromium.org]

You're going to need to get a file_manager OWNER.

https://codereview.chromium.org/14623021/

[jhaw...@chromium.org]

image_loader LGTM with nits.


https://codereview.chromium.org/14623021/diff/2001/chrome/browser/resources/image_loader/image_loader.js
File chrome/browser/resources/image_loader/image_loader.js (right):

https://codereview.chromium.org/14623021/diff/2001/chrome/browser/resources/image_loader/image_loader.js#newcode261
chrome/browser/resources/image_loader/image_loader.js:261: *
nit: Remove blank line.

https://codereview.chromium.org/14623021/diff/2001/chrome/browser/resources/image_loader/image_loader.js#newcode290
chrome/browser/resources/image_loader/image_loader.js:290: *
nit: Please remove all of these added blank lines.

https://codereview.chromium.org/14623021/

[mto...@chromium.org]

https://codereview.chromium.org/14623021/diff/2001/chrome/browser/resources/image_loader/image_loader.js
File chrome/browser/resources/image_loader/image_loader.js (right):

https://codereview.chromium.org/14623021/diff/2001/chrome/browser/resources/image_loader/image_loader.js#newcode261
chrome/browser/resources/image_loader/image_loader.js:261: *

On 2013/05/15 17:37:18, James Hawkins wrote:
> nit: Remove blank line.

Why? We put an empty line after the description for jsdoc block with
more than 3 lines for better readibility. We try to be consistent with
it.

https://codereview.chromium.org/14623021/

[mto...@chromium.org]

@yoshiki: PTAL at the Files.app's code.

https://codereview.chromium.org/14623021/

[yos...@chromium.org]

file_manager lgtm

https://codereview.chromium.org/14623021/