Commit 111324ea authored by Nick Diego Yamane's avatar Nick Diego Yamane Committed by Commit Bot

x11: Do not delay display list update task execution

This diverged from original implementation, which did not use such 250ms delay,
and is potentially causing issues when xrandr and scale factor change events
come in a short interval, as observed in crbug.com/1019344.

Rationale:

With delayed update, events are coalesced thus leading to issues when
DisplayObserver is waiting only for display metrics changes, e.g: scale factor
changes. This is what RenderWidgetHostViewAura does, in order to react to
system's scale factor changes and sync with the compositor, etc.

In this specific issue, the delayed display list update behaves as follows:

Assuming display list initially has a single display (Display 1). XRandR/Gtk
events start coming:

1. Display 2 added
2. Display 1 removed
3. Display 2 metrics changed (scale factor)

Then they are coalesced into (and delivered through DisplayObserver interface):

1. Display 1 removed
2. Display 2 added

So, RenderWidgetHostViewAura::OnDisplayMetricsChanged() is never called and it
gets out of sync with the compositor.

Bug: 1019344
Change-Id: Ic6a7b1264b5656d36a0b25fd7e3bc07ab419568a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930853
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718325}
parent d044a4bc
...@@ -17,7 +17,6 @@ namespace ui { ...@@ -17,7 +17,6 @@ namespace ui {
namespace { namespace {
constexpr int kMinXrandrVersion = 103; // Need at least xrandr version 1.3 constexpr int kMinXrandrVersion = 103; // Need at least xrandr version 1.3
constexpr auto kDisplayListUpdateDelay = base::TimeDelta::FromMilliseconds(250);
} // namespace } // namespace
...@@ -111,10 +110,10 @@ void XDisplayManager::UpdateDisplayList() { ...@@ -111,10 +110,10 @@ void XDisplayManager::UpdateDisplayList() {
} }
void XDisplayManager::DispatchDelayedDisplayListUpdate() { void XDisplayManager::DispatchDelayedDisplayListUpdate() {
delayed_update_task_.Reset(base::BindOnce(&XDisplayManager::UpdateDisplayList, update_task_.Reset(base::BindOnce(&XDisplayManager::UpdateDisplayList,
base::Unretained(this))); base::Unretained(this)));
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
FROM_HERE, delayed_update_task_.callback(), kDisplayListUpdateDelay); update_task_.callback());
} }
gfx::Point XDisplayManager::GetCursorLocation() const { gfx::Point XDisplayManager::GetCursorLocation() const {
......
...@@ -79,9 +79,8 @@ class COMPONENT_EXPORT(UI_BASE_X) XDisplayManager { ...@@ -79,9 +79,8 @@ class COMPONENT_EXPORT(UI_BASE_X) XDisplayManager {
// decoding events regarding output add/remove. // decoding events regarding output add/remove.
int xrandr_event_base_ = 0; int xrandr_event_base_ = 0;
// The task to delay fetching display info. We delay it so that we can // The task which fetches/updates display list info asynchronously.
// coalesce events. base::CancelableOnceClosure update_task_;
base::CancelableOnceClosure delayed_update_task_;
DISALLOW_COPY_AND_ASSIGN(XDisplayManager); DISALLOW_COPY_AND_ASSIGN(XDisplayManager);
}; };
......
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