Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(165)

Issue 12304013: Introduce Image loader extension. (Closed)

Created:
7 years, 10 months ago by mtomasz
Modified:
7 years, 9 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, tfarina, Aaron Boodman, rginda+watch_chromium.org, arv+watch_chromium.org, chromium-apps-reviews_chromium.org
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : Cleanup. #

Total comments: 24

Patch Set 3 : Addressed comments + cleaned up media_util.js. #

Patch Set 4 : Simplified. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+790 lines, -104 lines) Patch
M build/common.gypi View 1 2 3 5 chunks +13 lines, -1 line 0 comments Download
M chrome/browser/browser_resources.grd View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/extensions/component_loader.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/component_loader.cc View 1 2 2 chunks +17 lines, -0 lines 0 comments Download
M chrome/browser/resources/component_extension_resources.grd View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/css/photo_import.css View 1 2 1 chunk +15 lines, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/gallery.html View 1 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/js/action_choice.js View 1 2 1 chunk +6 lines, -1 line 0 comments Download
M chrome/browser/resources/file_manager/js/file_selection.js View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/js/image_editor/image_util.js View 2 chunks +3 lines, -5 lines 0 comments Download
M chrome/browser/resources/file_manager/js/main_scripts.js View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/js/media/media_util.js View 1 2 6 chunks +63 lines, -33 lines 0 comments Download
M chrome/browser/resources/file_manager/js/photo/gallery_scripts.js View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/js/photo/photo_import.js View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/resources/file_manager/js/photo/photo_import_scripts.js View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/js/photo/ribbon.js View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/js/util.js View 1 2 3 1 chunk +19 lines, -63 lines 0 comments Download
M chrome/browser/resources/file_manager/main.html View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/photo_import.html View 1 chunk +2 lines, -0 lines 0 comments Download
A chrome/browser/resources/image_loader/client.js View 1 2 1 chunk +302 lines, -0 lines 0 comments Download
A chrome/browser/resources/image_loader/image_loader.js View 1 2 3 1 chunk +305 lines, -0 lines 0 comments Download
A chrome/browser/resources/image_loader/manifest.json View 1 chunk +19 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
mtomasz
PTAL. More info in the bug's description. @jhawkins: C++ @yoshiki: Javascript.
7 years, 10 months ago (2013-02-19 06:03:30 UTC) #1
James Hawkins
On 2013/02/19 06:03:30, mtomasz wrote: > PTAL. More info in the bug's description. > > ...
7 years, 10 months ago (2013-02-19 20:46:22 UTC) #2
mtomasz
On 2013/02/19 20:46:22, James Hawkins wrote: > On 2013/02/19 06:03:30, mtomasz wrote: > > PTAL. ...
7 years, 10 months ago (2013-02-20 02:45:23 UTC) #3
not at google - send to devlin
lgtm
7 years, 10 months ago (2013-02-20 02:53:42 UTC) #4
James Hawkins
https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/image_loader/client.js File chrome/browser/resources/image_loader/client.js (right): https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/image_loader/client.js#newcode1 chrome/browser/resources/image_loader/client.js:1: // Copyright 2012 The Chromium Authors. All rights reserved. ...
7 years, 10 months ago (2013-02-20 04:31:06 UTC) #5
mtomasz
https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/image_loader/client.js File chrome/browser/resources/image_loader/client.js (right): https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/image_loader/client.js#newcode1 chrome/browser/resources/image_loader/client.js:1: // Copyright 2012 The Chromium Authors. All rights reserved. ...
7 years, 10 months ago (2013-02-21 05:19:23 UTC) #6
James Hawkins
lgtm https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/image_loader/client.js File chrome/browser/resources/image_loader/client.js (right): https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/image_loader/client.js#newcode127 chrome/browser/resources/image_loader/client.js:127: // Replace exchange id. On 2013/02/21 05:19:24, mtomasz ...
7 years, 10 months ago (2013-02-21 23:20:36 UTC) #7
mtomasz
https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/image_loader/image_loader.js File chrome/browser/resources/image_loader/image_loader.js (right): https://codereview.chromium.org/12304013/diff/3001/chrome/browser/resources/image_loader/image_loader.js#newcode60 chrome/browser/resources/image_loader/image_loader.js:60: ImageLoader.load = function() { On 2013/02/21 23:20:36, James Hawkins ...
7 years, 10 months ago (2013-02-22 04:08:33 UTC) #8
mtomasz
@yoshiki: Ping. PTAL at Files.app.
7 years, 10 months ago (2013-02-22 04:08:49 UTC) #9
yoshiki
lgtm
7 years, 10 months ago (2013-02-22 05:08:09 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtomasz@chromium.org/12304013/21001
7 years, 10 months ago (2013-02-22 07:45:36 UTC) #11
commit-bot: I haz the power
Change committed as 184088
7 years, 10 months ago (2013-02-22 11:17:27 UTC) #12
Vladislav Kaznacheev
7 years, 9 months ago (2013-03-06 12:34:15 UTC) #13
Message was sent while issue was closed.
On 2013/02/22 11:17:27, I haz the power (commit-bot) wrote:
> Change committed as 184088

I just saw that. Great work!

Powered by Google App Engine
This is Rietveld 408576698