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

Issue 657793004: Moved FolderImageSource class to a separate file. (Closed)

Created:
6 years, 2 months ago by Matt Giuca
Modified:
6 years, 2 months ago
Reviewers:
calamity
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git/+/app-list-typedefs-iwyu
Project:
chromium
Visibility:
Public.

Description

Moved FolderImageSource class to a separate file. Previously, this was a private class in app_list_folder_item.cc. It will soon be required from multiple locations (to generate folder icons), so it deserves its own file. Also moved methods/constants relating to the tiny icon size/position into static methods of this class (as needed by various clients). BUG=425444 Committed: https://crrev.com/793ecbcdd2730ac94f6cdc10e08d1b7c4c29e83d Cr-Commit-Position: refs/heads/master@{#301028}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Do supernit. #

Patch Set 3 : Rebase. #

Patch Set 4 : Fix GN. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+180 lines, -111 lines) Patch
M ui/app_list/BUILD.gn View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M ui/app_list/app_list.gyp View 1 chunk +2 lines, -0 lines 0 comments Download
M ui/app_list/app_list_folder_item.h View 1 2 1 chunk +0 lines, -6 lines 0 comments Download
M ui/app_list/app_list_folder_item.cc View 1 2 3 chunks +3 lines, -104 lines 0 comments Download
A ui/app_list/folder_image_source.h View 1 2 1 chunk +56 lines, -0 lines 0 comments Download
A ui/app_list/folder_image_source.cc View 1 chunk +115 lines, -0 lines 0 comments Download
M ui/app_list/views/apps_container_view.cc View 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 16 (6 generated)
Matt Giuca
6 years, 2 months ago (2014-10-21 07:47:21 UTC) #2
calamity
lgtm https://codereview.chromium.org/657793004/diff/1/ui/app_list/folder_image_source.h File ui/app_list/folder_image_source.h (right): https://codereview.chromium.org/657793004/diff/1/ui/app_list/folder_image_source.h#newcode27 ui/app_list/folder_image_source.h:27: nit: remove newline.
6 years, 2 months ago (2014-10-23 05:41:18 UTC) #3
Matt Giuca
https://codereview.chromium.org/657793004/diff/1/ui/app_list/folder_image_source.h File ui/app_list/folder_image_source.h (right): https://codereview.chromium.org/657793004/diff/1/ui/app_list/folder_image_source.h#newcode27 ui/app_list/folder_image_source.h:27: On 2014/10/23 05:41:18, calamity wrote: > nit: remove newline. ...
6 years, 2 months ago (2014-10-23 06:32:38 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/657793004/20001
6 years, 2 months ago (2014-10-23 06:33:03 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/82779) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/72426) android_aosp ...
6 years, 2 months ago (2014-10-23 06:36:22 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/657793004/40001
6 years, 2 months ago (2014-10-23 23:55:58 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_gn_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_rel/builds/27556)
6 years, 2 months ago (2014-10-24 00:16:57 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/657793004/60001
6 years, 2 months ago (2014-10-24 01:05:25 UTC) #14
commit-bot: I haz the power
Committed patchset #4 (id:60001)
6 years, 2 months ago (2014-10-24 02:01:37 UTC) #15
commit-bot: I haz the power
6 years, 2 months ago (2014-10-24 02:02:54 UTC) #16
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/793ecbcdd2730ac94f6cdc10e08d1b7c4c29e83d
Cr-Commit-Position: refs/heads/master@{#301028}

Powered by Google App Engine
This is Rietveld 408576698