Decrease the lag when switching between different categories in the Cros wallpaper selector.

Decrease the lag when switching between different categories in the Cros wallpaper selector.

In the CL, we maintain a thumbnail list so that we don't need to fetch the thumbnail images from file system
each time we switch between different categories. It might take some time at the first time, but will be
instantly ready after that.

And we also just preload grid items' thumbnail images in the current viewport, so that we don't experience noticeable
lag when switching to a category at the first time.

BUG=454932
TEST=tested on devices (pixel & peppy)

Committed: https://crrev.com/bb033ee318fc12de0b9e136c99acd25950b08e8f
Cr-Commit-Position: refs/heads/master@{#322015}

[xdai1, 2015-03-20 18:35:52]

Patchset #1 (id:1) has been deleted

[xdai1, 2015-03-20 18:42:23]

xdai@chromium.org changed reviewers:
+ bshe@chromium.org

[xdai1, 2015-03-20 18:42:24]

Biao, could you help to review the CL please? Thanks!

[bshe, 2015-03-21 00:31:46]

On 2015/03/20 18:42:24, xdai1 wrote:
> Biao, could you help to review the CL please? Thanks!

This looks good. But user still experience the lag when they first switch to a
category. I think we might just
need to show thumbnails in the current view port. Grid should have the
capability to do it. See here:
https://code.google.com/p/chromium/codesearch/#chromium/src/ui/webui/resource...
It would be good to spend some time with it. If we could do that, we not only
solve the first time loading issue,
but also much more efficient than this approach.

[xdai1, 2015-03-23 04:42:49]

On 2015/03/21 00:31:46, bshe wrote:
> On 2015/03/20 18:42:24, xdai1 wrote:
> > Biao, could you help to review the CL please? Thanks!
> 
> This looks good. But user still experience the lag when they first switch to a
> category. I think we might just
> need to show thumbnails in the current view port. Grid should have the
> capability to do it. See here:
>
https://code.google.com/p/chromium/codesearch/#chromium/src/ui/webui/resource...
> It would be good to spend some time with it. If we could do that, we not only
> solve the first time loading issue,
> but also much more efficient than this approach.

Can I do this in a following up CL? Since only showing thumbnails in the current
view port also requires maintaing a thumbnail list so that we don't need to
reload them again when switching back.

[xdai1, 2015-03-23 17:28:21]

Patchset #2 (id:40001) has been deleted

[xdai1, 2015-03-23 17:35:47]

On 2015/03/21 00:31:46, bshe wrote:
> On 2015/03/20 18:42:24, xdai1 wrote:
> > Biao, could you help to review the CL please? Thanks!
> 
> This looks good. But user still experience the lag when they first switch to a
> category. I think we might just
> need to show thumbnails in the current view port. Grid should have the
> capability to do it. See here:
>
https://code.google.com/p/chromium/codesearch/#chromium/src/ui/webui/resource...
> It would be good to spend some time with it. If we could do that, we not only
> solve the first time loading issue,
> but also much more efficient than this approach.

Turns out it's easy to just show thumbnails in the current view port. The lag
problem did get improved than before:)
Please take another look at the CL, thanks!

[bshe, 2015-03-23 21:40:32]

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js
(right):

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js:23:
* @param {Object} thumbnail The thumbnail image Object associated with this
nit: s/Object/object

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js:316:
* @param {number} wallpaperId The unique wallpaper ID that associated with
looks like wallpaperId and thumbnail  are optional? In that case, please use
opt_wallpaperId, opt_thumbnail.

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
(right):

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:1073:
for (var key = 0; key < numOnlineWallpaper; key++) {
nit: perhaps rename "key" to "i"?

[xdai1, 2015-03-23 22:06:12]

Biao, thanks for the review! Please take another look, thanks.

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js
(right):

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js:23:
* @param {Object} thumbnail The thumbnail image Object associated with this
On 2015/03/23 21:40:31, bshe wrote:
> nit: s/Object/object

Done.

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js:316:
* @param {number} wallpaperId The unique wallpaper ID that associated with
On 2015/03/23 21:40:31, bshe wrote:
> looks like wallpaperId and thumbnail  are optional? In that case, please use
> opt_wallpaperId, opt_thumbnail.

Done.

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
(right):

https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:1073:
for (var key = 0; key < numOnlineWallpaper; key++) {
On 2015/03/23 21:40:31, bshe wrote:
> nit: perhaps rename "key" to "i"?

Done.

[bshe, 2015-03-23 22:17:39]

On 2015/03/23 22:06:12, xdai1 wrote:
> Biao, thanks for the review! Please take another look, thanks.
> 
>
https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
> File
>
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js
> (right):
> 
>
https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
>
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js:23:
> * @param {Object} thumbnail The thumbnail image Object associated with this
> On 2015/03/23 21:40:31, bshe wrote:
> > nit: s/Object/object
> 
> Done.
> 
>
https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
>
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_images_grid.js:316:
> * @param {number} wallpaperId The unique wallpaper ID that associated with
> On 2015/03/23 21:40:31, bshe wrote:
> > looks like wallpaperId and thumbnail  are optional? In that case, please use
> > opt_wallpaperId, opt_thumbnail.
> 
> Done.
> 
>
https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
> File
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
> (right):
> 
>
https://codereview.chromium.org/1028513003/diff/80001/chrome/browser/resource...
>
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:1073:
> for (var key = 0; key < numOnlineWallpaper; key++) {
> On 2015/03/23 21:40:31, bshe wrote:
> > nit: perhaps rename "key" to "i"?
> 
> Done.

lgtm

[xdai1, 2015-03-24 05:38:43]

Patchset #5 (id:120001) has been deleted

[xdai1, 2015-03-24 16:31:15]

The CQ bit was checked by xdai@chromium.org

[xdai1, 2015-03-24 16:31:16]

The patchset sent to the CQ was uploaded after l-g-t-m from bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/1028513003/#ps140001
(title: "Fix the failed tests.")

[commit-bot: I haz the power, 2015-03-24 16:31:42]

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

[commit-bot: I haz the power, 2015-03-24 17:05:28]

Committed patchset #5 (id:140001)

[commit-bot: I haz the power, 2015-03-24 17:06:11]

Patchset 5 (id:??) landed as
https://crrev.com/bb033ee318fc12de0b9e136c99acd25950b08e8f
Cr-Commit-Position: refs/heads/master@{#322015}

[James Cook, 2015-03-24 20:21:27]

A revert of this CL (patchset #5 id:140001) has been created in
https://codereview.chromium.org/1030973002/ by jamescook@chromium.org.

The reason for reverting is: I suspect this is causing failures in the
cros_trunk official builder:
http://master.chrome.corp.google.com:8011/builders/cros%20trunk/builds/31816

See https://code.google.com/p/chromium/issues/detail?id=470237
.