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

Issue 10883062: Resize and save custom wallpaper to large and small size (Closed)

Created:
8 years, 3 months ago by bshe
Modified:
8 years, 3 months ago
CC:
chromium-reviews, oshima+watch_chromium.org, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org, rkc, Emmanuel Saint-loubert-Bié
Visibility:
Public.

Description

Resize and save custom wallpaper to large and small size if original wallpaper is large. This CL resizes user custom wallpaper if the size of wallpaper is large. 1) If size > 1366x800, scale and save a small (closest resolution to 1366x800) verison of custom wallpaper without changing aspect ratio. 2) If size > 2560x1700, scale and save a large (closest resolution to 2560x1700) verison of custom wallpaper without changing aspect ratio. BUG=144244 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154288

Patch Set 1 #

Patch Set 2 : #

Total comments: 32

Patch Set 3 : #

Total comments: 2

Patch Set 4 : reviews #

Total comments: 8

Patch Set 5 : #

Total comments: 2

Patch Set 6 : ivan's review #

Patch Set 7 : Merge to trunk #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+368 lines, -54 lines) Patch
M ash/desktop_background/desktop_background_controller.h View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
M ash/desktop_background/desktop_background_controller.cc View 1 2 3 3 chunks +7 lines, -4 lines 0 comments Download
A chrome/browser/chromeos/login/simple_jpeg_encoder.h View 1 2 3 4 1 chunk +53 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/login/simple_jpeg_encoder.cc View 1 2 3 4 5 1 chunk +57 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/user_manager_impl.cc View 1 2 3 4 5 6 2 chunks +9 lines, -1 line 0 comments Download
M chrome/browser/chromeos/login/wallpaper_manager.h View 1 2 3 4 chunks +42 lines, -13 lines 0 comments Download
M chrome/browser/chromeos/login/wallpaper_manager.cc View 1 2 3 4 5 6 16 chunks +189 lines, -36 lines 1 comment Download
M chrome/chrome_browser_chromeos.gypi View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
bshe
Hi Ivan. Could you please take a look at this CL? It resizes large custom ...
8 years, 3 months ago (2012-08-28 03:49:02 UTC) #1
bshe
+rkc FYI I use the simple_png_encoder you refactored in this CL: https://chromiumcodereview.appspot.com/10790084/ It looks like ...
8 years, 3 months ago (2012-08-28 14:43:11 UTC) #2
Ivan Korotkov
LGTM with some nits http://codereview.chromium.org/10883062/diff/9001/ash/desktop_background/desktop_background_controller.cc File ash/desktop_background/desktop_background_controller.cc (right): http://codereview.chromium.org/10883062/diff/9001/ash/desktop_background/desktop_background_controller.cc#newcode38 ash/desktop_background/desktop_background_controller.cc:38: const int kSmallWallpaperMaximalWidth = 1366; ...
8 years, 3 months ago (2012-08-28 16:16:12 UTC) #3
bshe
Thanks! +sky for owners ash/*
8 years, 3 months ago (2012-08-28 16:48:18 UTC) #4
sky
LGTM http://codereview.chromium.org/10883062/diff/12002/ash/desktop_background/desktop_background_controller.h File ash/desktop_background/desktop_background_controller.h (right): http://codereview.chromium.org/10883062/diff/12002/ash/desktop_background/desktop_background_controller.h#newcode27 ash/desktop_background/desktop_background_controller.h:27: ASH_EXPORT extern const int kSmallWallpaperMaximalWidth; Document what these ...
8 years, 3 months ago (2012-08-28 19:39:42 UTC) #5
bshe
Thanks for review. ivan@ could you take another quick look before I try to land. ...
8 years, 3 months ago (2012-08-29 14:30:07 UTC) #6
Ivan Korotkov
LGTM, thanks for fixes http://codereview.chromium.org/10883062/diff/11011/ash/desktop_background/desktop_background_controller.h File ash/desktop_background/desktop_background_controller.h (right): http://codereview.chromium.org/10883062/diff/11011/ash/desktop_background/desktop_background_controller.h#newcode28 ash/desktop_background/desktop_background_controller.h:28: // smaller than kSmallWallpaperMaxWidth and ...
8 years, 3 months ago (2012-08-30 10:53:15 UTC) #7
bshe
Thanks Ivan! I have addressed all you comments. Pre requested by saintlou. we need to ...
8 years, 3 months ago (2012-08-30 16:39:02 UTC) #8
Ivan Korotkov
Cool, LGTM https://chromiumcodereview.appspot.com/10883062/diff/31001/chrome/browser/chromeos/login/simple_jpeg_encoder.cc File chrome/browser/chromeos/login/simple_jpeg_encoder.cc (right): https://chromiumcodereview.appspot.com/10883062/diff/31001/chrome/browser/chromeos/login/simple_jpeg_encoder.cc#newcode19 chrome/browser/chromeos/login/simple_jpeg_encoder.cc:19: const int quality = 90; Use kNaming, ...
8 years, 3 months ago (2012-08-30 16:47:34 UTC) #9
bshe
done. Thanks! https://chromiumcodereview.appspot.com/10883062/diff/31001/chrome/browser/chromeos/login/simple_jpeg_encoder.cc File chrome/browser/chromeos/login/simple_jpeg_encoder.cc (right): https://chromiumcodereview.appspot.com/10883062/diff/31001/chrome/browser/chromeos/login/simple_jpeg_encoder.cc#newcode19 chrome/browser/chromeos/login/simple_jpeg_encoder.cc:19: const int quality = 90; On 2012/08/30 ...
8 years, 3 months ago (2012-08-30 16:55:50 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/bshe@chromium.org/10883062/35001
8 years, 3 months ago (2012-08-30 16:56:53 UTC) #11
commit-bot: I haz the power
Failed to apply patch for chrome/chrome_browser.gypi: While running patch -p1 --forward --force; patching file chrome/chrome_browser.gypi ...
8 years, 3 months ago (2012-08-30 16:56:59 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/bshe@chromium.org/10883062/28005
8 years, 3 months ago (2012-08-30 17:04:58 UTC) #13
commit-bot: I haz the power
Change committed as 154288
8 years, 3 months ago (2012-08-30 22:37:28 UTC) #14
Emmanuel Saint-loubert-Bié
Biao, I just added a suggestion on how to handle a TODO... https://chromiumcodereview.appspot.com/10883062/diff/28005/chrome/browser/chromeos/login/wallpaper_manager.cc File chrome/browser/chromeos/login/wallpaper_manager.cc ...
8 years, 3 months ago (2012-08-30 22:53:25 UTC) #15
bshe
8 years, 3 months ago (2012-08-31 13:30:50 UTC) #16
Absolutely. That's also my plan. I have noticed I can save jpeg files to .png
extension. So I will definitely remove the extension to avoid confusion. Thanks
for suggestion.

On 2012/08/30 22:53:25, Emmanuel Saint-loubert-Bié wrote:
> Biao,
> 
> I just added a suggestion on how to handle a TODO...
> 
>
https://chromiumcodereview.appspot.com/10883062/diff/28005/chrome/browser/chr...
> File chrome/browser/chromeos/login/wallpaper_manager.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10883062/diff/28005/chrome/browser/chr...
> chrome/browser/chromeos/login/wallpaper_manager.cc:80: const char
> kOriginalCustomWallpaperSuffix[] = "_wallpaper.png";
> FWIW I have observed that the extension has no impact on the wallpaper
decoding
> (the process that load the file is disjoint form the decoder). 
> So maybe it is best not to have any extension?

Powered by Google App Engine
This is Rietveld 408576698