|
|
Created:
4 years ago by Andra Paraschiv Modified:
4 years ago CC:
chromium-apps-reviews_chromium.org, chromium-reviews, extensions-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix icon with badge size for items shown in shelf
Fix icon size for items shown in shelf such that the badge appended to the image
appear correctly.
Based on https://codereview.chromium.org/2341643002
Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>;
BUG=610299
TEST = https://codereview.chromium.org/2297633002
Committed: https://crrev.com/e540da313b52ec3960a635acce256c042335a7eb
Cr-Commit-Position: refs/heads/master@{#437218}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Review v2 #
Total comments: 4
Patch Set 3 : Review v3 #Messages
Total messages: 23 (11 generated)
Description was changed from ========== Fix icon with badge size for items shown in shelf Fix icon size for items shown in shelf such that the badge appended to the image appear correctly. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ========== to ========== Fix icon with badge size for items shown in shelf Fix icon size for items shown in shelf such that the badge appended to the image appear correctly. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ==========
andra.paraschiv@intel.com changed reviewers: + reillyg@chromium.org, skuhne@chromium.org, stevenjb@chromium.org
Description was changed from ========== Fix icon with badge size for items shown in shelf Fix icon size for items shown in shelf such that the badge appended to the image appear correctly. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ========== to ========== Fix icon with badge size for items shown in shelf Fix icon size for items shown in shelf such that the badge appended to the image appear correctly. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ==========
lgtm https://codereview.chromium.org/2530903002/diff/1/extensions/browser/app_wind... File extensions/browser/app_window/app_window.cc (right): https://codereview.chromium.org/2530903002/diff/1/extensions/browser/app_wind... extensions/browser/app_window/app_window.cc:632: // Scale down/up the icon size to large. nit: // Scale the icon to EXTENSION_ICON_LARGE
Thank you, Steven. https://codereview.chromium.org/2530903002/diff/1/extensions/browser/app_wind... File extensions/browser/app_window/app_window.cc (right): https://codereview.chromium.org/2530903002/diff/1/extensions/browser/app_wind... extensions/browser/app_window/app_window.cc:632: // Scale down/up the icon size to large. On 2016/12/05 16:51:11, stevenjb wrote: > nit: // Scale the icon to EXTENSION_ICON_LARGE Done.
The CQ bit was checked by andra.paraschiv@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from stevenjb@chromium.org Link to the patchset: https://codereview.chromium.org/2530903002/#ps20001 (title: "Review v2")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
On 2016/12/06 08:52:51, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) @Reilly, @skuhne Could you please take a look? It is needed to pass the lgtm presubmit checks.
https://codereview.chromium.org/2530903002/diff/20001/extensions/browser/app_... File extensions/browser/app_window/app_window.cc (right): https://codereview.chromium.org/2530903002/diff/20001/extensions/browser/app_... extensions/browser/app_window/app_window.cc:633: int large_icon_size = extension_misc::EXTENSION_ICON_LARGE; nit: const https://codereview.chromium.org/2530903002/diff/20001/extensions/browser/app_... extensions/browser/app_window/app_window.cc:641: resized_image.AsImageSkia(), app_icon_image_->image_skia())); Why are we constantly converting between gfx::Image and ImageSkia here? Why not: gfx::ImageSkia resized_image = gfx::ImageSkiaOperations::CreateResizedImage( base_image.AsImageSkia(), skia::ImageOperations::RESIZE_BEST, gfx::Size(large_icon_size, large_icon_size))); app_icon_ = gfx::Image(gfx::ImageSkiaOperations::CreateIconWithBadge( resized_image, app_icon_image_->image_skia()));
Thank you Reilly, I updated the patch. https://codereview.chromium.org/2530903002/diff/20001/extensions/browser/app_... File extensions/browser/app_window/app_window.cc (right): https://codereview.chromium.org/2530903002/diff/20001/extensions/browser/app_... extensions/browser/app_window/app_window.cc:633: int large_icon_size = extension_misc::EXTENSION_ICON_LARGE; On 2016/12/06 18:49:08, Reilly Grant wrote: > nit: const Done. https://codereview.chromium.org/2530903002/diff/20001/extensions/browser/app_... extensions/browser/app_window/app_window.cc:641: resized_image.AsImageSkia(), app_icon_image_->image_skia())); On 2016/12/06 18:49:08, Reilly Grant wrote: > Why are we constantly converting between gfx::Image and ImageSkia here? Why not: > > gfx::ImageSkia resized_image = gfx::ImageSkiaOperations::CreateResizedImage( > base_image.AsImageSkia(), skia::ImageOperations::RESIZE_BEST, > gfx::Size(large_icon_size, large_icon_size))); > app_icon_ = gfx::Image(gfx::ImageSkiaOperations::CreateIconWithBadge( > resized_image, app_icon_image_->image_skia())); Done.
This is my first time i visit here. I exposed such a variety of charming stuff in your blog Truly its strange piece of writing. http://expressnews.global/
lgtm
The CQ bit was checked by andra.paraschiv@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from stevenjb@chromium.org Link to the patchset: https://codereview.chromium.org/2530903002/#ps40001 (title: "Review v3")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481184342180100, "parent_rev": "0dda3058e19c31b1a5de1d4370d69d689a3c1102", "commit_rev": "695b5844fb11748b9dca0e9d782b0e9f69f9d342"}
Message was sent while issue was closed.
Description was changed from ========== Fix icon with badge size for items shown in shelf Fix icon size for items shown in shelf such that the badge appended to the image appear correctly. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ========== to ========== Fix icon with badge size for items shown in shelf Fix icon size for items shown in shelf such that the badge appended to the image appear correctly. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Fix icon with badge size for items shown in shelf Fix icon size for items shown in shelf such that the badge appended to the image appear correctly. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 ========== to ========== Fix icon with badge size for items shown in shelf Fix icon size for items shown in shelf such that the badge appended to the image appear correctly. Based on https://codereview.chromium.org/2341643002 Co-Authored-By: Valentin Ilie <valentin.ilie@intel.com>; BUG=610299 TEST = https://codereview.chromium.org/2297633002 Committed: https://crrev.com/e540da313b52ec3960a635acce256c042335a7eb Cr-Commit-Position: refs/heads/master@{#437218} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/e540da313b52ec3960a635acce256c042335a7eb Cr-Commit-Position: refs/heads/master@{#437218} |