Introduce Image loader extension.

Introduce Image loader extension.

Files.app's ui used to be unresponsive, when displaying large images. This was caused because of contex.drawImage() synchronous
call on the UI thread, which takes sometimes even 300 ms per one picture. In case of loading more images it causes ui freezes.

This patch solves this issue by introducing an image loader extension, which loads and resizes images asynchronously.
With this fix, we will be able to enable mosaic view on each volume, not only on Drive.

TEST=Check thumbnails in Files.app, including photo importer and gallery.
BUG=175697, 176237, 168035
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=184088

[mtomasz, 2013-02-19 06:03:30]

PTAL. More info in the bug's description.

@jhawkins: C++
@yoshiki: Javascript.

[James Hawkins, 2013-02-19 20:46:22]

On 2013/02/19 06:03:30, mtomasz wrote:
> PTAL. More info in the bug's description.
> 
> @jhawkins: C++
> @yoshiki: Javascript.

Please find appropriate OWNERS for sub-dirs, e.g., chrome/browser/extensions
etc.

[mtomasz, 2013-02-20 02:45:23]

On 2013/02/19 20:46:22, James Hawkins wrote:
> On 2013/02/19 06:03:30, mtomasz wrote:
> > PTAL. More info in the bug's description.
> > 
> > @jhawkins: C++
> > @yoshiki: Javascript.
> 
> Please find appropriate OWNERS for sub-dirs, e.g., chrome/browser/extensions
> etc.

PTAL:

@kalman:
chrome/browser/extensions/component_loader.*

