Commit da158297 authored by Kyle Horimoto's avatar Kyle Horimoto Committed by Commit Bot

[CrOS Network] Fix errors with connecting network icons.

Before this CL, there were several related issues related to connecting
icons; sometimes, icons would appear smaller than they should, and other
times, icons would grow and/or shrink as they animated.

The issue was that network_icon.cc kept four caches for connecting
icons: light bars, dark bars, light arcs, and dark arcs. Each time an
icon was requested, we'd look to see if one existed in the cache and
return it if it existed; otherwise, we'd lazily create a new one.

The problem here was that different areas of the UI request different
icon sizes (e.g., Quick Settings requires a larger icon than the system
tray), but the caches described above were agnostic to size. This meant
that if a small icon was requested first, it would fill up the cache for
that icon type; then, if a larger icon size was requested later, the
same small one would be returned. This made the user-visible bug change
depending on what UIs were used in what order, making this bug difficult
to reproduce consistently.

This CL updates this code to include a cache for each icon size as well
as light vs. dark and bars vs. arcs.

Bug: 961843
Change-Id: I468542f29eec4e694a6bbfaa8623ebfe78700a04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1836454
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702623}
parent dc70013d
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "ash/system/network/network_icon.h" #include "ash/system/network/network_icon.h"
#include <tuple>
#include <utility> #include <utility>
#include "ash/public/cpp/ash_features.h" #include "ash/public/cpp/ash_features.h"
...@@ -14,6 +15,7 @@ ...@@ -14,6 +15,7 @@
#include "ash/system/network/network_icon_animation.h" #include "ash/system/network/network_icon_animation.h"
#include "ash/system/network/network_icon_animation_observer.h" #include "ash/system/network/network_icon_animation_observer.h"
#include "ash/system/tray/tray_constants.h" #include "ash/system/tray/tray_constants.h"
#include "base/containers/flat_map.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/numerics/ranges.h" #include "base/numerics/ranges.h"
...@@ -138,23 +140,14 @@ bool IsTrayIcon(IconType icon_type) { ...@@ -138,23 +140,14 @@ bool IsTrayIcon(IconType icon_type) {
icon_type == ICON_TYPE_TRAY_OOBE; icon_type == ICON_TYPE_TRAY_OOBE;
} }
bool IconTypeIsDark(IconType icon_type) {
// Dark icon is used for OOBE tray icon because the background is white.
return icon_type == ICON_TYPE_TRAY_OOBE;
}
bool IconTypeHasVPNBadge(IconType icon_type) { bool IconTypeHasVPNBadge(IconType icon_type) {
return (icon_type != ICON_TYPE_LIST && icon_type != ICON_TYPE_MENU_LIST); return (icon_type != ICON_TYPE_LIST && icon_type != ICON_TYPE_MENU_LIST);
} }
gfx::Size GetSizeForBaseIconSize(const gfx::Size& base_icon_size) {
return base_icon_size;
}
gfx::ImageSkia CreateNetworkIconImage(const gfx::ImageSkia& icon, gfx::ImageSkia CreateNetworkIconImage(const gfx::ImageSkia& icon,
const Badges& badges) { const Badges& badges) {
return gfx::CanvasImageSource::MakeImageSkia<NetworkIconImageSource>( return gfx::CanvasImageSource::MakeImageSkia<NetworkIconImageSource>(
GetSizeForBaseIconSize(icon.size()), icon, badges); icon.size(), icon, badges);
} }
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
...@@ -199,28 +192,36 @@ gfx::ImageSkia GetImageForIndex(ImageType image_type, ...@@ -199,28 +192,36 @@ gfx::ImageSkia GetImageForIndex(ImageType image_type,
gfx::ImageSkia* ConnectingWirelessImage(ImageType image_type, gfx::ImageSkia* ConnectingWirelessImage(ImageType image_type,
IconType icon_type, IconType icon_type,
double animation) { double animation) {
static const int kImageCount = kNumNetworkImages - 1; // Connecting icons animate by adjusting their signal strength up and down,
static gfx::ImageSkia* s_bars_images_dark[kImageCount]; // but the empty (no signal) image is skipped for aesthetic reasons.
static gfx::ImageSkia* s_bars_images_light[kImageCount]; static const int kNumConnectingImages = kNumNetworkImages - 1;
static gfx::ImageSkia* s_arcs_images_dark[kImageCount];
static gfx::ImageSkia* s_arcs_images_light[kImageCount]; // Cache of images used to avoid redrawing the icon during every animation;
int index = animation * nextafter(static_cast<float>(kImageCount), 0); // the key is a tuple including a bool representing whether the icon displays
index = base::ClampToRange(index, 0, kImageCount - 1); // bars (as oppose to arcs), the IconType, and an int representing the index
gfx::ImageSkia** images; // of the image (with respect to GetImageForIndex()).
bool dark = IconTypeIsDark(icon_type); static base::flat_map<std::tuple<bool, IconType, int>, gfx::ImageSkia*>
if (image_type == BARS) s_image_cache;
images = dark ? s_bars_images_dark : s_bars_images_light;
else // Note that if |image_type| is NONE, arcs are displayed by default.
images = dark ? s_arcs_images_dark : s_arcs_images_light; bool is_bars_image = image_type == BARS;
if (!images[index]) {
int index =
animation * nextafter(static_cast<float>(kNumConnectingImages), 0);
index = base::ClampToRange(index, 0, kNumConnectingImages - 1);
auto map_key = std::make_tuple(is_bars_image, icon_type, index);
if (!s_image_cache.contains(map_key)) {
// Lazily cache images. // Lazily cache images.
// TODO(estade): should the alpha be applied in SignalStrengthImageSource? // TODO(estade): should the alpha be applied in SignalStrengthImageSource?
gfx::ImageSkia source = GetImageForIndex(image_type, icon_type, index + 1); gfx::ImageSkia source = GetImageForIndex(image_type, icon_type, index + 1);
images[index] = s_image_cache[map_key] =
new gfx::ImageSkia(gfx::ImageSkiaOperations::CreateTransparentImage( new gfx::ImageSkia(gfx::ImageSkiaOperations::CreateTransparentImage(
source, kConnectingImageAlpha)); source, kConnectingImageAlpha));
} }
return images[index];
return s_image_cache[map_key];
} }
gfx::ImageSkia ConnectingVpnImage(double animation) { gfx::ImageSkia ConnectingVpnImage(double animation) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment