Skip to content

Commit

Permalink
ui/display/manager: Do not use DisplaySnapshot* in callbacks
Browse files Browse the repository at this point in the history
DisplaySnapshot* may be no longer available when used in callbacks with
different thread, so extract just necessary information to pass onto
callbacks

(cherry picked from commit b8f2d81)

Bug: b/285010517
Test: Pass unit tests
Change-Id: I08463eae9834440ac7f32ec74df4712c2a69ac8f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4575491
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Won Chung <wonchung@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1152755}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4591742
Cr-Commit-Position: refs/branch-heads/5790@{#389}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Won Chung authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent beb83e3 commit e07aa7e
Showing 1 changed file with 18 additions and 7 deletions.
25 changes: 18 additions & 7 deletions ui/display/manager/display_port_observer.cc
Expand Up @@ -23,14 +23,17 @@ namespace {
const LazyRE2 kTypecConnUeventPattern = {R"(TYPEC_PORT=port(\d+))"};

std::vector<uint32_t> ParseDrmSysfsAndFindPort(
const DisplayConfigurator::DisplayStateList& display_states) {
const std::vector<std::pair<uint64_t, base::FilePath>>&
base_connector_id_and_syspath) {
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK);
std::vector<uint32_t> port_nums;
for (auto* state : display_states) {
// Each DisplaySnapshot holds the `sys_path` of a DRM device (i.e.
// /sys/class/drm/cardX).
base::FileEnumerator enumerator(state->sys_path(), false,
// Each pair is from each DisplaySnapshot.
for (const auto& pair : base_connector_id_and_syspath) {
auto base_connector_id = pair.first;
const auto& sys_path = pair.second;
// `sys_path` of a DRM device, i.e. /sys/class/drm/cardX.
base::FileEnumerator enumerator(sys_path, false,
base::FileEnumerator::DIRECTORIES);
// Each directory in `sys_path` represents a connector, so we fetch each
// connector's ID by reading the `/sys/class/drm/*/connector_id` file
Expand All @@ -52,7 +55,7 @@ std::vector<uint32_t> ParseDrmSysfsAndFindPort(
<< path.value();
continue;
}
if (connector_id_int != state->base_connector_id()) {
if (connector_id_int != base_connector_id) {
continue;
}
// A connector with a matching id is found at this point, so break the
Expand Down Expand Up @@ -111,11 +114,19 @@ void DisplayPortObserver::OnDisplayModeChanged(
}
prev_base_connector_ids_ = base_connector_ids_;

// For each DisplaySnapshot, extract base_connector_id and sys_path.
std::vector<std::pair<uint64_t, base::FilePath>>
base_connector_id_and_syspath;
for (auto* state : display_states) {
base_connector_id_and_syspath.push_back(
std::make_pair(state->base_connector_id(), state->sys_path()));
}

base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
base::BindOnce(&ParseDrmSysfsAndFindPort, display_states),
base::BindOnce(&ParseDrmSysfsAndFindPort, base_connector_id_and_syspath),
base::BindOnce(&DisplayPortObserver::SetTypeCPortsUsingDisplays,
weak_ptr_factory_.GetWeakPtr()));
}
Expand Down

0 comments on commit e07aa7e

Please sign in to comment.