Commit 9d0a13d5 authored by Chris Davis's avatar Chris Davis Committed by Commit Bot

Move enumeration of color profile paths off main thread

This change moves the enumeration of color profile paths off main thread
and cache values correctly.  The code currently reads the color profiles
off the main thread so it is not changing when the profiles are made
available.  Also, with the correct caching of the paths we won't be
going down this expensive path as much.  Currently we are re-enumerating
and reading the color profiles whenever a top-level window message is
received. On my fast dev box with 3 monitors just the enumeration of the
color profile paths takes 80ms.  This is way early in startup so it is
critical to get this off the main thread.

Bug: 1012342
Change-Id: I96c36380d485a811c1b94ae38fcabc0a99faea94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1847865
Commit-Queue: Chris Davis <chrdavis@microsoft.com>
Reviewed-by: default avatarccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703940}
parent 110a8ec1
...@@ -52,31 +52,59 @@ ColorProfileReader::ColorProfileReader(Client* client) : client_(client) {} ...@@ -52,31 +52,59 @@ ColorProfileReader::ColorProfileReader(Client* client) : client_(client) {}
ColorProfileReader::~ColorProfileReader() {} ColorProfileReader::~ColorProfileReader() {}
void ColorProfileReader::UpdateIfNeeded() { void ColorProfileReader::UpdateIfNeeded() {
// There is a potential race condition wherein the result from
// EnumDisplayMonitors is already stale by the time that we get
// back to BuildDeviceToPathMapCompleted. To fix this we would
// need to record the fact that we early-out-ed because of
// update_in_flight_ was true, and then re-issue
// BuildDeviceToPathMapOnBackgroundThread when the update
// returned.
if (update_in_flight_) if (update_in_flight_)
return; return;
DeviceToPathMap new_device_to_path_map = BuildDeviceToPathMap();
if (device_to_path_map_ == new_device_to_path_map)
return;
update_in_flight_ = true; update_in_flight_ = true;
// Enumerate device profile paths on a background thread. When this
// completes it will run another task on a background thread to read
// the profiles.
base::PostTaskAndReplyWithResult( base::PostTaskAndReplyWithResult(
FROM_HERE, FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT}, {base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&ColorProfileReader::ReadProfilesOnBackgroundThread, base::Bind(&ColorProfileReader::BuildDeviceToPathMapOnBackgroundThread),
new_device_to_path_map), base::Bind(&ColorProfileReader::BuildDeviceToPathMapCompleted,
base::BindOnce(&ColorProfileReader::ReadProfilesCompleted, weak_factory_.GetWeakPtr()));
weak_factory_.GetWeakPtr()));
} }
// static // static
ColorProfileReader::DeviceToPathMap ColorProfileReader::BuildDeviceToPathMap() { ColorProfileReader::DeviceToPathMap
ColorProfileReader::BuildDeviceToPathMapOnBackgroundThread() {
DeviceToPathMap device_to_path_map; DeviceToPathMap device_to_path_map;
EnumDisplayMonitors(nullptr, nullptr, EnumMonitorForProfilePathCallback, EnumDisplayMonitors(nullptr, nullptr, EnumMonitorForProfilePathCallback,
reinterpret_cast<LPARAM>(&device_to_path_map)); reinterpret_cast<LPARAM>(&device_to_path_map));
return device_to_path_map; return device_to_path_map;
} }
void ColorProfileReader::BuildDeviceToPathMapCompleted(
DeviceToPathMap new_device_to_path_map) {
DCHECK(update_in_flight_);
// Are there any changes from previous results
if (device_to_path_map_ == new_device_to_path_map) {
update_in_flight_ = false;
return;
}
device_to_path_map_ = new_device_to_path_map;
base::PostTaskAndReplyWithResult(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::Bind(&ColorProfileReader::ReadProfilesOnBackgroundThread,
new_device_to_path_map),
base::Bind(&ColorProfileReader::ReadProfilesCompleted,
weak_factory_.GetWeakPtr()));
}
// static // static
ColorProfileReader::DeviceToDataMap ColorProfileReader::DeviceToDataMap
ColorProfileReader::ReadProfilesOnBackgroundThread( ColorProfileReader::ReadProfilesOnBackgroundThread(
......
...@@ -45,8 +45,13 @@ class DISPLAY_EXPORT ColorProfileReader { ...@@ -45,8 +45,13 @@ class DISPLAY_EXPORT ColorProfileReader {
typedef std::map<base::string16, base::string16> DeviceToPathMap; typedef std::map<base::string16, base::string16> DeviceToPathMap;
typedef std::map<base::string16, std::string> DeviceToDataMap; typedef std::map<base::string16, std::string> DeviceToDataMap;
// Enumerate displays and return a map to their ICC profile path. // Enumerate displays and return a map to their ICC profile path. This
static DeviceToPathMap BuildDeviceToPathMap(); // needs to be run off of the main thread.
static ColorProfileReader::DeviceToPathMap
BuildDeviceToPathMapOnBackgroundThread();
// Called on the main thread when the device paths have been retrieved
void BuildDeviceToPathMapCompleted(DeviceToPathMap new_device_to_path_map);
// Do the actual reading from the filesystem. This needs to be run off of the // Do the actual reading from the filesystem. This needs to be run off of the
// main thread. // main thread.
......
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