Commit 48f888de authored by James Cook's avatar James Cook Committed by Commit Bot

Chrome OS settings: Don't process avatar images on the UI thread

When the user selects a JPG file from disk to use as their profile image
we send the image to the renderer as a base64 encoded PNG. The PNG
encoding can be slow, so post a task to move encoding off the UI thread.

Test: Open OS settings > Personalization > Change device account image,
  and select a large JPEG. Select file window closing animation plays
  smoothly.

Bug: 812180
Change-Id: Ied7cebe1e85be580b5cd49e5ffb077815e73ebba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1826002
Commit-Queue: James Cook <jamescook@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Auto-Submit: James Cook <jamescook@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700451}
parent 9017789b
......@@ -17,6 +17,7 @@
#include "base/stl_util.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task/post_task.h"
#include "base/values.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
......@@ -225,11 +226,17 @@ void ChangePictureHandler::SendSelectedImage() {
user->has_image_bytes()) {
previous_image_bytes_ = user->image_bytes();
SendOldImage(webui::GetPngDataUrl(previous_image_bytes_->front(),
previous_image_bytes_->size()),
-1);
previous_image_bytes_->size()));
} else {
previous_image_bytes_ = nullptr;
SendOldImage(webui::GetBitmapDataUrl(*previous_image_.bitmap()), -1);
DCHECK(previous_image_.IsThreadSafe());
// Post a task because GetBitmapDataUrl does PNG encoding, which is
// slow for large images.
base::PostTaskAndReplyWithResult(
FROM_HERE, {base::ThreadPool(), base::TaskPriority::USER_BLOCKING},
base::BindOnce(&webui::GetBitmapDataUrl, *previous_image_.bitmap()),
base::BindOnce(&ChangePictureHandler::SendOldImage,
weak_ptr_factory_.GetWeakPtr()));
}
break;
}
......@@ -250,7 +257,7 @@ void ChangePictureHandler::SendSelectedImage() {
previous_image_ = user->GetImage();
previous_image_bytes_ = nullptr;
previous_image_format_ = user_manager::UserImage::FORMAT_UNKNOWN;
SendOldImage(
SendOldImageWithIndex(
default_user_image::GetDefaultImageUrl(previous_image_index_),
previous_image_index_);
}
......@@ -277,11 +284,15 @@ void ChangePictureHandler::UpdateProfileImage() {
user_image_manager->DownloadProfileImage(kProfileDownloadReason);
}
void ChangePictureHandler::SendOldImage(const std::string& image_url,
int image_index) {
void ChangePictureHandler::SendOldImage(std::string&& image_url) {
SendOldImageWithIndex(std::move(image_url), -1);
}
void ChangePictureHandler::SendOldImageWithIndex(std::string&& image_url,
int image_index) {
base::DictionaryValue result;
result.SetString("url", image_url);
result.SetInteger("index", image_index);
result.SetStringPath("url", std::move(image_url));
result.SetIntPath("index", image_index);
FireWebUIListener("old-image-changed", result);
}
......
......@@ -6,6 +6,7 @@
#define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_CHANGE_PICTURE_HANDLER_H_
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h"
#include "chrome/browser/chromeos/camera_presence_notifier.h"
#include "chrome/browser/image_decoder.h"
......@@ -60,11 +61,14 @@ class ChangePictureHandler : public ::settings::SettingsPageUIHandler,
// if any, on the page. Shouldn't be called before |SendProfileImage|.
void UpdateProfileImage();
// Sends the previous user image to the page.
void SendOldImage(std::string&& image_url);
// Sends the previous user image to the page. Also sends |image_index| which
// is either the index of the previous user image (if it was from an older
// default image set) or -1 otherwise. This allows the WebUI to show credits
// for older default images.
void SendOldImage(const std::string& image_url, int image_index);
void SendOldImageWithIndex(std::string&& image_url, int image_index);
// Starts camera presence check.
void CheckCameraPresence();
......@@ -139,6 +143,8 @@ class ChangePictureHandler : public ::settings::SettingsPageUIHandler,
user_manager_observer_;
ScopedObserver<CameraPresenceNotifier, ChangePictureHandler> camera_observer_;
base::WeakPtrFactory<ChangePictureHandler> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(ChangePictureHandler);
};
......
......@@ -36,8 +36,8 @@
namespace webui {
std::string GetBitmapDataUrl(const SkBitmap& bitmap) {
TRACE_EVENT2("oobe", "GetImageDataUrl",
"width", bitmap.width(), "height", bitmap.height());
TRACE_EVENT2("ui", "GetBitmapDataUrl", "width", bitmap.width(), "height",
bitmap.height());
std::vector<unsigned char> output;
gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, &output);
return GetPngDataUrl(output.data(), output.size());
......
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