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

Issue 8970032: Changing Aura background for new temporary one. (Closed)

Created:
9 years ago by Emmanuel Saint-loubert-Bié
Modified:
9 years ago
Reviewers:
sky
CC:
chromium-reviews, dhollowa+watch_chromium.org, sadrul, ben+watch_chromium.org
Visibility:
Public.

Description

As per UX request this CL is changing the background for Aura. Note that this is the first resource which is a JPG. As a result I had to modify the bundle code (which assumes PNGs). Converting that file to PNG even with all kind of fancy optimization would triple its size. Note on the PNGCodec, it says in the header: "This is currently designed for use in tests only (where we control the files)" So I investigated using the Skia decoding helper but it seems to be unused and not really working. Finally, I am not crazy about having a fallback for the PNG decoding failure like this in the code. I would have preferred testing for the image type (but there was no clean way which was obvious to me, of course we could look at the first few bytes in the header since we have it memory). BUG=105508 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114871

Patch Set 1 #

Total comments: 2

Patch Set 2 : Much smaller wallpaper. #

Patch Set 3 : Adressed comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -5 lines) Patch
M ui/aura_shell/desktop_background_view.cc View 2 chunks +5 lines, -1 line 0 comments Download
M ui/base/resource/resource_bundle.cc View 1 2 2 chunks +11 lines, -5 lines 0 comments Download
D ui/resources/aura/damask.png View 0 chunks +-1 lines, --1 lines 0 comments Download
A ui/resources/aura/wallpaper.jpg View 1 0 chunks +-1 lines, --1 lines 0 comments Download
M ui/resources/ui_resources.grd View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
Emmanuel Saint-loubert-Bié
Hi Scott, Pleas see my comments in the CL and let me know if you ...
9 years ago (2011-12-16 19:17:06 UTC) #1
sky
LGTM http://codereview.chromium.org/8970032/diff/1/ui/base/resource/resource_bundle.cc File ui/base/resource/resource_bundle.cc (right): http://codereview.chromium.org/8970032/diff/1/ui/base/resource/resource_bundle.cc#newcode377 ui/base/resource/resource_bundle.cc:377: SkBitmap *allocated_bitmap = 'SkBitmap *' -> 'SkBitmap* '
9 years ago (2011-12-16 19:27:23 UTC) #2
Emmanuel Saint-loubert-Bié
9 years ago (2011-12-16 21:43:39 UTC) #3
http://codereview.chromium.org/8970032/diff/1/ui/base/resource/resource_bundl...
File ui/base/resource/resource_bundle.cc (right):

http://codereview.chromium.org/8970032/diff/1/ui/base/resource/resource_bundl...
ui/base/resource/resource_bundle.cc:377: SkBitmap *allocated_bitmap =
On 2011/12/16 19:27:23, sky wrote:
> 'SkBitmap *' -> 'SkBitmap* '

Done.

Powered by Google App Engine
This is Rietveld 408576698