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

Issue 11778073: Add support for getting the 256x256 app icon on Windows. (Closed)

Created:
7 years, 11 months ago by Alexei Svitkine (slow)
Modified:
7 years, 11 months ago
CC:
chromium-reviews, erikwright+watch_chromium.org, brettw, willchan no longer on Chromium
Visibility:
Public.

Description

Add support for getting the 256x256 app icon on Windows as a SkBitmap. Also, adds an icon (a copy of chrome/app/theme/chromium/chromium.ico) to ui_unittests.exe, which is needed by the tests for this new functionality. The icon was landed here: https://codereview.chromium.org/11877018/ BUG=167277 TEST=New unit tests in icon_util_unittests.cc. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=176700

Patch Set 1 : #

Total comments: 2

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+207 lines, -27 lines) Patch
M base/win/resource_util.h View 1 1 chunk +13 lines, -2 lines 0 comments Download
M base/win/resource_util.cc View 3 chunks +15 lines, -3 lines 0 comments Download
M chrome/browser/app_icon_win.h View 1 chunk +6 lines, -2 lines 0 comments Download
M chrome/browser/app_icon_win.cc View 1 chunk +17 lines, -14 lines 0 comments Download
M chrome/browser/profiles/profile_shortcut_manager_win.cc View 1 chunk +1 line, -4 lines 0 comments Download
M ui/gfx/icon_util.h View 5 chunks +33 lines, -1 line 0 comments Download
M ui/gfx/icon_util.cc View 4 chunks +61 lines, -1 line 0 comments Download
M ui/gfx/icon_util_unittest.cc View 2 chunks +19 lines, -0 lines 0 comments Download
A ui/test/ui_unittests.rc View 1 chunk +36 lines, -0 lines 0 comments Download
A ui/test/ui_unittests_resource.h View 1 chunk +5 lines, -0 lines 0 comments Download
M ui/ui_unittests.gypi View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Alexei Svitkine (slow)
Hey Scott, Can you review this CL too? It's another change to icon_util.cc. Thanks!
7 years, 11 months ago (2013-01-11 16:28:12 UTC) #1
sky
LGTM. You'll need a base reviewer. https://codereview.chromium.org/11778073/diff/29001/base/win/resource_util.cc File base/win/resource_util.cc (right): https://codereview.chromium.org/11778073/diff/29001/base/win/resource_util.cc#newcode11 base/win/resource_util.cc:11: bool GetResourceFromModule(HMODULE module, ...
7 years, 11 months ago (2013-01-11 17:21:49 UTC) #2
Alexei Svitkine (slow)
brettw: Can you approve the base/ changes? https://codereview.chromium.org/11778073/diff/29001/base/win/resource_util.cc File base/win/resource_util.cc (right): https://codereview.chromium.org/11778073/diff/29001/base/win/resource_util.cc#newcode11 base/win/resource_util.cc:11: bool GetResourceFromModule(HMODULE ...
7 years, 11 months ago (2013-01-11 17:41:18 UTC) #3
Alexei Svitkine (slow)
brettw seems to be OOO willchan: Can you approve the base/ changes here? Thanks!
7 years, 11 months ago (2013-01-11 21:53:25 UTC) #4
Alexei Svitkine (slow)
willchan is apparently in Patagonia. Switching out for jar. Jim, can you approve the base/ ...
7 years, 11 months ago (2013-01-12 14:56:08 UTC) #5
jar (doing other things)
base LGTM
7 years, 11 months ago (2013-01-12 15:59:53 UTC) #6
Alexei Svitkine (slow)
I'm landing the icon file in a separate CL (https://codereview.chromium.org/11877018/) so that this one can ...
7 years, 11 months ago (2013-01-14 15:55:10 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/asvitkine@chromium.org/11778073/36002
7 years, 11 months ago (2013-01-14 16:01:04 UTC) #8
commit-bot: I haz the power
7 years, 11 months ago (2013-01-14 19:45:36 UTC) #9
Message was sent while issue was closed.
Change committed as 176700

Powered by Google App Engine
This is Rietveld 408576698