@jhawkins:
chrome/browser/browser_resources.grd
chrome/browser/resources/component_extension_resources.grd
chrome/browser/resources/image_loader/*

@yoshiki:
chrome/browser/resources/file_manager/*

Thanks.

[not at google - send to devlin, 2013-02-20 02:53:42]

lgtm

[James Hawkins, 2013-02-20 04:31:06]

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
File chrome/browser/resources/image_loader/client.js (right):

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:1: // Copyright 2012 The
Chromium Authors. All rights reserved.
2013

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:15: * Client used to connect to
the remote ImageLoader class working in a separate
What is the 'separate extension'?

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:28: this.tasks_ = {};
nit: Document member variables.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:34: * Returns a singleton
instance. Static.
nit: No need to specify 'static'.  Here and elsewhere.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:44: * Loads and resizes and
image. Use opt_isValid to easily cancel requests
s/and/an/?

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:55: ImageLoader.Client.load =
function(url, callback, opt_options, opt_isValid) {
Please remove all of these 'singleton' methods and just have clients call
getInstance().

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:127: // Replace exchange id.
What does 'exchange id' mean?

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:127: // Replace exchange id.
nit: s/id/ID/

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
File chrome/browser/resources/image_loader/image_loader.js (right):

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/image_loader.js:60: ImageLoader.load =
function() {
Why is there a load method?  Just have the client call getInstance().

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/image_loader.js:65: * Returns the
singleton instance. Static.
nit: Remove 'static' here and elsewhere.

[mtomasz, 2013-02-21 05:19:23]

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
File chrome/browser/resources/image_loader/client.js (right):

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:1: // Copyright 2012 The
Chromium Authors. All rights reserved.
On 2013/02/20 04:31:06, James Hawkins wrote:
> 2013

Done.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:15: * Client used to connect to
the remote ImageLoader class working in a separate
On 2013/02/20 04:31:06, James Hawkins wrote:
> What is the 'separate extension'?

Clarified the comment. Client runs in Files.app extension, while ImageLoader
runs in ImageLoader extension. Done.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:28: this.tasks_ = {};
On 2013/02/20 04:31:06, James Hawkins wrote:
> nit: Document member variables.

Done.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:34: * Returns a singleton
instance. Static.
On 2013/02/20 04:31:06, James Hawkins wrote:
> nit: No need to specify 'static'.  Here and elsewhere.

I wanted to use @static, however it is not accepted by our linter. Removed,
anyway. Done.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:44: * Loads and resizes and
image. Use opt_isValid to easily cancel requests
On 2013/02/20 04:31:06, James Hawkins wrote:
> s/and/an/?

Done.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:55: ImageLoader.Client.load =
function(url, callback, opt_options, opt_isValid) {
On 2013/02/20 04:31:06, James Hawkins wrote:
> Please remove all of these 'singleton' methods and just have clients call
> getInstance().

Done.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:127: // Replace exchange id.
On 2013/02/20 04:31:06, James Hawkins wrote:
> What does 'exchange id' mean?

Should be extension id. Fixed.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:127: // Replace exchange id.
On 2013/02/20 04:31:06, James Hawkins wrote:
> nit: s/id/ID/

Done.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:127: // Replace exchange id.
On 2013/02/20 04:31:06, James Hawkins wrote:
> nit: s/id/ID/

git grep 'extension id' | wc -l
152

git grep 'extension ID' | wc -l
133

Can we leave it lowercase for consistency?

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
File chrome/browser/resources/image_loader/image_loader.js (right):

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/image_loader.js:60: ImageLoader.load =
function() {
On 2013/02/20 04:31:06, James Hawkins wrote:
> Why is there a load method?  Just have the client call getInstance().

Client doesn't call it. Client communicates with ImageLoader via messages, so we
have to register the message listener first.

Load() is called in the end of the file, so just when the extension is loaded.
Most of our extensions have load method, so I did it for consistency. What do
you think?

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/image_loader.js:65: * Returns the
singleton instance. Static.
On 2013/02/20 04:31:06, James Hawkins wrote:
> nit: Remove 'static' here and elsewhere.

Done.

[James Hawkins, 2013-02-21 23:20:36]

lgtm

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
File chrome/browser/resources/image_loader/client.js (right):

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/client.js:127: // Replace exchange id.
On 2013/02/21 05:19:24, mtomasz wrote:
> On 2013/02/20 04:31:06, James Hawkins wrote:
> > nit: s/id/ID/
> 
> git grep 'extension id' | wc -l
> 152
> 
> git grep 'extension ID' | wc -l
> 133
> 
> Can we leave it lowercase for consistency?

I'm not going to block the CL on this, but for reference:

id  
Noun
The part of the mind in which innate instinctive impulses and primary processes
are manifest.

ID
Noun
identification.

I personally don't think citing bad precedence is a good reason to do the wrong
thing moving forward, but like I said, not going to block on this.

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
File chrome/browser/resources/image_loader/image_loader.js (right):

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/image_loader.js:60: ImageLoader.load =
function() {
On 2013/02/21 05:19:24, mtomasz wrote:
> On 2013/02/20 04:31:06, James Hawkins wrote:
> > Why is there a load method?  Just have the client call getInstance().
> 
> Client doesn't call it. Client communicates with ImageLoader via messages, so
we
> have to register the message listener first.
> 
> Load() is called in the end of the file, so just when the extension is loaded.
> Most of our extensions have load method, so I did it for consistency. What do
> you think?

Keep the code (more) simple and get rid of it.

[mtomasz, 2013-02-22 04:08:33]

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
File chrome/browser/resources/image_loader/image_loader.js (right):

https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/i...
chrome/browser/resources/image_loader/image_loader.js:60: ImageLoader.load =
function() {
On 2013/02/21 23:20:36, James Hawkins wrote:
> On 2013/02/21 05:19:24, mtomasz wrote:
> > On 2013/02/20 04:31:06, James Hawkins wrote:
> > > Why is there a load method?  Just have the client call getInstance().
> > 
> > Client doesn't call it. Client communicates with ImageLoader via messages,
so
> we
> > have to register the message listener first.
> > 
> > Load() is called in the end of the file, so just when the extension is
loaded.
> > Most of our extensions have load method, so I did it for consistency. What
do
> > you think?
> 
> Keep the code (more) simple and get rid of it.

Done.

[mtomasz, 2013-02-22 04:08:49]

@yoshiki: Ping. PTAL at Files.app.

[yoshiki, 2013-02-22 05:08:09]

lgtm

[commit-bot: I haz the power, 2013-02-22 07:45:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/12304013/21001

[commit-bot: I haz the power, 2013-02-22 11:17:27]

Change committed as 184088

[Vladislav Kaznacheev, 2013-03-06 12:34:15]

On 2013/02/22 11:17:27, I haz the power (commit-bot) wrote:
> Change committed as 184088

I just saw that. Great work!