Decode ImageManager cached images on demand.

Decode ImageManager cached images on demand.

ImageManager was decoding and storing cached images during load, taking 1.27MiB
for 6 NTP tiles in the Clank's case (6 * 296 * 188 * 4). Since NTP tiles are only
requested when NTP page is created, it makes sense to decode them on demand and
store encoded image data in the cache instead.

BUG=493883
Committed: https://crrev.com/960b00e491d272f403dbf24980c0ea4528bc03d3
Cr-Commit-Position: refs/heads/master@{#334063}

[Dmitry Skiba, 2015-06-03 23:07:03]

dskiba@google.com changed reviewers:
+ jam@chromium.org, mathp@chromium.org

[jam, 2015-06-04 16:04:30]

https://codereview.chromium.org/1156003008/diff/30008/components/suggestions/...
File components/suggestions/image_manager.cc (right):

https://codereview.chromium.org/1156003008/diff/30008/components/suggestions/...
components/suggestions/image_manager.cc:136:
content::BrowserThread::PostTaskAndReplyWithResult(
instead of adding a dependency on content just for BrowserThread, have the class
that creates this in chrom pass in the task runner for db thread. that way this
component can avoid a dependency on a big module

[Dmitry Skiba, 2015-06-04 19:59:44]

Done.

On 2015/06/04 16:04:30, jam wrote:
>
https://codereview.chromium.org/1156003008/diff/30008/components/suggestions/...
> File components/suggestions/image_manager.cc (right):
> 
>
https://codereview.chromium.org/1156003008/diff/30008/components/suggestions/...
> components/suggestions/image_manager.cc:136:
> content::BrowserThread::PostTaskAndReplyWithResult(
> instead of adding a dependency on content just for BrowserThread, have the
class
> that creates this in chrom pass in the task runner for db thread. that way
this
> component can avoid a dependency on a big module

[jam, 2015-06-04 22:17:43]

On 2015/06/04 19:59:44, Dmitry Skiba wrote:
> Done.
thanks. I think you don't need my review anymore as a result?

[Dmitry Skiba, 2015-06-04 22:21:20]

On 2015/06/04 22:17:43, jam wrote:
> On 2015/06/04 19:59:44, Dmitry Skiba wrote:
> > Done.
> thanks. I think you don't need my review anymore as a result?

Oh yes, you are right. I'll remove you.

[Dmitry Skiba, 2015-06-04 22:21:43]

dskiba@google.com changed reviewers:
- jam@chromium.org

[Mathieu, 2015-06-08 14:04:32]

Hi Dmitry, sor

[Mathieu, 2015-06-08 14:06:12]

On 2015/06/08 14:04:32, Mathieu Perreault wrote:
> Hi Dmitry, sor

sigh. I was going to say, sorry for the delay again.

The change lgtm, as long as you're confident it will work on iOS (I understand
you've been testing on Android). 

Thanks

[Dmitry Skiba, 2015-06-09 15:14:14]

On 2015/06/08 14:06:12, Mathieu Perreault wrote:
> On 2015/06/08 14:04:32, Mathieu Perreault wrote:
> > Hi Dmitry, sor
> 
> sigh. I was going to say, sorry for the delay again.
> 
> The change lgtm, as long as you're confident it will work on iOS (I understand
> you've been testing on Android). 
> 
> Thanks

Hi Mathieu,

Thanks for the review!

As for iOS - I think I can try building it on my MacBook, but since all
iOS-related tests pass, I think it should "just work".

[stuartmorgan, 2015-06-10 18:18:17]

stuartmorgan@chromium.org changed reviewers:
+ stuartmorgan@chromium.org

[stuartmorgan, 2015-06-10 18:18:18]

+skyostil

https://codereview.chromium.org/1156003008/diff/110001/ios/chrome/browser/sug...
File ios/chrome/browser/suggestions/suggestions_service_factory.mm (right):

https://codereview.chromium.org/1156003008/diff/110001/ios/chrome/browser/sug...
ios/chrome/browser/suggestions/suggestions_service_factory.mm:73:
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
This needs to use WebThread, not BrowserThread. I'm not actually sure how this
compiled, since content/ headers shouldn't be bleeding into this code, ideally.

But there's also an active effort underway to eliminate MessageLoopProxy in
general, and as part of that WebThread had the MLP methods removed. Why isn't
this using SingleThreadedTaskRunner?

[Sami, 2015-06-10 18:40:15]

skyostil@chromium.org changed reviewers:
+ skyostil@chromium.org

[Sami, 2015-06-10 18:40:16]

https://codereview.chromium.org/1156003008/diff/110001/ios/chrome/browser/sug...
File ios/chrome/browser/suggestions/suggestions_service_factory.mm (right):

https://codereview.chromium.org/1156003008/diff/110001/ios/chrome/browser/sug...
ios/chrome/browser/suggestions/suggestions_service_factory.mm:73:
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
On 2015/06/10 18:18:17, stuartmorgan wrote:
> This needs to use WebThread, not BrowserThread. I'm not actually sure how this
> compiled, since content/ headers shouldn't be bleeding into this code,
ideally.
> 
> But there's also an active effort underway to eliminate MessageLoopProxy in
> general, and as part of that WebThread had the MLP methods removed. Why isn't
> this using SingleThreadedTaskRunner?

BrowserThread::GetMessageLoopProxyForThread returns a SingleThreadTaskRunner --
we just haven't renamed it yet :\

But yeah, looks like this should be calling WebThread::GetTaskRunnerForThread().

[Dmitry Skiba, 2015-06-11 21:58:49]

The CQ bit was checked by dskiba@google.com

[Dmitry Skiba, 2015-06-11 21:58:50]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org
Link to the patchset: https://codereview.chromium.org/1156003008/#ps150001
(title: "Don't use MessageLoop API")

[Dmitry Skiba, 2015-06-11 21:58:50]

The CQ bit was unchecked by dskiba@google.com

[Dmitry Skiba, 2015-06-11 22:01:17]

Verified with stuartmorgan@ and justincohen@ that this change doesn't break
Bling.

[Dmitry Skiba, 2015-06-11 22:02:14]

On 2015/06/10 18:40:16, Sami wrote:
>
https://codereview.chromium.org/1156003008/diff/110001/ios/chrome/browser/sug...
> File ios/chrome/browser/suggestions/suggestions_service_factory.mm (right):
> 
>
https://codereview.chromium.org/1156003008/diff/110001/ios/chrome/browser/sug...
> ios/chrome/browser/suggestions/suggestions_service_factory.mm:73:
> BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
> On 2015/06/10 18:18:17, stuartmorgan wrote:
> > This needs to use WebThread, not BrowserThread. I'm not actually sure how
this
> > compiled, since content/ headers shouldn't be bleeding into this code,
> ideally.
> > 
> > But there's also an active effort underway to eliminate MessageLoopProxy in
> > general, and as part of that WebThread had the MLP methods removed. Why
isn't
> > this using SingleThreadedTaskRunner?
> 
> BrowserThread::GetMessageLoopProxyForThread returns a SingleThreadTaskRunner
--
> we just haven't renamed it yet :\
> 
> But yeah, looks like this should be calling
WebThread::GetTaskRunnerForThread().

Done.

[Dmitry Skiba, 2015-06-11 22:35:38]

The CQ bit was checked by dskiba@google.com

[commit-bot: I haz the power, 2015-06-11 22:38:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1156003008/150001

[commit-bot: I haz the power, 2015-06-11 22:44:56]

Committed patchset #9 (id:150001)

[commit-bot: I haz the power, 2015-06-11 22:45:53]

Patchset 9 (id:??) landed as
https://crrev.com/960b00e491d272f403dbf24980c0ea4528bc03d3
Cr-Commit-Position: refs/heads/master@{#334063}